From 1d59cf974ee823fb784c1b3c191f1e373f549f00 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 25 Feb 2024 22:59:20 +0100 Subject: [PATCH 1/2] Add test with wrong output --- .../DOMNamedNodeMap_string_references.phpt | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 ext/dom/DOMNamedNodeMap_string_references.phpt diff --git a/ext/dom/DOMNamedNodeMap_string_references.phpt b/ext/dom/DOMNamedNodeMap_string_references.phpt new file mode 100644 index 0000000000000..594e8f918e9a5 --- /dev/null +++ b/ext/dom/DOMNamedNodeMap_string_references.phpt @@ -0,0 +1,52 @@ +--TEST-- +DOMNamedNodeMap string references +--EXTENSIONS-- +dom +--FILE-- +loadXML(''); + +$attributes = $dom->documentElement->attributes; + +var_dump(isset($attributes['href']), $attributes['href']->value); + +var_dump(isset($attributes['foo']), $attributes['foo']->value); + +$str = 'href'; +$ref =& $str; +var_dump(isset($attributes[$ref]), $attributes[$ref]->value); + +$str = 'foo'; +$ref =& $str; +var_dump(isset($attributes[$ref]), $attributes[$ref]->value); + +$str = 'this does not exist'; +$ref =& $str; +var_dump(isset($attributes[$ref]), $attributes[$ref]->value); + +$str = '0'; +$ref =& $str; +var_dump(isset($attributes[$ref]), $attributes[$ref]->value); + +$str = '1'; +$ref =& $str; +var_dump(isset($attributes[$ref]), $attributes[$ref]->value); + +?> +--EXPECT-- +bool(true) +string(2) "hi" +bool(true) +string(3) "bar" +bool(true) +string(2) "hi" +bool(true) +string(2) "hi" +bool(true) +string(2) "hi" +bool(true) +string(2) "hi" +bool(true) +string(3) "bar" From 931fb2d3ae56faf0dd7e5da0253463a8af3813b2 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 25 Feb 2024 23:00:30 +0100 Subject: [PATCH 2/2] Fix reference access in dimensions for DOMNodeList and DOMNodeMap --- .../DOMNamedNodeMap_string_references.phpt | 10 ++-- ext/dom/php_dom.c | 8 +++ ext/dom/tests/bug67949.phpt | 51 ++++++++++--------- 3 files changed, 40 insertions(+), 29 deletions(-) diff --git a/ext/dom/DOMNamedNodeMap_string_references.phpt b/ext/dom/DOMNamedNodeMap_string_references.phpt index 594e8f918e9a5..0539116510798 100644 --- a/ext/dom/DOMNamedNodeMap_string_references.phpt +++ b/ext/dom/DOMNamedNodeMap_string_references.phpt @@ -35,7 +35,7 @@ $ref =& $str; var_dump(isset($attributes[$ref]), $attributes[$ref]->value); ?> ---EXPECT-- +--EXPECTF-- bool(true) string(2) "hi" bool(true) @@ -43,9 +43,11 @@ string(3) "bar" bool(true) string(2) "hi" bool(true) -string(2) "hi" -bool(true) -string(2) "hi" +string(3) "bar" + +Warning: Attempt to read property "value" on null in %s on line %d +bool(false) +NULL bool(true) string(2) "hi" bool(true) diff --git a/ext/dom/php_dom.c b/ext/dom/php_dom.c index 6610a504e4fda..c86cab99c6c79 100644 --- a/ext/dom/php_dom.c +++ b/ext/dom/php_dom.c @@ -1643,6 +1643,8 @@ static zval *dom_nodelist_read_dimension(zend_object *object, zval *offset, int return NULL; } + ZVAL_DEREF(offset); + zend_long lval; if (dom_nodemap_or_nodelist_process_offset_as_named(offset, &lval)) { /* does not support named lookup */ @@ -1656,6 +1658,8 @@ static zval *dom_nodelist_read_dimension(zend_object *object, zval *offset, int static int dom_nodelist_has_dimension(zend_object *object, zval *member, int check_empty) { + ZVAL_DEREF(member); + zend_long offset; if (dom_nodemap_or_nodelist_process_offset_as_named(member, &offset)) { /* does not support named lookup */ @@ -1672,6 +1676,8 @@ static zval *dom_nodemap_read_dimension(zend_object *object, zval *offset, int t return NULL; } + ZVAL_DEREF(offset); + zend_long lval; if (dom_nodemap_or_nodelist_process_offset_as_named(offset, &lval)) { /* exceptional case, switch to named lookup */ @@ -1691,6 +1697,8 @@ static zval *dom_nodemap_read_dimension(zend_object *object, zval *offset, int t static int dom_nodemap_has_dimension(zend_object *object, zval *member, int check_empty) { + ZVAL_DEREF(member); + zend_long offset; if (dom_nodemap_or_nodelist_process_offset_as_named(member, &offset)) { /* exceptional case, switch to named lookup */ diff --git a/ext/dom/tests/bug67949.phpt b/ext/dom/tests/bug67949.phpt index 270beb02b494c..f087633bdfe6f 100644 --- a/ext/dom/tests/bug67949.phpt +++ b/ext/dom/tests/bug67949.phpt @@ -5,6 +5,8 @@ dom --FILE-- data hello world @@ -14,57 +16,56 @@ $doc->loadHTML($html); $nodes = $doc->getElementsByTagName('div'); -echo "testing has_dimension\n"; +echo "--- testing has_dimension ---\n"; var_dump(isset($nodes[0])); var_dump(isset($nodes[1])); var_dump(isset($nodes[-1])); -echo "testing property access\n"; +echo "--- testing property access ---\n"; var_dump($nodes[0]->textContent); var_dump($nodes[1]->textContent); -echo "testing offset not a long\n"; +echo "--- testing offset not a long: array ---\n"; $offset = ['test']; var_dump($offset); var_dump(isset($nodes[$offset]), $nodes[$offset]->textContent); -var_dump($offset); -$something = 'test'; +echo "--- testing offset not a long: Reference to string ---\n"; +$something = 'href'; $offset = &$something; var_dump($offset); var_dump(isset($nodes[$offset]), $nodes[$offset]->textContent); -var_dump($offset); +echo "--- testing offset not a long: string ---\n"; $offset = 'test'; var_dump($offset); var_dump(isset($nodes[$offset]), $nodes[$offset]->textContent); -var_dump($offset); -echo "testing read_dimension with null offset\n"; +echo "--- testing read_dimension with null offset ---\n"; try { var_dump($nodes[][] = 1); } catch (Error $e) { echo $e->getMessage(), "\n"; } -echo "testing attribute access\n"; +echo "--- testing attribute access ---\n"; $anchor = $doc->getElementsByTagName('a')[0]; var_dump($anchor->attributes[0]->name); echo "==DONE==\n"; ?> --EXPECTF-- -testing has_dimension +--- testing has_dimension --- bool(true) bool(false) bool(false) -testing property access +--- testing property access --- string(4) "data" Warning: Attempt to read property "textContent" on null in %s on line %d NULL -testing offset not a long +--- testing offset not a long: array --- array(1) { [0]=> string(4) "test" @@ -73,20 +74,20 @@ array(1) { Warning: Attempt to read property "textContent" on null in %s on line %d bool(false) NULL -array(1) { - [0]=> - string(4) "test" -} -string(4) "test" -bool(true) -string(4) "data" -string(4) "test" -string(4) "test" -bool(true) -string(4) "data" +--- testing offset not a long: Reference to string --- +string(4) "href" + +Warning: Attempt to read property "textContent" on null in %s on line %d +bool(false) +NULL +--- testing offset not a long: string --- string(4) "test" -testing read_dimension with null offset + +Warning: Attempt to read property "textContent" on null in %s on line %d +bool(false) +NULL +--- testing read_dimension with null offset --- Cannot access DOMNodeList without offset -testing attribute access +--- testing attribute access --- string(4) "href" ==DONE==