Skip to content

Commit

Permalink
Fix XML serializer errata: xmlns="" serialization should be allowed
Browse files Browse the repository at this point in the history
The spec doesn't want to serialize xmlns:foo="", but the description of
the step that checks this does not take into account that xmlns="" must
be allowed. This patch corrects this errata.

Closes GH-15894.
  • Loading branch information
nielsdos committed Sep 15, 2024
1 parent 5121aca commit ed54d6d
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 8 deletions.
4 changes: 4 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ PHP NEWS
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
?? ??? ????, PHP 8.4.0RC1

- DOM:
. Fix XML serializer errata: xmlns="" serialization should be allowed.
(nielsdos)

- MBString:
. Fixed bug GH-15824 (mb_detect_encoding(): Argument $encodings contains
invalid encoding "UTF8"). (Yuya Hamada)
Expand Down
25 changes: 25 additions & 0 deletions ext/dom/tests/modern/xml/serialize_empty_xmlns.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
--TEST--
XML serializer spec errata: xmlns="" serialization should be allowed
--EXTENSIONS--
dom
--FILE--
<?php

// Should be allowed
$dom = Dom\XMLDocument::createFromString('<root><x xmlns=""/></root>');
var_dump($dom->documentElement->innerHTML);

// Should not be allowed
$dom = Dom\XMLDocument::createFromString('<root><x/></root>');
$x = $dom->documentElement->firstChild;
$x->setAttributeNS('http://www.w3.org/2000/xmlns/', 'xmlns:a', '');
try {
var_dump($dom->documentElement->innerHTML);
} catch (DOMException $e) {
echo $e->getMessage(), "\n";
}

?>
--EXPECT--
string(13) "<x xmlns=""/>"
The resulting XML serialization is not well-formed
18 changes: 10 additions & 8 deletions ext/dom/xml_serializer.c
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,7 @@ static int dom_xml_serialize_attribute_node_value(xmlOutputBufferPtr out, xmlAtt
/* These steps are from the attribute serialization algorithm's well-formed checks.
* Note that this does not return a boolean but an int to be compatible with the TRY/TRY_CLEANUP interface
* that we do for compatibility with libxml's interfaces. */
static zend_always_inline int dom_xml_check_xmlns_attribute_requirements(const xmlAttr *attr)
static zend_always_inline int dom_xml_check_xmlns_attribute_requirements(const xmlAttr *attr, const xmlChar *candidate_prefix)
{
const xmlChar *attr_value = dom_get_attribute_value(attr);

Expand All @@ -609,8 +609,9 @@ static zend_always_inline int dom_xml_check_xmlns_attribute_requirements(const x
return -1;
}

/* 3.5.2.3. If the require well-formed flag is set and the value of attr's value attribute is the empty string */
if (*attr_value == '\0') {
/* 3.5.2.3. If the require well-formed flag is set and the value of attr's value attribute is the empty string.
* Errata: an "xmlns" attribute is allowed but not one with a prefix, so the idea in the spec is right but the description isn't. */
if (*attr_value == '\0' && candidate_prefix != NULL) {
return -1;
}

Expand Down Expand Up @@ -790,15 +791,16 @@ static int dom_xml_serialize_attributes(
}
}

if (require_well_formed) {
/* 3.5.2.2 and 3.5.2.3 are done by this call. */
TRY_OR_CLEANUP(dom_xml_check_xmlns_attribute_requirements(attr));
}

/* 3.5.2.4. the attr's prefix matches the string "xmlns", then let candidate prefix be the string "xmlns". */
if (attr->ns->prefix != NULL && strcmp((const char *) attr->ns->prefix, "xmlns") == 0) {
candidate_prefix = BAD_CAST "xmlns";
}

/* Errata: step 3.5.2.3 can only really be checked if we already know the candidate prefix. */
if (require_well_formed) {
/* 3.5.2.2 and 3.5.2.3 are done by this call. */
TRY_OR_CLEANUP(dom_xml_check_xmlns_attribute_requirements(attr, candidate_prefix));
}
}
/* 3.5.3. Otherwise, the attribute namespace in not the XMLNS namespace. Run these steps: */
else if (candidate_prefix == NULL) { /* https://github.com/w3c/DOM-Parsing/issues/29 */
Expand Down

0 comments on commit ed54d6d

Please sign in to comment.