From 8a81d005e53bcfac700c0760aa99afba65ab563d Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Sun, 26 Jan 2025 03:33:38 +0000 Subject: [PATCH 1/4] ext/pdo: Add a test creating instances of Directory This should not be possible, other opaque classes cannot be instantiated in practice as they do not have properties and prevent dynamic properties --- .../tests/pdo_fetch_class_opaque_object.phpt | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 ext/pdo/tests/pdo_fetch_class_opaque_object.phpt diff --git a/ext/pdo/tests/pdo_fetch_class_opaque_object.phpt b/ext/pdo/tests/pdo_fetch_class_opaque_object.phpt new file mode 100644 index 0000000000000..eaef3f6e8f353 --- /dev/null +++ b/ext/pdo/tests/pdo_fetch_class_opaque_object.phpt @@ -0,0 +1,57 @@ +--TEST-- +PDO Common: Attempting to initialize an opaque object via PDO::FETCH_CLASS +--EXTENSIONS-- +pdo +--SKIPIF-- + +--FILE-- +exec('CREATE TABLE pdo_fetch_class_opaque_object(id int NOT NULL PRIMARY KEY, path VARCHAR(10))'); +$db->exec("INSERT INTO pdo_fetch_class_opaque_object VALUES(1, 'AA')"); +$db->exec("INSERT INTO pdo_fetch_class_opaque_object VALUES(2, 'BB')"); +$db->exec("INSERT INTO pdo_fetch_class_opaque_object VALUES(3, 'CC')"); + +$stmt = $db->prepare('SELECT path FROM pdo_fetch_class_opaque_object'); +$stmt->execute(); + +var_dump($stmt->fetchAll(PDO::FETCH_CLASS, 'Directory', [])); +?> +--CLEAN-- + +--EXPECTF-- +array(3) { + [0]=> + object(Directory)#%s (1) { + ["path"]=> + string(2) "AA" + ["handle"]=> + uninitialized(mixed) + } + [1]=> + object(Directory)#%s (1) { + ["path"]=> + string(2) "BB" + ["handle"]=> + uninitialized(mixed) + } + [2]=> + object(Directory)#%s (1) { + ["path"]=> + string(2) "CC" + ["handle"]=> + uninitialized(mixed) + } +} From 37cd00e17026aa680ea1b9b9a2d38978aa1825ea Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Sun, 26 Jan 2025 04:31:49 +0000 Subject: [PATCH 2/4] ext/pdo: Add a test with a fetchAll() call being interupted partways --- .../pdo_fetch_function_exception_in_call.phpt | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 ext/pdo/tests/pdo_fetch_function_exception_in_call.phpt diff --git a/ext/pdo/tests/pdo_fetch_function_exception_in_call.phpt b/ext/pdo/tests/pdo_fetch_function_exception_in_call.phpt new file mode 100644 index 0000000000000..7afeaf971a3df --- /dev/null +++ b/ext/pdo/tests/pdo_fetch_function_exception_in_call.phpt @@ -0,0 +1,55 @@ +--TEST-- +PDO Common: PDO::FETCH_FUNC with a call that throws an exception for one specific value +--EXTENSIONS-- +pdo +--SKIPIF-- + +--FILE-- +exec('CREATE TABLE pdo_fetch_function_exception_in_call(id int NOT NULL PRIMARY KEY, val1 VARCHAR(10), val2 VARCHAR(10))'); +$db->exec("INSERT INTO pdo_fetch_function_exception_in_call VALUES(1, 'A', 'alpha')"); +$db->exec("INSERT INTO pdo_fetch_function_exception_in_call VALUES(2, 'B', 'beta')"); +$db->exec("INSERT INTO pdo_fetch_function_exception_in_call VALUES(3, 'C', 'gamma')"); +$db->exec("INSERT INTO pdo_fetch_function_exception_in_call VALUES(4, 'D', 'delta')"); + +$selectVals = $db->prepare('SELECT val1, val2 FROM pdo_fetch_function_exception_in_call'); + +function bogusCallback(string $val1, string $val2) { + $r = $val1 . ': ' . $val2 . PHP_EOL; + echo $r; + if ($val2 === 'gamma') { + throw new Exception("GAMMA IS BAD"); + } + return $val1 . ': ' . $val2; +} + +$selectVals->execute(); + +try { + $result = $selectVals->fetchAll(PDO::FETCH_FUNC, 'bogusCallback'); + var_dump($result); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +?> +--CLEAN-- + +--EXPECT-- +A: alpha +B: beta +C: gamma +Exception: GAMMA IS BAD From fc7c353519740393449f3a7e240b84b8ba38b8a7 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 26 Jan 2025 11:05:37 +0100 Subject: [PATCH 3/4] Fix GH-17572: getElementsByTagName returns collections with tagName-based indexing, causing loss of elements when converted to arrays Only (dtd) named node maps should have string-based indexing. The ce check is fragile, just check for the presence of an xml hash table. Closes GH-17580. --- NEWS | 2 + ext/dom/dom_iterators.c | 27 ++++++---- ext/dom/tests/modern/xml/gh17572.phpt | 71 +++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 9 deletions(-) create mode 100644 ext/dom/tests/modern/xml/gh17572.phpt diff --git a/NEWS b/NEWS index 8afdb56f2758f..7587ada8216d1 100644 --- a/NEWS +++ b/NEWS @@ -30,6 +30,8 @@ PHP NEWS (nielsdos) . Fixed bug GH-17485 (upstream fix, Self-closing tag on void elements shouldn't be a parse error/warning in \Dom\HTMLDocument). (lexborisov) + . Fixed bug GH-17572 (getElementsByTagName returns collections with + tagName-based indexing). (nielsdos) - Enchant: . Fix crashes in enchant when passing null bytes. (nielsdos) diff --git a/ext/dom/dom_iterators.c b/ext/dom/dom_iterators.c index ad7dae5b3f129..36ed3cd3e6f5f 100644 --- a/ext/dom/dom_iterators.c +++ b/ext/dom/dom_iterators.c @@ -49,6 +49,13 @@ static void itemHashScanner (void *payload, void *data, xmlChar *name) } /* }}} */ +static dom_nnodemap_object *php_dom_iterator_get_nnmap(const php_dom_iterator *iterator) +{ + const zval *object = &iterator->intern.data; + dom_object *nnmap = Z_DOMOBJ_P(object); + return nnmap->ptr; +} + xmlNodePtr create_notation(const xmlChar *name, const xmlChar *ExternalID, const xmlChar *SystemID) /* {{{ */ { xmlEntityPtr ret = xmlMalloc(sizeof(xmlEntity)); @@ -120,18 +127,22 @@ zval *php_dom_iterator_current_data(zend_object_iterator *iter) /* {{{ */ static void php_dom_iterator_current_key(zend_object_iterator *iter, zval *key) /* {{{ */ { php_dom_iterator *iterator = (php_dom_iterator *)iter; - zval *object = &iterator->intern.data; - zend_class_entry *ce = Z_OBJCE_P(object); + dom_nnodemap_object *objmap = php_dom_iterator_get_nnmap(iterator); - /* Nodelists have the index as a key while named node maps have the name as a key. */ - if (instanceof_function(ce, dom_nodelist_class_entry) || instanceof_function(ce, dom_modern_nodelist_class_entry)) { + /* Only dtd named node maps, i.e. the ones based on a libxml hash table or attribute collections, + * are keyed by the name because in that case the name is unique. */ + if (!objmap->ht && objmap->nodetype != XML_ATTRIBUTE_NODE) { ZVAL_LONG(key, iter->index); } else { dom_object *intern = Z_DOMOBJ_P(&iterator->curobj); if (intern != NULL && intern->ptr != NULL) { - xmlNodePtr curnode = (xmlNodePtr)((php_libxml_node_ptr *)intern->ptr)->node; - ZVAL_STRINGL(key, (char *) curnode->name, xmlStrlen(curnode->name)); + xmlNodePtr curnode = ((php_libxml_node_ptr *)intern->ptr)->node; + if (objmap->nodetype == XML_ATTRIBUTE_NODE && php_dom_follow_spec_intern(intern)) { + ZVAL_NEW_STR(key, dom_node_get_node_name_attribute_or_element(curnode, false)); + } else { + ZVAL_STRINGL(key, (const char *) curnode->name, xmlStrlen(curnode->name)); + } } else { ZVAL_NULL(key); } @@ -169,9 +180,7 @@ static void php_dom_iterator_move_forward(zend_object_iterator *iter) /* {{{ */ } dom_object *intern = Z_DOMOBJ_P(&iterator->curobj); - zval *object = &iterator->intern.data; - dom_object *nnmap = Z_DOMOBJ_P(object); - dom_nnodemap_object *objmap = nnmap->ptr; + dom_nnodemap_object *objmap = php_dom_iterator_get_nnmap(iterator); if (intern != NULL && intern->ptr != NULL) { if (objmap->nodetype != XML_ENTITY_NODE && diff --git a/ext/dom/tests/modern/xml/gh17572.phpt b/ext/dom/tests/modern/xml/gh17572.phpt new file mode 100644 index 0000000000000..1f1c2937b00a5 --- /dev/null +++ b/ext/dom/tests/modern/xml/gh17572.phpt @@ -0,0 +1,71 @@ +--TEST-- +GH-17572 (getElementsByTagName returns collections with tagName-based indexing, causing loss of elements when converted to arrays) +--EXTENSIONS-- +dom +--FILE-- + +]> + +

+

+ +XML); + +echo "--- querySelectorAll('p') ---\n"; + +foreach ($dom->querySelectorAll('p') as $k => $v) { + var_dump($k, $v->nodeName); +} + +echo "--- getElementsByTagName('p') ---\n"; + +foreach ($dom->getElementsByTagName('p') as $k => $v) { + var_dump($k, $v->nodeName); +} + +echo "--- entities ---\n"; + +foreach ($dom->doctype->entities as $k => $v) { + var_dump($k, $v->nodeName); +} + +echo "--- attributes ---\n"; + +$attribs = $dom->getElementsByTagName('p')[1]->attributes; +foreach ($attribs as $k => $v) { + var_dump($k, $v->value); + var_dump($attribs[$k]->value); +} + +?> +--EXPECT-- +--- querySelectorAll('p') --- +int(0) +string(1) "p" +int(1) +string(1) "p" +--- getElementsByTagName('p') --- +int(0) +string(1) "p" +int(1) +string(1) "p" +--- entities --- +string(1) "a" +string(1) "a" +--- attributes --- +string(7) "xmlns:x" +string(5) "urn:x" +string(5) "urn:x" +string(3) "x:a" +string(1) "1" +string(1) "1" +string(1) "b" +string(1) "2" +string(1) "2" +string(1) "a" +string(1) "3" +string(1) "3" From bd23d3ab6b02cbb051f753b7be172af7e044549a Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Sun, 26 Jan 2025 17:19:40 +0100 Subject: [PATCH 4/4] Remove unused variable (GH-17573) --- Zend/zend_call_stack.c | 1 - 1 file changed, 1 deletion(-) diff --git a/Zend/zend_call_stack.c b/Zend/zend_call_stack.c index ac12b8b7d8716..ed86ecc74a238 100644 --- a/Zend/zend_call_stack.c +++ b/Zend/zend_call_stack.c @@ -380,7 +380,6 @@ static bool zend_call_stack_get_freebsd(zend_call_stack *stack) static bool zend_call_stack_get_win32(zend_call_stack *stack) { ULONG_PTR low_limit, high_limit; - ULONG size; MEMORY_BASIC_INFORMATION guard_region = {0}, uncommitted_region = {0}; size_t result_size, page_size;