From e309fd84610802c67413fb48284e85495034e7a9 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sat, 10 Jun 2023 22:48:16 +0200 Subject: [PATCH 1/3] Fix lifetime issue with getAttributeNodeNS() It's the same issue that I fixed previously in GH-11402, but in a different place. Closes GH-11422. --- NEWS | 1 + ext/dom/element.c | 20 ++++--------------- ...ifetime_parentNode_getAttributeNodeNS.phpt | 20 +++++++++++++++++++ 3 files changed, 25 insertions(+), 16 deletions(-) create mode 100644 ext/dom/tests/bug_lifetime_parentNode_getAttributeNodeNS.phpt diff --git a/NEWS b/NEWS index 139c6963744..776745d073a 100644 --- a/NEWS +++ b/NEWS @@ -30,6 +30,7 @@ PHP NEWS . Fixed bug #70359 (print_r() on DOMAttr causes Segfault in php_libxml_node_free_list()). (nielsdos) . Fixed bug #78577 (Crash in DOMNameSpace debug info handlers). (nielsdos) + . Fix lifetime issue with getAttributeNodeNS(). (nielsdos) - Opcache: . Fix allocation loop in zend_shared_alloc_startup(). (nielsdos) diff --git a/ext/dom/element.c b/ext/dom/element.c index f84caa629cc..44c576a0736 100644 --- a/ext/dom/element.c +++ b/ext/dom/element.c @@ -787,7 +787,7 @@ Since: DOM Level 2 PHP_METHOD(DOMElement, getAttributeNodeNS) { zval *id; - xmlNodePtr elemp, fakeAttrp; + xmlNodePtr elemp; xmlAttrPtr attrp; dom_object *intern; size_t uri_len, name_len; @@ -808,21 +808,9 @@ PHP_METHOD(DOMElement, getAttributeNodeNS) xmlNsPtr nsptr; nsptr = dom_get_nsdecl(elemp, (xmlChar *)name); if (nsptr != NULL) { - xmlNsPtr curns; - curns = xmlNewNs(NULL, nsptr->href, NULL); - if (nsptr->prefix) { - curns->prefix = xmlStrdup((xmlChar *) nsptr->prefix); - } - if (nsptr->prefix) { - fakeAttrp = xmlNewDocNode(elemp->doc, NULL, (xmlChar *) nsptr->prefix, nsptr->href); - } else { - fakeAttrp = xmlNewDocNode(elemp->doc, NULL, (xmlChar *)"xmlns", nsptr->href); - } - fakeAttrp->type = XML_NAMESPACE_DECL; - fakeAttrp->parent = elemp; - fakeAttrp->ns = curns; - - DOM_RET_OBJ(fakeAttrp, &ret, intern); + /* Keep parent alive, because we're a fake child. */ + GC_ADDREF(&intern->std); + (void) php_dom_create_fake_namespace_decl(elemp, nsptr, return_value, intern); } else { RETURN_NULL(); } diff --git a/ext/dom/tests/bug_lifetime_parentNode_getAttributeNodeNS.phpt b/ext/dom/tests/bug_lifetime_parentNode_getAttributeNodeNS.phpt new file mode 100644 index 00000000000..3c53e08d4db --- /dev/null +++ b/ext/dom/tests/bug_lifetime_parentNode_getAttributeNodeNS.phpt @@ -0,0 +1,20 @@ +--TEST-- +Lifetime issue with parentNode on getAttributeNodeNS() +--EXTENSIONS-- +dom +--FILE-- + + + +'; + +$xml=new DOMDocument(); +$xml->loadXML($xmlString); +$ns2 = $xml->documentElement->getAttributeNodeNS("http://www.w3.org/2000/xmlns/", "ns2"); +$ns2->parentNode->remove(); +var_dump($ns2->parentNode->localName); + +?> +--EXPECT-- +string(4) "root" From 10d94aca4c5ee7a101ed39bc395bcc1bb9d68507 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 11 Jun 2023 23:44:58 +0200 Subject: [PATCH 2/3] Fix "invalid state error" with cloned namespace declarations Closes GH-11429. --- NEWS | 1 + ext/dom/php_dom.c | 57 +++++++++++++++++++++------ ext/dom/tests/clone_nodes.phpt | 72 ++++++++++++++++++++++++++++++++++ 3 files changed, 117 insertions(+), 13 deletions(-) create mode 100644 ext/dom/tests/clone_nodes.phpt diff --git a/NEWS b/NEWS index 776745d073a..cad46534389 100644 --- a/NEWS +++ b/NEWS @@ -31,6 +31,7 @@ PHP NEWS php_libxml_node_free_list()). (nielsdos) . Fixed bug #78577 (Crash in DOMNameSpace debug info handlers). (nielsdos) . Fix lifetime issue with getAttributeNodeNS(). (nielsdos) + . Fix "invalid state error" with cloned namespace declarations. (nielsdos) - Opcache: . Fix allocation loop in zend_shared_alloc_startup(). (nielsdos) diff --git a/ext/dom/php_dom.c b/ext/dom/php_dom.c index 9e0bb1f3d1d..454dc54d8e2 100644 --- a/ext/dom/php_dom.c +++ b/ext/dom/php_dom.c @@ -89,6 +89,7 @@ static HashTable dom_xpath_prop_handlers; static zend_object *dom_objects_namespace_node_new(zend_class_entry *class_type); static void dom_object_namespace_node_free_storage(zend_object *object); +static xmlNodePtr php_dom_create_fake_namespace_decl_node_ptr(xmlNodePtr nodep, xmlNsPtr original); typedef int (*dom_read_t)(dom_object *obj, zval *retval); typedef int (*dom_write_t)(dom_object *obj, zval *newval); @@ -477,6 +478,19 @@ PHP_FUNCTION(dom_import_simplexml) static dom_object* dom_objects_set_class(zend_class_entry *class_type); +static void dom_update_refcount_after_clone(dom_object *original, xmlNodePtr original_node, dom_object *clone, xmlNodePtr cloned_node) +{ + /* If we cloned a document then we must create new doc proxy */ + if (cloned_node->doc == original_node->doc) { + clone->document = original->document; + } + php_libxml_increment_doc_ref((php_libxml_node_object *)clone, cloned_node->doc); + php_libxml_increment_node_ptr((php_libxml_node_object *)clone, cloned_node, (void *)clone); + if (original->document != clone->document) { + dom_copy_doc_props(original->document, clone->document); + } +} + static zend_object *dom_objects_store_clone_obj(zend_object *zobject) /* {{{ */ { dom_object *intern = php_dom_obj_from_obj(zobject); @@ -489,15 +503,7 @@ static zend_object *dom_objects_store_clone_obj(zend_object *zobject) /* {{{ */ if (node != NULL) { xmlNodePtr cloned_node = xmlDocCopyNode(node, node->doc, 1); if (cloned_node != NULL) { - /* If we cloned a document then we must create new doc proxy */ - if (cloned_node->doc == node->doc) { - clone->document = intern->document; - } - php_libxml_increment_doc_ref((php_libxml_node_object *)clone, cloned_node->doc); - php_libxml_increment_node_ptr((php_libxml_node_object *)clone, cloned_node, (void *)clone); - if (intern->document != clone->document) { - dom_copy_doc_props(intern->document, clone->document); - } + dom_update_refcount_after_clone(intern, node, clone, cloned_node); } } @@ -509,6 +515,26 @@ static zend_object *dom_objects_store_clone_obj(zend_object *zobject) /* {{{ */ } /* }}} */ +static zend_object *dom_object_namespace_node_clone_obj(zend_object *zobject) +{ + dom_object_namespace_node *intern = php_dom_namespace_node_obj_from_obj(zobject); + zend_object *clone = dom_objects_namespace_node_new(intern->dom.std.ce); + dom_object_namespace_node *clone_intern = php_dom_namespace_node_obj_from_obj(clone); + + xmlNodePtr original_node = dom_object_get_node(&intern->dom); + ZEND_ASSERT(original_node->type == XML_NAMESPACE_DECL); + xmlNodePtr cloned_node = php_dom_create_fake_namespace_decl_node_ptr(original_node->parent, original_node->ns); + + if (intern->parent_intern) { + clone_intern->parent_intern = intern->parent_intern; + GC_ADDREF(&clone_intern->parent_intern->std); + } + dom_update_refcount_after_clone(&intern->dom, original_node, &clone_intern->dom, cloned_node); + + zend_objects_clone_members(clone, &intern->dom.std); + return clone; +} + static void dom_copy_prop_handler(zval *zv) /* {{{ */ { dom_prop_handler *hnd = Z_PTR_P(zv); @@ -577,6 +603,7 @@ PHP_MINIT_FUNCTION(dom) memcpy(&dom_object_namespace_node_handlers, &dom_object_handlers, sizeof(zend_object_handlers)); dom_object_namespace_node_handlers.offset = XtOffsetOf(dom_object_namespace_node, dom.std); dom_object_namespace_node_handlers.free_obj = dom_object_namespace_node_free_storage; + dom_object_namespace_node_handlers.clone_obj = dom_object_namespace_node_clone_obj; zend_hash_init(&classes, 0, NULL, NULL, 1); @@ -1579,8 +1606,7 @@ xmlNsPtr dom_get_nsdecl(xmlNode *node, xmlChar *localName) { } /* }}} end dom_get_nsdecl */ -/* Note: Assumes the additional lifetime was already added in the caller. */ -xmlNodePtr php_dom_create_fake_namespace_decl(xmlNodePtr nodep, xmlNsPtr original, zval *return_value, dom_object *parent_intern) +static xmlNodePtr php_dom_create_fake_namespace_decl_node_ptr(xmlNodePtr nodep, xmlNsPtr original) { xmlNodePtr attrp; xmlNsPtr curns = xmlNewNs(NULL, original->href, NULL); @@ -1593,11 +1619,16 @@ xmlNodePtr php_dom_create_fake_namespace_decl(xmlNodePtr nodep, xmlNsPtr origina attrp->type = XML_NAMESPACE_DECL; attrp->parent = nodep; attrp->ns = curns; + return attrp; +} +/* Note: Assumes the additional lifetime was already added in the caller. */ +xmlNodePtr php_dom_create_fake_namespace_decl(xmlNodePtr nodep, xmlNsPtr original, zval *return_value, dom_object *parent_intern) +{ + xmlNodePtr attrp = php_dom_create_fake_namespace_decl_node_ptr(nodep, original); php_dom_create_object(attrp, return_value, parent_intern); /* This object must exist, because we just created an object for it via php_dom_create_object(). */ - dom_object *obj = ((php_libxml_node_ptr *)attrp->_private)->_private; - php_dom_namespace_node_obj_from_obj(&obj->std)->parent_intern = parent_intern; + php_dom_namespace_node_obj_from_obj(Z_OBJ_P(return_value))->parent_intern = parent_intern; return attrp; } diff --git a/ext/dom/tests/clone_nodes.phpt b/ext/dom/tests/clone_nodes.phpt new file mode 100644 index 00000000000..1841c702caf --- /dev/null +++ b/ext/dom/tests/clone_nodes.phpt @@ -0,0 +1,72 @@ +--TEST-- +Clone nodes +--EXTENSIONS-- +dom +--FILE-- +loadXML(''); + +$attr = $doc->documentElement->getAttributeNode('xmlns'); +var_dump($attr); + +$attrClone = clone $attr; +var_dump($attrClone->nodeValue); +var_dump($attrClone->parentNode->nodeName); + +unset($doc); +unset($attr); + +var_dump($attrClone->nodeValue); +var_dump($attrClone->parentNode->nodeName); + +echo "-- Clone DOMNode --\n"; + +$doc = new DOMDocument; +$doc->loadXML(''); + +$bar = $doc->documentElement->firstChild; +$barClone = clone $bar; +$bar->remove(); +unset($bar); + +var_dump($barClone->nodeName); + +$doc->firstElementChild->remove(); +unset($doc); + +var_dump($barClone->nodeName); +var_dump($barClone->parentNode); + +?> +--EXPECT-- +-- Clone DOMNameSpaceNode -- +object(DOMNameSpaceNode)#3 (8) { + ["nodeName"]=> + string(5) "xmlns" + ["nodeValue"]=> + string(19) "http://php.net/test" + ["nodeType"]=> + int(18) + ["prefix"]=> + string(0) "" + ["localName"]=> + string(5) "xmlns" + ["namespaceURI"]=> + string(19) "http://php.net/test" + ["ownerDocument"]=> + string(22) "(object value omitted)" + ["parentNode"]=> + string(22) "(object value omitted)" +} +string(19) "http://php.net/test" +string(3) "foo" +string(19) "http://php.net/test" +string(3) "foo" +-- Clone DOMNode -- +string(3) "bar" +string(3) "bar" +NULL From a8a3b99e00747f3a1198c526674c9dad513a203f Mon Sep 17 00:00:00 2001 From: nielsdos <7771979+nielsdos@users.noreply.github.com> Date: Mon, 12 Jun 2023 23:58:34 +0200 Subject: [PATCH 3/3] Fix GH-11433: Unable to set CURLOPT_ACCEPT_ENCODING to NULL Closes GH-11446. --- NEWS | 4 ++ ext/curl/interface.c | 2 +- .../curl_setopt_CURLOPT_ACCEPT_ENCODING.phpt | 38 +++++++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 ext/curl/tests/curl_setopt_CURLOPT_ACCEPT_ENCODING.phpt diff --git a/NEWS b/NEWS index cad46534389..6baaae22e86 100644 --- a/NEWS +++ b/NEWS @@ -9,6 +9,10 @@ PHP NEWS - Core: . Fixed build for the riscv64 architecture/GCC 12. (Daniil Gentili) +- Curl: + . Fixed bug GH-11433 (Unable to set CURLOPT_ACCEPT_ENCODING to NULL). + (nielsdos) + - DOM: . Fixed bugs GH-11288 and GH-11289 and GH-11290 and GH-9142 (DOMExceptions and segfaults with replaceWith). (nielsdos) diff --git a/ext/curl/interface.c b/ext/curl/interface.c index 025c876ad5b..807b27cb78c 100644 --- a/ext/curl/interface.c +++ b/ext/curl/interface.c @@ -2493,7 +2493,6 @@ static int _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue, bool i case CURLOPT_TLSAUTH_TYPE: case CURLOPT_TLSAUTH_PASSWORD: case CURLOPT_TLSAUTH_USERNAME: - case CURLOPT_ACCEPT_ENCODING: case CURLOPT_TRANSFER_ENCODING: case CURLOPT_DNS_SERVERS: case CURLOPT_MAIL_AUTH: @@ -2553,6 +2552,7 @@ static int _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue, bool i case CURLOPT_RANGE: case CURLOPT_FTP_ACCOUNT: case CURLOPT_RTSP_SESSION_ID: + case CURLOPT_ACCEPT_ENCODING: #if LIBCURL_VERSION_NUM >= 0x072100 /* Available since 7.33.0 */ case CURLOPT_DNS_INTERFACE: case CURLOPT_DNS_LOCAL_IP4: diff --git a/ext/curl/tests/curl_setopt_CURLOPT_ACCEPT_ENCODING.phpt b/ext/curl/tests/curl_setopt_CURLOPT_ACCEPT_ENCODING.phpt new file mode 100644 index 00000000000..c170308c2e9 --- /dev/null +++ b/ext/curl/tests/curl_setopt_CURLOPT_ACCEPT_ENCODING.phpt @@ -0,0 +1,38 @@ +--TEST-- +Test curl_setopt() with CURLOPT_ACCEPT_ENCODING +--EXTENSIONS-- +curl +--FILE-- + +--EXPECTF-- +GET /get.inc?test= HTTP/1.1 +Host: %s +Accept: */* +Accept-Encoding: gzip + +GET /get.inc?test= HTTP/1.1 +Host: %s +Accept: */*