[php-src] PHP-8.5: ext/dom: getElementsByClassName() item() returns wrong element on random access.
David Carlier <[email protected]>
| Newsgroups | gmane.comp.php.cvs.general |
|---|---|
| Message-ID | <[email protected]> |
Author: David Carlier (devnexen)
Date: 2026-07-12T23:32:59+01:00
Commit: https://github.com/php/php-src/commit/7f97b839764b55f68ad820f98836c8897eaf6648
Raw diff: https://github.com/php/php-src/commit/7f97b839764b55f68ad820f98836c8897eaf6648.diff
ext/dom: getElementsByClassName() item() returns wrong element on random access.
The loop never advanced past the current match, so a cold item($n) with
$n >= 1, or a backwards seek, returned the first match. Fixed by mirroring
dom_map_get_elements_item(): advance to the next match once per remaining index.
close GH-22701
Changed paths:
A ext/dom/tests/modern/common/Element_getElementsByClassName_item_random_access.phpt
M NEWS
M ext/dom/obj_map.c
Diff:
diff --git a/NEWS b/NEWS
index a4f1a24223ec..9cb1c52aacdd 100644
--- a/NEWS
+++ b/NEWS
@@ -27,6 +27,8 @@ PHP NEWS
- DOM:
. Fixed bug GH-22570 (Stack overflow when serializing a deeply nested
Dom\XMLDocument). (iliaal)
+ . Fixed getElementsByClassName() item() returning the wrong element on
+ random access. (David Carlier)
- Exif:
. Fixed bug GH-11020 (exif_read_data() emits a spurious "Illegal IFD size"
diff --git a/ext/dom/obj_map.c b/ext/dom/obj_map.c
index 6e65143648d8..800dadbfb422 100644
--- a/ext/dom/obj_map.c
+++ b/ext/dom/obj_map.c
@@ -346,22 +346,20 @@ static void dom_map_get_by_class_name_item(dom_nnodemap_object *map, zend_long i
if (nodep && index >= 0) {
dom_node_idx_pair start_point = dom_obj_map_get_start_point(map, nodep, index);
if (start_point.node) {
- if (start_point.index > 0) {
- /* Only start iteration at next point if we actually have an index to seek to. */
- itemnode = php_dom_next_in_tree_order(start_point.node, nodep);
- } else {
- itemnode = start_point.node;
- }
+ itemnode = start_point.node;
} else {
itemnode = php_dom_first_child_of_container_node(nodep);
+ while (itemnode != NULL && !dom_matches_class_name(map, itemnode)) {
+ itemnode = php_dom_next_in_tree_order(itemnode, nodep);
+ }
}
- do {
- --start_point.index;
+ for (; start_point.index > 0 && itemnode != NULL; --start_point.index) {
+ itemnode = php_dom_next_in_tree_order(itemnode, nodep);
while (itemnode != NULL && !dom_matches_class_name(map, itemnode)) {
itemnode = php_dom_next_in_tree_order(itemnode, nodep);
}
- } while (start_point.index > 0 && itemnode);
+ }
}
dom_ret_node_to_zobj(map, itemnode, return_value);
if (itemnode) {
diff --git a/ext/dom/tests/modern/common/Element_getElementsByClassName_item_random_access.phpt b/ext/dom/tests/modern/common/Element_getElementsByClassName_item_random_access.phpt
new file mode 100644
index 000000000000..005ad45d3c25
--- /dev/null
+++ b/ext/dom/tests/modern/common/Element_getElementsByClassName_item_random_access.phpt
@@ -0,0 +1,84 @@
+--TEST--
+Dom\Element::getElementsByClassName() item() random access (cold and backwards)
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+
+/* Regression: the item() lookup must return the n-th match for random access,
+ * not only for a strictly ascending / foreach walk. A fresh collection whose
+ * first access is item(n) exercises the uncached path, and a decreasing index
+ * on the same collection exercises the cache-discard path. Non-matching and
+ * nested elements verify that iteration advances to the next matching element
+ * in tree order, not merely to the next sibling. */
+
+$dom = Dom\HTMLDocument::createFromString(<<<HTML
+<!DOCTYPE html>
+<body>
+ <span class="x" id="E0"></span>
+ <div class="y" id="skip1"><span class="x" id="E1"></span></div>
+ <span class="x" id="E2"></span>
+ <p class="z"><b class="x" id="E3"></b></p>
+ <span class="x" id="E4"></span>
+</body>
+HTML);
+
+$body = $dom->getElementsByTagName('body')->item(0);
+
+function id(?Dom\Element $e): string {
+ return $e === null ? 'NULL' : $e->id;
+}
+
+echo "-- cold random access (fresh collection per call) --\n";
+foreach ([0, 1, 2, 3, 4, 5] as $i) {
+ $collection = $body->getElementsByClassName('x');
+ echo "item($i) = ", id($collection->item($i)), "\n";
+}
+
+echo "-- backwards seek on one collection --\n";
+$collection = $body->getElementsByClassName('x');
+foreach ([4, 2, 0, 3, 1] as $i) {
+ echo "item($i) = ", id($collection->item($i)), "\n";
+}
+
+echo "-- item() seed then foreach --\n";
+$collection = $body->getElementsByClassName('x');
+$collection->item(3);
+$ids = [];
+foreach ($collection as $node) {
+ $ids[] = $node->id;
+}
+echo implode(" ", $ids), "\n";
+
+echo "-- last-element idiom --\n";
+$collection = $body->getElementsByClassName('x');
+echo "length = ", $collection->length, ", last = ", id($collection->item($collection->length - 1)), "\n";
+
+echo "-- live collection after mutation --\n";
+$collection = $body->getElementsByClassName('x');
+echo "item(1) = ", id($collection->item(1)), "\n";
+$dom->getElementById('E1')->remove();
+echo "item(1) = ", id($collection->item(1)), ", length = ", $collection->length, "\n";
+
+?>
+--EXPECT--
+-- cold random access (fresh collection per call) --
+item(0) = E0
+item(1) = E1
+item(2) = E2
+item(3) = E3
+item(4) = E4
+item(5) = NULL
+-- backwards seek on one collection --
+item(4) = E4
+item(2) = E2
+item(0) = E0
+item(3) = E3
+item(1) = E1
+-- item() seed then foreach --
+E0 E1 E2 E3 E4
+-- last-element idiom --
+length = 5, last = E4
+-- live collection after mutation --
+item(1) = E1
+item(1) = E2, length = 4