Skip to content

Fix reference access in dimensions for DOMNodeList and DOMNodeMap #13511

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Fix reference access in dimensions for DOMNodeList and DOMNodeMap #13511

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1d59cf9
Add test with wrong output
nielsdos Feb 25, 2024
931fb2d
Fix reference access in dimensions for DOMNodeList and DOMNodeMap
nielsdos Feb 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions ext/dom/DOMNamedNodeMap_string_references.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
--TEST--
DOMNamedNodeMap string references
--EXTENSIONS--
dom
--FILE--
<?php

$dom = new DOMDocument;
$dom->loadXML('<a href="hi" foo="bar"/>');

$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);

?>
--EXPECTF--
bool(true)
string(2) "hi"
bool(true)
string(3) "bar"
bool(true)
string(2) "hi"
bool(true)
string(3) "bar"

Warning: Attempt to read property "value" on null in %s on line %d
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this warning kinda confusing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's because the lookup returns NULL, but at least for this test I can adapt the test so it's less confusing.

bool(false)
NULL
bool(true)
string(2) "hi"
bool(true)
string(3) "bar"
8 changes: 8 additions & 0 deletions ext/dom/php_dom.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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 */
Expand All @@ -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 */
Expand All @@ -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 */
Expand Down
51 changes: 26 additions & 25 deletions ext/dom/tests/bug67949.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ dom
--FILE--
<?php

// Note: non-numeric string accesses fail on NodeLists (as per spec).

$html = <<<HTML
<div>data</div>
<a href="test">hello world</a>
Expand All @@ -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"
Expand All @@ -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==