From c7d3dc6fabc0b069e2891ba1f9f69dc42b8d2b8c Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Fri, 7 Mar 2025 17:56:17 +0100 Subject: [PATCH 1/2] Fix GH-17989: mb_output_handler crash with unset http_output_conv_mimetypes The INI option can be NULL or invalid, resulting in a NULL global. So we have to add a NULL check. Closes GH-17996. --- NEWS | 4 ++++ ext/mbstring/mbstring.c | 4 +++- ext/mbstring/tests/gh17989.phpt | 16 ++++++++++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 ext/mbstring/tests/gh17989.phpt diff --git a/NEWS b/NEWS index 46b0934b870..02b02bb84bd 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,10 @@ PHP NEWS . Fixed bug GH-17984 (calls with arguments as array with references). (David Carlier) +- Mbstring: + . Fixed bug GH-17989 (mb_output_handler crash with unset + http_output_conv_mimetypes). (nielsdos) + - Treewide: . Fixed bug GH-17736 (Assertion failure zend_reference_destroy()). (nielsdos) diff --git a/ext/mbstring/mbstring.c b/ext/mbstring/mbstring.c index dbf012174c4..dec565707fa 100644 --- a/ext/mbstring/mbstring.c +++ b/ext/mbstring/mbstring.c @@ -1567,7 +1567,9 @@ PHP_FUNCTION(mb_output_handler) char *mimetype = NULL; /* Analyze mime type */ - if (SG(sapi_headers).mimetype && _php_mb_match_regex(MBSTRG(http_output_conv_mimetypes), SG(sapi_headers).mimetype, strlen(SG(sapi_headers).mimetype))) { + if (SG(sapi_headers).mimetype + && MBSTRG(http_output_conv_mimetypes) + && _php_mb_match_regex(MBSTRG(http_output_conv_mimetypes), SG(sapi_headers).mimetype, strlen(SG(sapi_headers).mimetype))) { char *s; if ((s = strchr(SG(sapi_headers).mimetype, ';')) == NULL) { mimetype = estrdup(SG(sapi_headers).mimetype); diff --git a/ext/mbstring/tests/gh17989.phpt b/ext/mbstring/tests/gh17989.phpt new file mode 100644 index 00000000000..40efd5866c1 --- /dev/null +++ b/ext/mbstring/tests/gh17989.phpt @@ -0,0 +1,16 @@ +--TEST-- +GH-17989 (mb_output_handler crash with unset http_output_conv_mimetypes) +--EXTENSIONS-- +mbstring +--INI-- +mbstring.http_output_conv_mimetypes= +--FILE-- + +--EXPECT-- +set mime type via this echo +hi From 9be9f70caab7e58141d1be145d23cf39d676be0b Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sat, 8 Mar 2025 19:38:42 +0100 Subject: [PATCH 2/2] Fix weird unpack behaviour in DOM Engine pitfall: the iter index is only updated by foreach opcodes, so the existing code that used it as an index for the nodes w.r.t. the start did not work properly. Fix it by using our own counter. Closes GH-18004. --- NEWS | 3 ++ ext/dom/dom_iterators.c | 12 ++++---- ext/dom/php_dom.h | 3 ++ ext/dom/tests/unpack_foreach_behaviour.phpt | 31 +++++++++++++++++++++ 4 files changed, 44 insertions(+), 5 deletions(-) create mode 100644 ext/dom/tests/unpack_foreach_behaviour.phpt diff --git a/NEWS b/NEWS index 02b02bb84bd..c3156e8ce4e 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,9 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? ????, PHP 8.3.19 +- DOM: + . Fix weird unpack behaviour in DOM. (nielsdos) + - GD: . Fixed bug GH-17984 (calls with arguments as array with references). (David Carlier) diff --git a/ext/dom/dom_iterators.c b/ext/dom/dom_iterators.c index ff63342595a..c8256de239d 100644 --- a/ext/dom/dom_iterators.c +++ b/ext/dom/dom_iterators.c @@ -158,7 +158,7 @@ static void php_dom_iterator_current_key(zend_object_iterator *iter, zval *key) zval *object = &iterator->intern.data; if (instanceof_function(Z_OBJCE_P(object), dom_nodelist_class_entry)) { - ZVAL_LONG(key, iter->index); + ZVAL_LONG(key, iterator->index); } else { dom_object *intern = Z_DOMOBJ_P(&iterator->curobj); @@ -189,6 +189,8 @@ static void php_dom_iterator_move_forward(zend_object_iterator *iter) /* {{{ */ return; } + iterator->index++; + intern = Z_DOMOBJ_P(&iterator->curobj); object = &iterator->intern.data; nnmap = Z_DOMOBJ_P(object); @@ -227,18 +229,18 @@ static void php_dom_iterator_move_forward(zend_object_iterator *iter) /* {{{ */ curnode = basenode->children; } } else { - previndex = iter->index - 1; + previndex = iterator->index - 1; curnode = (xmlNodePtr)((php_libxml_node_ptr *)intern->ptr)->node; } curnode = dom_get_elements_by_tag_name_ns_raw( - basenode, curnode, (char *) objmap->ns, (char *) objmap->local, &previndex, iter->index); + basenode, curnode, (char *) objmap->ns, (char *) objmap->local, &previndex, iterator->index); } } } else { if (objmap->nodetype == XML_ENTITY_NODE) { - curnode = php_dom_libxml_hash_iter(objmap->ht, iter->index); + curnode = php_dom_libxml_hash_iter(objmap->ht, iterator->index); } else { - curnode = php_dom_libxml_notation_iter(objmap->ht, iter->index); + curnode = php_dom_libxml_notation_iter(objmap->ht, iterator->index); } } } diff --git a/ext/dom/php_dom.h b/ext/dom/php_dom.h index 2bccb2d5692..120af426765 100644 --- a/ext/dom/php_dom.h +++ b/ext/dom/php_dom.h @@ -98,6 +98,9 @@ typedef struct { zend_object_iterator intern; zval curobj; HashPosition pos; + /* intern->index is only updated for FE_* opcodes, not for e.g. unpacking, + * yet we need to track the position of the node relative to the start. */ + zend_ulong index; php_libxml_cache_tag cache_tag; } php_dom_iterator; diff --git a/ext/dom/tests/unpack_foreach_behaviour.phpt b/ext/dom/tests/unpack_foreach_behaviour.phpt new file mode 100644 index 00000000000..42fe896d9f7 --- /dev/null +++ b/ext/dom/tests/unpack_foreach_behaviour.phpt @@ -0,0 +1,31 @@ +--TEST-- +Unpacking vs foreach behaviour +--EXTENSIONS-- +dom +--FILE-- +loadXML(''); + +echo "--- By foreach: ---\n"; + +foreach ($dom->documentElement->getElementsByTagName('*') as $node) { + var_dump($node->localName); +} + +echo "--- By unpacking: ---\n"; + +$iter = $dom->documentElement->getElementsByTagName('*'); +foreach ([...$iter] as $node) { + var_dump($node->localName); +} + +?> +--EXPECT-- +--- By foreach: --- +string(1) "a" +string(1) "b" +--- By unpacking: --- +string(1) "a" +string(1) "b"