From 46b79024c1eb7ea597802c63ef6aea000a9e6d9c Mon Sep 17 00:00:00 2001 From: ndossche <7771979+ndossche@users.noreply.github.com> Date: Sat, 7 Mar 2026 11:05:49 +0100 Subject: [PATCH 1/3] Fix GH-21357: XSLTProcessor works with DOMDocument, but fails with Dom\XMLDocument Registering namespace after the parsing is too late because parsing can fail due to attributes referencing namespaces. So we have to register fake namespaces before the parsing. However, the clone operation reconciles namespaces in the wrong way, so we have to clone via an object. --- .../auto_registration_namespaces_new_dom.phpt | 2 +- ext/xsl/tests/gh21357.phpt | 18 +++ ext/xsl/xsltprocessor.c | 120 +++++++----------- 3 files changed, 64 insertions(+), 76 deletions(-) create mode 100644 ext/xsl/tests/gh21357.phpt diff --git a/ext/xsl/tests/auto_registration_namespaces_new_dom.phpt b/ext/xsl/tests/auto_registration_namespaces_new_dom.phpt index f96b7d1918419..f68f18413bdea 100644 --- a/ext/xsl/tests/auto_registration_namespaces_new_dom.phpt +++ b/ext/xsl/tests/auto_registration_namespaces_new_dom.phpt @@ -24,7 +24,7 @@ $sheet = Dom\XMLDocument::createFromString(<<documentElement->append($sheet->createElementNS('urn:test', 'test:dummy')); +$sheet->documentElement->setAttributeNS('http://www.w3.org/2000/xmlns/', 'xmlns:test', 'urn:test'); $input = Dom\XMLDocument::createFromString(<< diff --git a/ext/xsl/tests/gh21357.phpt b/ext/xsl/tests/gh21357.phpt new file mode 100644 index 0000000000000..686f21e19d842 --- /dev/null +++ b/ext/xsl/tests/gh21357.phpt @@ -0,0 +1,18 @@ +--TEST-- +GH-21357 (XSLTProcessor works with \DOMDocument, but fails with \Dom\XMLDocument) +--EXTENSIONS-- +dom +xsl +--FILE-- + + + +XML; + +$dom = Dom\XMLDocument::createFromString($xml); +var_dump(new XSLTProcessor()->importStylesheet($dom)); +?> +--EXPECT-- +bool(true) diff --git a/ext/xsl/xsltprocessor.c b/ext/xsl/xsltprocessor.c index eaefdb2f8ef7c..286d56b3d23c8 100644 --- a/ext/xsl/xsltprocessor.c +++ b/ext/xsl/xsltprocessor.c @@ -123,65 +123,38 @@ static void xsl_ext_function_trampoline(xmlXPathParserContextPtr ctxt, int nargs } } -static void xsl_add_ns_to_map(xmlHashTablePtr table, xsltStylesheetPtr sheet, const xmlNode *cur, const xmlChar *prefix, const xmlChar *uri) +static void xsl_add_ns_def(xmlNodePtr node) { - const xmlChar *existing_url = xmlHashLookup(table, prefix); - if (existing_url != NULL && !xmlStrEqual(existing_url, uri)) { - xsltTransformError(NULL, sheet, (xmlNodePtr) cur, "Namespaces prefix %s used for multiple namespaces\n", prefix); - sheet->warnings++; - } else if (existing_url == NULL) { - xmlHashUpdateEntry(table, prefix, (void *) uri, NULL); - } -} - -/* Adds all namespace declaration (not using nsDef) into a hash map that maps prefix to uri. Warns on conflicting declarations. */ -static void xsl_build_ns_map(xmlHashTablePtr table, xsltStylesheetPtr sheet, php_dom_libxml_ns_mapper *ns_mapper, const xmlDoc *doc) -{ - const xmlNode *cur = xmlDocGetRootElement(doc); + if (node->type == XML_ELEMENT_NODE) { + for (xmlAttrPtr attr = node->properties; attr; attr = attr->next) { + if (php_dom_ns_is_fast((const xmlNode *) attr, php_dom_ns_is_xmlns_magic_token)) { + xmlNsPtr ns = xmlMalloc(sizeof(*ns)); + if (!ns) { + return; + } - while (cur != NULL) { - if (cur->type == XML_ELEMENT_NODE) { - if (cur->ns != NULL && cur->ns->prefix != NULL) { - xsl_add_ns_to_map(table, sheet, cur, cur->ns->prefix, cur->ns->href); - } + bool should_free; + xmlChar *attr_value = php_libxml_attr_value(attr, &should_free); - for (const xmlAttr *attr = cur->properties; attr != NULL; attr = attr->next) { - if (attr->ns != NULL && attr->ns->prefix != NULL && php_dom_ns_is_fast_ex(attr->ns, php_dom_ns_is_xmlns_magic_token) - && attr->children != NULL && attr->children->content != NULL) { - /* This attribute declares a namespace, get the relevant instance. - * The declared namespace is not the same as the namespace of this attribute (which is xmlns). */ - const xmlChar *prefix = attr->name; - xmlNsPtr ns = php_dom_libxml_ns_mapper_get_ns_raw_strings_nullsafe(ns_mapper, (const char *) prefix, (const char *) attr->children->content); - xsl_add_ns_to_map(table, sheet, cur, prefix, ns->href); - } + memset(ns, 0, sizeof(*ns)); + ns->type = XML_LOCAL_NAMESPACE; + ns->href = should_free ? attr_value : xmlStrdup(attr_value); + ns->prefix = attr->ns->prefix ? xmlStrdup(attr->name) : NULL; + ns->next = node->nsDef; + node->nsDef = ns; } } - - cur = php_dom_next_in_tree_order(cur, (const xmlNode *) doc); } } -/* Apply namespace corrections for new DOM */ -typedef enum { - XSL_NS_HASH_CORRECTION_NONE = 0, - XSL_NS_HASH_CORRECTION_APPLIED = 1, - XSL_NS_HASH_CORRECTION_FAILED = 2 -} xsl_ns_hash_correction_status; - -static zend_always_inline xsl_ns_hash_correction_status xsl_apply_ns_hash_corrections(xsltStylesheetPtr sheetp, xmlNodePtr nodep, xmlDocPtr doc) +static void xsl_add_ns_defs(xmlDocPtr doc) { - if (sheetp->nsHash == NULL) { - dom_object *node_intern = php_dom_object_get_data(nodep); - if (node_intern != NULL && php_dom_follow_spec_intern(node_intern)) { - sheetp->nsHash = xmlHashCreate(10); - if (UNEXPECTED(!sheetp->nsHash)) { - return XSL_NS_HASH_CORRECTION_FAILED; - } - xsl_build_ns_map(sheetp->nsHash, sheetp, php_dom_get_ns_mapper(node_intern), doc); - return XSL_NS_HASH_CORRECTION_APPLIED; - } + xmlNodePtr base = (xmlNodePtr) doc; + xmlNodePtr node = base->children; + while (node != NULL) { + xsl_add_ns_def(node); + node = php_dom_next_in_tree_order(node, base); } - return XSL_NS_HASH_CORRECTION_NONE; } /* {{{ URL: http://www.w3.org/TR/2003/WD-DOM-Level-3-Core-20030226/DOM3-Core.html# @@ -190,11 +163,11 @@ static zend_always_inline xsl_ns_hash_correction_status xsl_apply_ns_hash_correc PHP_METHOD(XSLTProcessor, importStylesheet) { zval *id, *docp = NULL; - xmlDoc *doc = NULL, *newdoc = NULL; + xmlDoc *newdoc = NULL; xsltStylesheetPtr sheetp; bool clone_docu = false; xmlNode *nodep = NULL; - zval *cloneDocu, rv; + zval *cloneDocu, rv, clone_zv; zend_string *member; id = ZEND_THIS; @@ -202,51 +175,48 @@ PHP_METHOD(XSLTProcessor, importStylesheet) RETURN_THROWS(); } - nodep = php_libxml_import_node(docp); + /* libxslt uses _private, so we must copy the imported + * stylesheet document otherwise the node proxies will be a mess. + * We will clone the object and detach the libxml internals later. */ + zend_object *clone = Z_OBJ_HANDLER_P(docp, clone_obj)(Z_OBJ_P(docp)); + if (!clone) { + RETURN_THROWS(); + } + + ZVAL_OBJ(&clone_zv, clone); + nodep = php_libxml_import_node(&clone_zv); if (nodep) { - doc = nodep->doc; + newdoc = nodep->doc; } - if (doc == NULL) { + if (newdoc == NULL) { + OBJ_RELEASE(clone); zend_argument_type_error(1, "must be a valid XML node"); RETURN_THROWS(); } - /* libxslt uses _private, so we must copy the imported - stylesheet document otherwise the node proxies will be a mess */ - newdoc = xmlCopyDoc(doc, 1); - xmlNodeSetBase((xmlNodePtr) newdoc, (xmlChar *)doc->URL); PHP_LIBXML_SANITIZE_GLOBALS(parse); ZEND_DIAGNOSTIC_IGNORED_START("-Wdeprecated-declarations") xmlSubstituteEntitiesDefault(1); xmlLoadExtDtdDefaultValue = XML_DETECT_IDS | XML_COMPLETE_ATTRS; ZEND_DIAGNOSTIC_IGNORED_END + xsl_add_ns_defs(newdoc); + sheetp = xsltParseStylesheetDoc(newdoc); PHP_LIBXML_RESTORE_GLOBALS(parse); if (!sheetp) { - xmlFreeDoc(newdoc); + OBJ_RELEASE(clone); RETURN_FALSE; } - xsl_object *intern = Z_XSL_P(id); + /* Detach object */ + php_libxml_node_object *clone_lxml_obj = (php_libxml_node_object *) ((char *) Z_OBJ(clone_zv) - Z_OBJ_HT(clone_zv)->offset); + clone_lxml_obj->document->ptr = NULL; + OBJ_RELEASE(clone); - xsl_ns_hash_correction_status status = xsl_apply_ns_hash_corrections(sheetp, nodep, doc); - if (UNEXPECTED(status == XSL_NS_HASH_CORRECTION_FAILED)) { - xsltFreeStylesheet(sheetp); - xmlFreeDoc(newdoc); - RETURN_FALSE; - } else if (status == XSL_NS_HASH_CORRECTION_APPLIED) { - /* The namespace mappings need to be kept alive. - * This is stored in the ref obj outside of libxml2, but that means that the sheet won't keep it alive - * unlike with namespaces from old DOM. */ - if (intern->sheet_ref_obj) { - php_libxml_decrement_doc_ref_directly(intern->sheet_ref_obj); - } - intern->sheet_ref_obj = Z_LIBXML_NODE_P(docp)->document; - intern->sheet_ref_obj->refcount++; - } + xsl_object *intern = Z_XSL_P(id); member = ZSTR_INIT_LITERAL("cloneDocument", 0); cloneDocu = zend_std_read_property(Z_OBJ_P(id), member, BP_VAR_R, NULL, &rv); From bc25ba66549e9f521f1f54edc3d0599c02769058 Mon Sep 17 00:00:00 2001 From: ndossche <7771979+ndossche@users.noreply.github.com> Date: Sun, 8 Mar 2026 00:03:17 +0100 Subject: [PATCH 2/3] fixup! fix another issue --- .../tests/{gh21357.phpt => gh21357_1.phpt} | 2 ++ ext/xsl/tests/gh21357_2.phpt | 35 +++++++++++++++++++ ext/xsl/xsltprocessor.c | 14 ++++++-- 3 files changed, 48 insertions(+), 3 deletions(-) rename ext/xsl/tests/{gh21357.phpt => gh21357_1.phpt} (95%) create mode 100644 ext/xsl/tests/gh21357_2.phpt diff --git a/ext/xsl/tests/gh21357.phpt b/ext/xsl/tests/gh21357_1.phpt similarity index 95% rename from ext/xsl/tests/gh21357.phpt rename to ext/xsl/tests/gh21357_1.phpt index 686f21e19d842..17e5de98aa1ea 100644 --- a/ext/xsl/tests/gh21357.phpt +++ b/ext/xsl/tests/gh21357_1.phpt @@ -3,6 +3,8 @@ GH-21357 (XSLTProcessor works with \DOMDocument, but fails with \Dom\XMLDocument --EXTENSIONS-- dom xsl +--CREDITS-- +jacekkow --FILE-- + + + + + + + + + + + +'; +$dom = Dom\XMLDocument::createFromString($xsl); +$xsl = new XSLTProcessor(); +$xsl->importStylesheet($dom); +var_dump($xsl->transformToXml(\Dom\XMLDocument::createFromString(''))); +?> +--EXPECT-- +string(138) " + +" diff --git a/ext/xsl/xsltprocessor.c b/ext/xsl/xsltprocessor.c index 286d56b3d23c8..6a02ef4ff5d63 100644 --- a/ext/xsl/xsltprocessor.c +++ b/ext/xsl/xsltprocessor.c @@ -211,13 +211,21 @@ PHP_METHOD(XSLTProcessor, importStylesheet) RETURN_FALSE; } + xsl_object *intern = Z_XSL_P(id); + /* Detach object */ - php_libxml_node_object *clone_lxml_obj = (php_libxml_node_object *) ((char *) Z_OBJ(clone_zv) - Z_OBJ_HT(clone_zv)->offset); + php_libxml_node_object *clone_lxml_obj = Z_LIBXML_NODE_P(&clone_zv); clone_lxml_obj->document->ptr = NULL; + /* The namespace mappings need to be kept alive. + * This is stored in the ref obj outside of libxml2, but that means that the sheet won't keep it alive + * unlike with namespaces from old DOM. */ + if (intern->sheet_ref_obj) { + php_libxml_decrement_doc_ref_directly(intern->sheet_ref_obj); + } + intern->sheet_ref_obj = clone_lxml_obj->document; + intern->sheet_ref_obj->refcount++; OBJ_RELEASE(clone); - xsl_object *intern = Z_XSL_P(id); - member = ZSTR_INIT_LITERAL("cloneDocument", 0); cloneDocu = zend_std_read_property(Z_OBJ_P(id), member, BP_VAR_R, NULL, &rv); clone_docu = zend_is_true(cloneDocu); From 9ab00afe206e0c5d28afbbbf2e885971b46f7eae Mon Sep 17 00:00:00 2001 From: ndossche <7771979+ndossche@users.noreply.github.com> Date: Sun, 8 Mar 2026 00:04:49 +0100 Subject: [PATCH 3/3] fixup! skip work for old dom --- ext/xsl/xsltprocessor.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/ext/xsl/xsltprocessor.c b/ext/xsl/xsltprocessor.c index 6a02ef4ff5d63..d066e2706510f 100644 --- a/ext/xsl/xsltprocessor.c +++ b/ext/xsl/xsltprocessor.c @@ -195,13 +195,17 @@ PHP_METHOD(XSLTProcessor, importStylesheet) RETURN_THROWS(); } + php_libxml_node_object *clone_lxml_obj = Z_LIBXML_NODE_P(&clone_zv); + PHP_LIBXML_SANITIZE_GLOBALS(parse); ZEND_DIAGNOSTIC_IGNORED_START("-Wdeprecated-declarations") xmlSubstituteEntitiesDefault(1); xmlLoadExtDtdDefaultValue = XML_DETECT_IDS | XML_COMPLETE_ATTRS; ZEND_DIAGNOSTIC_IGNORED_END - xsl_add_ns_defs(newdoc); + if (clone_lxml_obj->document->class_type == PHP_LIBXML_CLASS_MODERN) { + xsl_add_ns_defs(newdoc); + } sheetp = xsltParseStylesheetDoc(newdoc); PHP_LIBXML_RESTORE_GLOBALS(parse); @@ -214,7 +218,6 @@ PHP_METHOD(XSLTProcessor, importStylesheet) xsl_object *intern = Z_XSL_P(id); /* Detach object */ - php_libxml_node_object *clone_lxml_obj = Z_LIBXML_NODE_P(&clone_zv); clone_lxml_obj->document->ptr = NULL; /* The namespace mappings need to be kept alive. * This is stored in the ref obj outside of libxml2, but that means that the sheet won't keep it alive