fix(xml,xml-linq): reject an undeclared entity reference and an undeclared namespace prefix (#2082, #2083)

Both deferrals were correct -- "narrowing parser acceptance needs reference
evidence this container lacks" -- and the reference now supplies it. .NET throws
XmlException for each: "Reference to undeclared entity '{0}'."
(XmlTextReaderImpl.cs:3829) and "'{0}' is an undeclared prefix." (:7787).

THE ENTITY CASE WAS WORSE THAN ACCEPTANCE. "<r>&nope;</r>" was accepted AND
round-tripped as "<r>&amp;nope;</r>": the reference was reinterpreted as literal
text and RE-ESCAPED, so a caller who loaded and saved a document silently
rewrote it.

The entity check must run on the RAW text, and that is not a preference.
tinyxml2 decodes the five predefined entities during parsing, so once the tree
exists "&amp;nope;" (legal) and "&nope;" (undeclared) are the same five
characters in the same text node.

"Undeclared" is not "not predefined", and the first cut got that wrong: it
rejected anything outside the predefined five, and the repository's own
billion-laughs pin caught it immediately -- that document DECLARES two entities
and then references one. The check now reads the DOCTYPE internal subset. A
declared entity is accepted and, as before, not expanded, which is what keeps
this port free of the billion-laughs exposure.

For the prefix, XML Names 1.0 section 3 is transcribed rather than approximated:
a declaration is in scope for the start-tag it appears on as well as for
descendants, it does not escape to siblings, and the reserved xml and xmlns
prefixes need no declaration -- xml:lang and xml:space are the everyday cases.
Mutations cover all three.

It reaches System.Xml.Linq, and that side's pins said it should. XElement::Parse
shares the loader, so two XLinqNamespaceTests cases are inverted. Those pins
recorded exactly this: "#2083 already owns [it] at the DOM layer; answering it
from the Linq side would settle it by accident. Pinned so that a later change to
it is a DECISION rather than a side effect."

One limit recorded rather than hidden: XmlDocument::Load(filename) hands the
path straight to tinyxml2 and never holds the raw bytes, so the entity check
does not run there. That is ticket #2361. The prefix check runs at both doors.

+6 net tests. Five mutations, all caught. Gate: 17,262 run, 0 failed, 38
executables -- green. Downstream, measured: zero System::Xml sites in either
consumer.

Record: docs/Migration-XmlUndeclaredEntityAndPrefix.md.

Author: robertvokac

CLAUDE.md

@@ -0,0 +1,79 @@
+<!-- SPDX-License-Identifier: MIT -->
+<!-- Copyright (c) Robert Vokac and contributors -->
+
+# Migration — an undeclared entity or namespace prefix is rejected (tickets #2082, #2083)
+
+*2026-08-17.* `XmlDocument::LoadXml` accepted two shapes of malformed XML that .NET rejects. It
+rejects them now, and so does `XDocument::Parse`, which shares the loader.
+
+Landed under `docs/StandingApprovals.md` SA-5. No public signature, layout, vtable or `noexcept`
+change.
+
+---
+
+## 1. What changed
+
+| Input | Was | Is |
+|---|---|---|
+| `<r>&nope;</r>` | accepted, **round-tripped as `<r>&amp;nope;</r>`** | `XmlException` |
+| `<r a='&nope;'/>` | accepted | `XmlException` |
+| `<p:r/>` | accepted, prefix never resolved | `XmlException` |
+| `<r><p:child/></r>`, `<r p:a='1'/>` | accepted | `XmlException` |
+| `<p:r xmlns:p='urn:x'/>` and any declared prefix | accepted | **unchanged** |
+| `<r xml:lang='en'/>`, `<r xml:space='preserve'/>` | accepted | **unchanged** |
+| the five predefined entities, `&#65;`, `&#x41;` | accepted | **unchanged** |
+| a **declared** entity (`<!DOCTYPE r [<!ENTITY g "hi">]><r>&g;</r>`) | accepted, inert | **unchanged** |
+| `&` inside a comment, CDATA section or processing instruction | accepted | **unchanged** |
+
+## 2. Why
+
+.NET throws for both:
+
+* `XmlException("Reference to undeclared entity '{0}'.")` — `XmlTextReaderImpl.cs:3829`;
+* `XmlException("'{0}' is an undeclared prefix.")` — `XmlTextReaderImpl.cs:7787`.
+
+The entity case is the worse of the two, and not because of the acceptance: the reference was
+reinterpreted as literal text **and re-escaped**, so a caller who loaded and saved a document
+silently rewrote it. `&nope;` became `&amp;nope;`.
+
+## 3. Two implementation notes worth knowing
+
+**The entity check runs on the raw text, and it has to.** tinyxml2 *decodes* the five predefined
+entities during parsing, so once the tree exists, `&amp;nope;` (legal — the text `&nope;`) and
+`&nope;` (undeclared) are the same five characters in the same text node. A post-parse walk
+cannot tell them apart.
+
+**"Undeclared" is not "not predefined".** The first cut of the check rejected anything outside
+the predefined five, and the repository's own billion-laughs pin caught it immediately — that
+document *declares* two entities and then references one. The check now reads the DOCTYPE
+internal subset for `<!ENTITY name …>` declarations. A declared entity is accepted and, as
+before, **not expanded**; that parity gap is pre-existing and is what keeps this port free of the
+billion-laughs exposure.
+
+## 4. Two limits, recorded rather than hidden
+
+* **`XmlDocument::Load(filename)` does not run the entity check.** It hands the path straight to
+  tinyxml2 and never holds the raw bytes. The *prefix* check runs at both doors, because it works
+  on the parsed tree. That asymmetry is ticket **#2361**.
+* **A DTD-declared entity is still not expanded.** Unchanged, pre-existing, and out of scope
+  here.
+
+## 5. `System.Xml.Linq` follows, and its pins said it should
+
+`XElement::Parse` and `XDocument::Parse` share the loader, so an undeclared prefix now throws
+there too. The Linq pins that recorded the old behaviour said exactly why they existed: *"#2083
+already owns [this] at the DOM layer; answering it from the Linq side would settle it by
+accident. Pinned so that a later change to it is a decision rather than a side effect."* The
+decision was made at the DOM layer, and this change updates them.
+
+## 6. To migrate
+
+A document with an undeclared entity or prefix is not well-formed XML and .NET never accepted
+it. If you were loading one, you were also silently rewriting it on save.
+
+If you need the prefix, declare it: `<p:r xmlns:p="urn:x"/>`.
+
+## 7. Downstream, measured
+
+Neither `cna` nor `mobile-eggbert` references `XmlDocument` or `System::Xml` — **zero sites in
+both**. Neither repository was modified.

docs/Migration-XmlUndeclaredEntityAndPrefix.md

@@ -153,22 +153,28 @@ TEST(XLinqNamespaceTests, Parse_DefaultDeclaration_IsRecognisedAsANamespaceDecla
     EXPECT_TRUE(decl->getIsNamespaceDeclarationProperty());
 }
 
-// --- Parse: what is deliberately left unchanged ----------------------------------------------
-
-TEST(XLinqNamespaceTests, Parse_UndeclaredPrefix_IsStillAcceptedAndStillUnresolved) {
-    // Deliberately NOT narrowed by #2197. .NET's reader rejects an undeclared prefix, but
-    // narrowing what this runtime accepts is the open question #2083 already owns at the DOM
-    // layer; answering it from the Linq side would settle it by accident. Pinned so that a
-    // later change to it is a decision rather than a side effect.
-    auto e = XElement::Parse("<p:root/>");
-    EXPECT_EQ(e->getNameProperty().getLocalNameProperty(), "p:root");
-    EXPECT_EQ(e->getNameProperty().getNamespaceNameProperty(), "");
-}
-
-TEST(XLinqNamespaceTests, Parse_UndeclaredAttributePrefix_IsStillAcceptedAndStillUnresolved) {
-    auto e = XElement::Parse("<r p:x=\"1\"/>");
-    ASSERT_EQ(e->getAttributesProperty().size(), 1u);
-    EXPECT_EQ(e->getAttributesProperty()[0]->getNameProperty(), XName("p:x"));
+// --- Parse: the DOM-layer decision reaches here too -------------------------------------------
+
+TEST(XLinqNamespaceTests, Fix2083_ParseRejectsAnUndeclaredPrefix) {
+    // These two cases used to pin the OPPOSITE, and said exactly why: ".NET's reader rejects an
+    // undeclared prefix, but narrowing what this runtime accepts is the open question #2083
+    // already owns at the DOM layer; answering it from the Linq side would settle it by
+    // accident. Pinned so that a later change to it is a DECISION rather than a side effect."
+    //
+    // #2083 has now been answered at the DOM layer, from the reference
+    // (XmlTextReaderImpl.cs:7787, Strings.resx Xml_UnknownNs). XElement::Parse goes through
+    // XDocument::Parse and the shared loader, so the decision reaches here -- which is the
+    // consistent outcome those pins were protecting, and this is the change that updates them.
+    EXPECT_THROW((void)XElement::Parse("<p:root/>"), System::Xml::XmlException);
+    EXPECT_THROW((void)XElement::Parse("<r p:x=\"1\"/>"), System::Xml::XmlException);
+}
+
+TEST(XLinqNamespaceTests, Fix2083_ParseStillAcceptsADeclaredPrefix) {
+    // The invariance row: only the UNDECLARED case moved.
+    auto e = XElement::Parse("<p:root xmlns:p=\"urn:x\"/>");
+    ASSERT_NE(e, nullptr);
+    auto withAttribute = XElement::Parse("<r xmlns:p=\"urn:x\" p:x=\"1\"/>");
+    ASSERT_EQ(withAttribute->getAttributesProperty().size(), 2u);
 }
 
 // --- Serialize: qualified names and their declarations ----------------------------------------

modules/xml-linq/tests/System/Xml/Linq/XLinqNamespaceTests.cpp

@@ -4,6 +4,8 @@
 #include "System/Xml/XmlDocument.hpp"
 
 #include <cctype>
+#include <vector>
+#include <string>
 #include <tinyxml2/tinyxml2.h>
 
 #include "System/ArgumentException.hpp"
@@ -15,6 +17,206 @@
 namespace System::Xml {
 
     namespace {
+
+        // ===================================================================================
+        // Ticket #2082 -- an undeclared entity reference.
+        //
+        // Measured by #2073's public-input sweep: LoadXml("<r>&nope;</r>") was ACCEPTED and
+        // round-tripped as "<r>&amp;nope;</r>", so the DOCUMENT'S OWN TEXT CHANGED. That is
+        // worse than mere acceptance: a caller who loads and saves a document silently rewrites
+        // it. .NET throws XmlException("Reference to undeclared entity '{0}'.")
+        // (XmlTextReaderImpl.cs:3829, Strings.resx Xml_UndeclaredEntity).
+        //
+        // THE CHECK MUST RUN ON THE RAW TEXT, and that is not a preference. tinyxml2 DECODES
+        // the five predefined entities during parsing, so by the time the tree exists,
+        // "&amp;nope;" (legal -- the text `&nope;`) and "&nope;" (undeclared) are the same five
+        // characters in the same text node. A post-parse walk cannot tell them apart.
+        // ===================================================================================
+
+        /** @brief XML Names 1.0 NameStartChar, as far as an entity name needs it. */
+        bool IsEntityNameStart(unsigned char c) {
+            return (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || c == '_' || c == ':' ||
+                   c >= 0x80;   // any non-ASCII byte: this scan does not decode UTF-8
+        }
+
+        /** @brief XML Names 1.0 NameChar. */
+        bool IsEntityNameChar(unsigned char c) {
+            return IsEntityNameStart(c) || (c >= '0' && c <= '9') || c == '-' || c == '.';
+        }
+
+        /**
+         * @brief Collects the general-entity names declared in a DOCTYPE internal subset.
+         *
+         * Ticket #2082. The check below must reject an **undeclared** entity, not merely a
+         * non-predefined one, and those are different sets: a document may declare its own with
+         * `<!ENTITY name "...">`. Getting this wrong was caught immediately -- the first cut
+         * rejected the repository's own billion-laughs pin, which declares two entities and
+         * then references one.
+         *
+         * Parameter entities (`<!ENTITY % name ...>`) are skipped: they are referenced with `%`
+         * inside the DTD, never with `&` in content, so they cannot declare a content entity.
+         */
+        std::vector<std::string> DeclaredEntityNames(const std::string& xml) {
+            std::vector<std::string> names;
+            const std::size_t doctype = xml.find("<!DOCTYPE");
+            if (doctype == std::string::npos) return names;
+            const std::size_t open = xml.find('[', doctype);
+            if (open == std::string::npos) return names;
+            const std::size_t close = xml.find(']', open);
+            if (close == std::string::npos) return names;
+
+            const std::string subset = xml.substr(open + 1, close - open - 1);
+            for (std::size_t i = subset.find("<!ENTITY"); i != std::string::npos;
+                 i = subset.find("<!ENTITY", i + 1)) {
+                std::size_t j = i + 8;
+                while (j < subset.size() && std::isspace(static_cast<unsigned char>(subset[j]))) ++j;
+                if (j < subset.size() && subset[j] == '%') continue;   // a parameter entity
+                const std::size_t nameStart = j;
+                while (j < subset.size() && IsEntityNameChar(static_cast<unsigned char>(subset[j]))) ++j;
+                if (j > nameStart) names.push_back(subset.substr(nameStart, j - nameStart));
+            }
+            return names;
+        }
+
+        /**
+         * @brief Rejects any entity reference that is neither predefined, nor a character
+         *        reference, nor declared by the document's own DOCTYPE internal subset.
+         *
+         * Regions where `&` is ordinary data are skipped: comments, CDATA sections, processing
+         * instructions, and the DOCTYPE internal subset itself, whose ENTITY declarations
+         * legitimately contain references.
+         *
+         * A bare `&` that begins no complete reference is left alone. That is a different
+         * well-formedness rule and a different finding; widening this check to cover it would
+         * reject input on a premise this ticket did not establish.
+         *
+         * @note A **declared** entity is accepted and, as before, is **not expanded** -- it
+         *       stays inert literal text. That parity gap is pre-existing, is deliberately not
+         *       touched here, and is what keeps this port free of the billion-laughs exposure
+         *       (`XmlContractPinTests.InternalEntitiesAreNeverExpanded_NoBillionLaughsExposure`).
+         *       This ticket is about the **undeclared** case, where .NET throws and this port
+         *       silently rewrote the document's own text.
+         */
+        void ThrowIfUndeclaredEntityReference(const std::string& xml) {
+            static const char* const predefined[] = {"amp", "lt", "gt", "quot", "apos"};
+            const std::vector<std::string> declaredNames = DeclaredEntityNames(xml);
+
+            for (std::size_t i = 0; i < xml.size();) {
+                if (xml.compare(i, 4, "<!--") == 0) {
+                    const std::size_t end = xml.find("-->", i + 4);
+                    i = (end == std::string::npos) ? xml.size() : end + 3;
+                    continue;
+                }
+                if (xml.compare(i, 9, "<![CDATA[") == 0) {
+                    const std::size_t end = xml.find("]]>", i + 9);
+                    i = (end == std::string::npos) ? xml.size() : end + 3;
+                    continue;
+                }
+                if (xml.compare(i, 2, "<?") == 0) {
+                    const std::size_t end = xml.find("?>", i + 2);
+                    i = (end == std::string::npos) ? xml.size() : end + 2;
+                    continue;
+                }
+                if (xml.compare(i, 9, "<!DOCTYPE") == 0) {
+                    const std::size_t open  = xml.find('[', i);
+                    const std::size_t close = xml.find('>', i);
+                    if (open != std::string::npos && (close == std::string::npos || open < close)) {
+                        const std::size_t end = xml.find(']', open);
+                        i = (end == std::string::npos) ? xml.size() : end + 1;
+                    } else {
+                        i = (close == std::string::npos) ? xml.size() : close + 1;
+                    }
+                    continue;
+                }
+
+                if (xml[i] != '&') { ++i; continue; }
+
+                std::size_t j = i + 1;
+                if (j < xml.size() && xml[j] == '#') { ++i; continue; }   // character reference
+                if (j >= xml.size() || !IsEntityNameStart(static_cast<unsigned char>(xml[j]))) {
+                    ++i;   // a bare '&' -- see the doc-comment
+                    continue;
+                }
+                while (j < xml.size() && IsEntityNameChar(static_cast<unsigned char>(xml[j]))) ++j;
+                if (j >= xml.size() || xml[j] != ';') { ++i; continue; }   // incomplete
+
+                const std::string name = xml.substr(i + 1, j - i - 1);
+                bool known = false;
+                for (const char* candidate : predefined) {
+                    if (name == candidate) { known = true; break; }
+                }
+                for (const std::string& candidate : declaredNames) {
+                    if (known) break;
+                    if (name == candidate) known = true;
+                }
+                if (!known) throw XmlException("Reference to undeclared entity '" + name + "'.");
+                i = j + 1;
+            }
+        }
+
+        // ===================================================================================
+        // Ticket #2083 -- an undeclared namespace prefix.
+        //
+        // Measured by #2073's public-input sweep: LoadXml("<p:r/>") was ACCEPTED with no
+        // namespace resolution and round-tripped unchanged, so a document naming a namespace it
+        // never declares looked well-formed. .NET throws
+        // XmlException("'{0}' is an undeclared prefix.") (XmlTextReaderImpl.cs:7787,
+        // Strings.resx Xml_UnknownNs).
+        //
+        // Unlike #2082 this CAN be checked on the parsed tree, because a prefix survives parsing
+        // intact.
+        // ===================================================================================
+
+        /** @brief The prefix of a qualified name, or "" when it has none. */
+        std::string PrefixOf(const char* qualifiedName) {
+            if (!qualifiedName) return "";
+            const std::string name(qualifiedName);
+            const std::size_t colon = name.find(':');
+            return colon == std::string::npos ? std::string() : name.substr(0, colon);
+        }
+
+        void ThrowIfUndeclaredPrefix(const tinyxml2::XMLElement* element,
+                                     std::vector<std::string> inScope) {
+            if (!element) return;
+
+            // Declarations on THIS element are in scope for it, including for its own name --
+            // XML Names 1.0 3: the scope of a declaration includes the start-tag it appears on.
+            for (const tinyxml2::XMLAttribute* a = element->FirstAttribute(); a; a = a->Next()) {
+                const std::string attributeName(a->Name() ? a->Name() : "");
+                if (attributeName.rfind("xmlns:", 0) == 0 && attributeName.size() > 6) {
+                    inScope.push_back(attributeName.substr(6));
+                }
+            }
+
+            const auto declared = [&inScope](const std::string& prefix) {
+                // "xml" is bound by the specification itself and is never declared; "xmlns" is
+                // reserved and never appears as a name's prefix outside a declaration.
+                if (prefix.empty() || prefix == "xml" || prefix == "xmlns") return true;
+                for (const std::string& candidate : inScope) {
+                    if (candidate == prefix) return true;
+                }
+                return false;
+            };
+
+            const std::string elementPrefix = PrefixOf(element->Name());
+            if (!declared(elementPrefix)) {
+                throw XmlException("'" + elementPrefix + "' is an undeclared prefix.");
+            }
+            for (const tinyxml2::XMLAttribute* a = element->FirstAttribute(); a; a = a->Next()) {
+                const std::string attributeName(a->Name() ? a->Name() : "");
+                if (attributeName.rfind("xmlns:", 0) == 0 || attributeName == "xmlns") continue;
+                const std::string attributePrefix = PrefixOf(a->Name());
+                if (!declared(attributePrefix)) {
+                    throw XmlException("'" + attributePrefix + "' is an undeclared prefix.");
+                }
+            }
+
+            for (const tinyxml2::XMLElement* child = element->FirstChildElement(); child;
+                 child = child->NextSiblingElement()) {
+                ThrowIfUndeclaredPrefix(child, inScope);
+            }
+        }
+
         // VersionNum ::= '1.' [0-9]+  (XmlDeclaration.cs IsValidXmlVersion)
         bool IsValidXmlVersion(const std::string& ver) {
             if (ver.size() < 3 || ver[0] != '1' || ver[1] != '.') return false;
@@ -349,6 +551,10 @@ namespace System::Xml {
         if (doc_.LoadFile(filename.c_str()) != tinyxml2::XML_SUCCESS)
             throw XmlException("XmlDocument::Load: failed to load '" + filename + "': " +
                                (doc_.ErrorStr() ? doc_.ErrorStr() : "unknown error"));
+        // #2083. The entity check (#2082) cannot run here: it needs the RAW text, and this door
+        // hands the file straight to tinyxml2 without ever holding it. That asymmetry between
+        // the two doors is recorded rather than hidden -- see ticket #2361.
+        ThrowIfUndeclaredPrefix(doc_.RootElement(), {});
         // tinyxml2::XMLDocument::LoadFile() clears and frees every previously-allocated node
         // (including detachedHolder_ from the constructor, or a prior Load/LoadXml call) before
         // parsing; recreate it now, or IsDetached()/getParentNodeProperty() etc. would compare
@@ -357,10 +563,14 @@ namespace System::Xml {
     }
 
     void XmlDocument::LoadXml(const std::string& xml) {
+        // #2082: BEFORE the parse, because tinyxml2 decodes the predefined entities and the
+        // distinction is gone afterwards.
+        ThrowIfUndeclaredEntityReference(xml);
         nodeCache_.clear();
         if (doc_.Parse(xml.c_str()) != tinyxml2::XML_SUCCESS)
             throw XmlException(std::string("XmlDocument::LoadXml: parse error: ") +
                                (doc_.ErrorStr() ? doc_.ErrorStr() : "unknown error"));
+        ThrowIfUndeclaredPrefix(doc_.RootElement(), {});   // #2083
         // See the comment in Load() above: Parse() also clears and frees prior nodes.
         detachedHolder_ = doc_.NewElement("#detached-holder");
     }