From d5f68b50fc451ab0d8ef9bdef814fb93d84c554a Mon Sep 17 00:00:00 2001 From: Pierrick Charron Date: Tue, 23 May 2023 16:56:58 -0400 Subject: [PATCH 01/54] PHP-8.2 is now for PHP 8.2.8-dev --- NEWS | 5 ++++- Zend/zend.h | 2 +- configure.ac | 2 +- main/php_version.h | 6 +++--- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/NEWS b/NEWS index 14ac06359aef7..89bf5c1dda162 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,9 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| -?? ??? ????, PHP 8.2.7 +?? ??? ????, PHP 8.2.8 + + +01 Jun 2023, PHP 8.2.7 - Core: . Fixed bug GH-11152 (Unable to alias namespaces containing reserved class diff --git a/Zend/zend.h b/Zend/zend.h index cd10119651ba8..a0f87c6c9dba8 100644 --- a/Zend/zend.h +++ b/Zend/zend.h @@ -20,7 +20,7 @@ #ifndef ZEND_H #define ZEND_H -#define ZEND_VERSION "4.2.7-dev" +#define ZEND_VERSION "4.2.8-dev" #define ZEND_ENGINE_3 diff --git a/configure.ac b/configure.ac index e95a37fd428e0..59b92b958ac3b 100644 --- a/configure.ac +++ b/configure.ac @@ -17,7 +17,7 @@ dnl Basic autoconf initialization, generation of config.nice. dnl ---------------------------------------------------------------------------- AC_PREREQ([2.68]) -AC_INIT([PHP],[8.2.7-dev],[https://github.com/php/php-src/issues],[php],[https://www.php.net]) +AC_INIT([PHP],[8.2.8-dev],[https://github.com/php/php-src/issues],[php],[https://www.php.net]) AC_CONFIG_SRCDIR([main/php_version.h]) AC_CONFIG_AUX_DIR([build]) AC_PRESERVE_HELP_ORDER diff --git a/main/php_version.h b/main/php_version.h index f24a3aa56bcde..80517df950fb5 100644 --- a/main/php_version.h +++ b/main/php_version.h @@ -2,7 +2,7 @@ /* edit configure.ac to change version number */ #define PHP_MAJOR_VERSION 8 #define PHP_MINOR_VERSION 2 -#define PHP_RELEASE_VERSION 7 +#define PHP_RELEASE_VERSION 8 #define PHP_EXTRA_VERSION "-dev" -#define PHP_VERSION "8.2.7-dev" -#define PHP_VERSION_ID 80207 +#define PHP_VERSION "8.2.8-dev" +#define PHP_VERSION_ID 80208 From 2f2fd06be0d99feac2370412d1894317a817dfa9 Mon Sep 17 00:00:00 2001 From: Ben Ramsey Date: Tue, 23 May 2023 16:19:16 -0500 Subject: [PATCH 02/54] PHP-8.1 is now for PHP 8.1.21-dev --- NEWS | 6 +++++- Zend/zend.h | 2 +- configure.ac | 2 +- main/php_version.h | 6 +++--- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/NEWS b/NEWS index ff7857f62e9bd..4e4935cbfdaa5 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,10 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| -?? ??? ????, PHP 8.1.20 +?? ??? ????, PHP 8.1.21 + + + +08 Jun 2023, PHP 8.1.20 - Core: . Fixed bug GH-9068 (Conditional jump or move depends on uninitialised diff --git a/Zend/zend.h b/Zend/zend.h index 5c3c1de49d5ce..e3b67150b7817 100644 --- a/Zend/zend.h +++ b/Zend/zend.h @@ -20,7 +20,7 @@ #ifndef ZEND_H #define ZEND_H -#define ZEND_VERSION "4.1.20-dev" +#define ZEND_VERSION "4.1.21-dev" #define ZEND_ENGINE_3 diff --git a/configure.ac b/configure.ac index 166d313f133bc..661df89a03de5 100644 --- a/configure.ac +++ b/configure.ac @@ -17,7 +17,7 @@ dnl Basic autoconf initialization, generation of config.nice. dnl ---------------------------------------------------------------------------- AC_PREREQ([2.68]) -AC_INIT([PHP],[8.1.20-dev],[https://github.com/php/php-src/issues],[php],[https://www.php.net]) +AC_INIT([PHP],[8.1.21-dev],[https://github.com/php/php-src/issues],[php],[https://www.php.net]) AC_CONFIG_SRCDIR([main/php_version.h]) AC_CONFIG_AUX_DIR([build]) AC_PRESERVE_HELP_ORDER diff --git a/main/php_version.h b/main/php_version.h index 8b01d87fbacc1..0cf267f2e80b5 100644 --- a/main/php_version.h +++ b/main/php_version.h @@ -2,7 +2,7 @@ /* edit configure.ac to change version number */ #define PHP_MAJOR_VERSION 8 #define PHP_MINOR_VERSION 1 -#define PHP_RELEASE_VERSION 20 +#define PHP_RELEASE_VERSION 21 #define PHP_EXTRA_VERSION "-dev" -#define PHP_VERSION "8.1.20-dev" -#define PHP_VERSION_ID 80120 +#define PHP_VERSION "8.1.21-dev" +#define PHP_VERSION_ID 80121 From f5c54fd88b3694c601e58789196d46c29ebadd08 Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Tue, 23 May 2023 12:10:27 +0200 Subject: [PATCH 03/54] Fix access on NULL pointer in array_merge_recursive() Closes GH-11303 --- NEWS | 3 ++- ...ray_merge_recursive_next_key_overflow.phpt | 25 +++++++++++++++++++ Zend/zend_execute.c | 2 +- Zend/zend_execute.h | 2 ++ ext/standard/array.c | 11 +++++++- 5 files changed, 40 insertions(+), 3 deletions(-) create mode 100644 Zend/tests/array_merge_recursive_next_key_overflow.phpt diff --git a/NEWS b/NEWS index 4e4935cbfdaa5..553d8acca6811 100644 --- a/NEWS +++ b/NEWS @@ -2,7 +2,8 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? ????, PHP 8.1.21 - +- Standard: + . Fix access on NULL pointer in array_merge_recursive(). (ilutov) 08 Jun 2023, PHP 8.1.20 diff --git a/Zend/tests/array_merge_recursive_next_key_overflow.phpt b/Zend/tests/array_merge_recursive_next_key_overflow.phpt new file mode 100644 index 0000000000000..f7d2872957837 --- /dev/null +++ b/Zend/tests/array_merge_recursive_next_key_overflow.phpt @@ -0,0 +1,25 @@ +--TEST-- +Access on NULL pointer in array_merge_recursive() +--FILE-- + [PHP_INT_MAX => null]], + ['' => [null]], + ); +} catch (Throwable $e) { + echo $e->getMessage(), "\n"; +} + +try { + array_merge_recursive( + ['foo' => [PHP_INT_MAX => null]], + ['foo' => str_repeat('a', 2)], + ); +} catch (Throwable $e) { + echo $e->getMessage(), "\n"; +} +?> +--EXPECT-- +Cannot add element to the array as the next element is already occupied +Cannot add element to the array as the next element is already occupied diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c index ad4396e5d9e07..5c9a59bb953bf 100644 --- a/Zend/zend_execute.c +++ b/Zend/zend_execute.c @@ -2231,7 +2231,7 @@ static zend_never_inline ZEND_COLD void ZEND_FASTCALL zend_use_scalar_as_array(v zend_throw_error(NULL, "Cannot use a scalar value as an array"); } -static zend_never_inline ZEND_COLD void ZEND_FASTCALL zend_cannot_add_element(void) +ZEND_API zend_never_inline ZEND_COLD void ZEND_FASTCALL zend_cannot_add_element(void) { zend_throw_error(NULL, "Cannot add element to the array as the next element is already occupied"); } diff --git a/Zend/zend_execute.h b/Zend/zend_execute.h index bf7b2afdf88bc..dba57320108e0 100644 --- a/Zend/zend_execute.h +++ b/Zend/zend_execute.h @@ -76,6 +76,8 @@ ZEND_API ZEND_COLD void zend_wrong_string_offset_error(void); ZEND_API ZEND_COLD void ZEND_FASTCALL zend_readonly_property_modification_error(zend_property_info *info); ZEND_API ZEND_COLD void ZEND_FASTCALL zend_readonly_property_indirect_modification_error(zend_property_info *info); +ZEND_API ZEND_COLD void ZEND_FASTCALL zend_cannot_add_element(void); + ZEND_API bool zend_verify_scalar_type_hint(uint32_t type_mask, zval *arg, bool strict, bool is_internal_arg); ZEND_API ZEND_COLD void zend_verify_arg_error( const zend_function *zf, const zend_arg_info *arg_info, uint32_t arg_num, zval *value); diff --git a/ext/standard/array.c b/ext/standard/array.c index fb705cd34c4e8..bfd4f95d4137f 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -3595,7 +3595,12 @@ PHPAPI int php_array_merge_recursive(HashTable *dest, HashTable *src) /* {{{ */ } } else { Z_TRY_ADDREF_P(src_zval); - zend_hash_next_index_insert(Z_ARRVAL_P(dest_zval), src_zval); + zval *zv = zend_hash_next_index_insert(Z_ARRVAL_P(dest_zval), src_zval); + if (EXPECTED(!zv)) { + Z_TRY_DELREF_P(src_zval); + zend_cannot_add_element(); + return 0; + } } zval_ptr_dtor(&tmp); } else { @@ -3604,6 +3609,10 @@ PHPAPI int php_array_merge_recursive(HashTable *dest, HashTable *src) /* {{{ */ } } else { zval *zv = zend_hash_next_index_insert(dest, src_entry); + if (UNEXPECTED(!zv)) { + zend_cannot_add_element(); + return 0; + } zval_add_ref(zv); } } ZEND_HASH_FOREACH_END(); From 7c7698f754ea19435e5b0d2bcf096e446e0bb828 Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Tue, 23 May 2023 13:55:50 +0200 Subject: [PATCH 04/54] Fix preg_replace_callback_array() pattern validation Closes GH-11301 --- NEWS | 3 +++ ext/pcre/php_pcre.c | 4 ++++ ...eplace_callback_array_numeric_index_error.phpt | 15 +++++++++++++++ 3 files changed, 22 insertions(+) create mode 100644 ext/pcre/tests/preg_replace_callback_array_numeric_index_error.phpt diff --git a/NEWS b/NEWS index f352246fb79e7..10cda5af84c63 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,9 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? ????, PHP 8.2.8 +- PCRE: + . Fix preg_replace_callback_array() pattern validation. (ilutov) + - Standard: . Fix access on NULL pointer in array_merge_recursive(). (ilutov) diff --git a/ext/pcre/php_pcre.c b/ext/pcre/php_pcre.c index 82560ddd83a61..ea5e6a01ff065 100644 --- a/ext/pcre/php_pcre.c +++ b/ext/pcre/php_pcre.c @@ -2425,6 +2425,10 @@ PHP_FUNCTION(preg_replace_callback_array) zend_argument_type_error(1, "must contain only valid callbacks"); goto error; } + if (!str_idx_regex) { + zend_argument_type_error(1, "must contain only string patterns as keys"); + goto error; + } ZVAL_COPY_VALUE(&fci.function_name, replace); diff --git a/ext/pcre/tests/preg_replace_callback_array_numeric_index_error.phpt b/ext/pcre/tests/preg_replace_callback_array_numeric_index_error.phpt new file mode 100644 index 0000000000000..55dfabce8649c --- /dev/null +++ b/ext/pcre/tests/preg_replace_callback_array_numeric_index_error.phpt @@ -0,0 +1,15 @@ +--TEST-- +preg_replace_callback_array() invalid pattern +--FILE-- + function () {}], + 'a', +); +?> +--EXPECTF-- +Fatal error: Uncaught TypeError: preg_replace_callback_array(): Argument #1 ($pattern) must contain only string patterns as keys in %s:%d +Stack trace: +#0 %s(%d): preg_replace_callback_array(Array, 'a') +#1 {main} + thrown in %s on line %d From b2ec6c24f8b5d8a2767d1fc2557424bf68608b47 Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Tue, 23 May 2023 09:54:54 +0200 Subject: [PATCH 05/54] Fix exception handling in array_multisort() Closes GH-11302 --- NEWS | 1 + Zend/tests/array_multisort_exception.phpt | 13 +++++++++++++ ext/standard/array.c | 7 +++++-- 3 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 Zend/tests/array_multisort_exception.phpt diff --git a/NEWS b/NEWS index 553d8acca6811..d529159d54fe6 100644 --- a/NEWS +++ b/NEWS @@ -4,6 +4,7 @@ PHP NEWS - Standard: . Fix access on NULL pointer in array_merge_recursive(). (ilutov) + . Fix exception handling in array_multisort(). (ilutov) 08 Jun 2023, PHP 8.1.20 diff --git a/Zend/tests/array_multisort_exception.phpt b/Zend/tests/array_multisort_exception.phpt new file mode 100644 index 0000000000000..8ee6007745e03 --- /dev/null +++ b/Zend/tests/array_multisort_exception.phpt @@ -0,0 +1,13 @@ +--TEST-- +Exception handling in array_multisort() +--FILE-- + new DateTime(), 0 => new DateTime()]; +array_multisort($array, SORT_STRING); +?> +--EXPECTF-- +Fatal error: Uncaught Error: Object of class DateTime could not be converted to string in %s:%d +Stack trace: +#0 %s(%d): array_multisort(Array, 2) +#1 {main} + thrown in %s on line %d diff --git a/ext/standard/array.c b/ext/standard/array.c index bfd4f95d4137f..86d70a8844e30 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -5598,6 +5598,9 @@ PHP_FUNCTION(array_multisort) /* Do the actual sort magic - bada-bim, bada-boom. */ zend_sort(indirect, array_size, sizeof(Bucket *), php_multisort_compare, (swap_func_t)array_bucket_p_sawp); + if (EG(exception)) { + goto clean_up; + } /* Restructure the arrays based on sorted indirect - this is mostly taken from zend_hash_sort() function. */ for (i = 0; i < num_arrays; i++) { @@ -5623,15 +5626,15 @@ PHP_FUNCTION(array_multisort) zend_hash_rehash(hash); } } + RETVAL_TRUE; - /* Clean up. */ +clean_up: for (i = 0; i < array_size; i++) { efree(indirect[i]); } efree(indirect); efree(func); efree(arrays); - RETURN_TRUE; } /* }}} */ From 6267601f84981577d2b9faaa033b776dddfbfb75 Mon Sep 17 00:00:00 2001 From: nielsdos <7771979+nielsdos@users.noreply.github.com> Date: Tue, 23 May 2023 19:50:09 +0200 Subject: [PATCH 06/54] Fix allocation loop in zend_shared_alloc_startup() The break is outside the if, so if it succeeds or not this will always stop after the first loop iteration instead of trying more allocators if the first one fails. Closes GH-11306. --- NEWS | 3 +++ ext/opcache/zend_shared_alloc.c | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index d529159d54fe6..332bd89835b34 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,9 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? ????, PHP 8.1.21 +- Opcache: + . Fix allocation loop in zend_shared_alloc_startup(). (nielsdos) + - Standard: . Fix access on NULL pointer in array_merge_recursive(). (ilutov) . Fix exception handling in array_multisort(). (ilutov) diff --git a/ext/opcache/zend_shared_alloc.c b/ext/opcache/zend_shared_alloc.c index 53c7b61ff3f36..be931f526c1b1 100644 --- a/ext/opcache/zend_shared_alloc.c +++ b/ext/opcache/zend_shared_alloc.c @@ -179,8 +179,8 @@ int zend_shared_alloc_startup(size_t requested_size, size_t reserved_size) res = zend_shared_alloc_try(he, requested_size, &ZSMMG(shared_segments), &ZSMMG(shared_segments_count), &error_in); if (res) { /* this model works! */ + break; } - break; } } } From 150825d176a6a46216e2f29eb71cf1081d840f74 Mon Sep 17 00:00:00 2001 From: Pierrick Charron Date: Wed, 24 May 2023 23:41:36 -0400 Subject: [PATCH 07/54] [skip ci] Fix release date of PHP 8.2.7 --- NEWS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS b/NEWS index a4ca945bf08d2..6b9667d91ee2f 100644 --- a/NEWS +++ b/NEWS @@ -12,7 +12,7 @@ PHP NEWS . Fix access on NULL pointer in array_merge_recursive(). (ilutov) . Fix exception handling in array_multisort(). (ilutov) -01 Jun 2023, PHP 8.2.7 +08 Jun 2023, PHP 8.2.7 - Core: . Fixed bug GH-11152 (Unable to alias namespaces containing reserved class From 8946b7b141ea72aa72d1330c27fa76c9f0af9b03 Mon Sep 17 00:00:00 2001 From: KoudelkaB <33930155+KoudelkaB@users.noreply.github.com> Date: Wed, 24 May 2023 17:23:27 +0200 Subject: [PATCH 08/54] Access violation when ALLOC_FALLBACK fixed Close GH-11312 --- NEWS | 1 + ext/opcache/zend_shared_alloc.c | 2 ++ 2 files changed, 3 insertions(+) diff --git a/NEWS b/NEWS index 332bd89835b34..35e9fb124cfbe 100644 --- a/NEWS +++ b/NEWS @@ -4,6 +4,7 @@ PHP NEWS - Opcache: . Fix allocation loop in zend_shared_alloc_startup(). (nielsdos) + . Access violation on smm_shared_globals with ALLOC_FALLBACK. (KoudelkaB) - Standard: . Fix access on NULL pointer in array_merge_recursive(). (ilutov) diff --git a/ext/opcache/zend_shared_alloc.c b/ext/opcache/zend_shared_alloc.c index be931f526c1b1..afe539bf987a7 100644 --- a/ext/opcache/zend_shared_alloc.c +++ b/ext/opcache/zend_shared_alloc.c @@ -191,6 +191,7 @@ int zend_shared_alloc_startup(size_t requested_size, size_t reserved_size) } #if ENABLE_FILE_CACHE_FALLBACK if (ALLOC_FALLBACK == res) { + smm_shared_globals = NULL; return ALLOC_FALLBACK; } #endif @@ -216,6 +217,7 @@ int zend_shared_alloc_startup(size_t requested_size, size_t reserved_size) } #if ENABLE_FILE_CACHE_FALLBACK if (ALLOC_FALLBACK == res) { + smm_shared_globals = NULL; return ALLOC_FALLBACK; } #endif From cba335d61e68fa4ae9e0b36184552c76878ff615 Mon Sep 17 00:00:00 2001 From: nielsdos <7771979+nielsdos@users.noreply.github.com> Date: Mon, 22 May 2023 23:50:06 +0200 Subject: [PATCH 09/54] Fix GH-11288 and GH-11289 and GH-11290 and GH-9142: DOMExceptions and segfaults with replaceWith This replaces the implementation of before and after with one following the spec very strictly, instead of trying to figure out the state we're in by looking at the pointers. Also relaxes the condition on text node copying to prevent working on a stale node pointer. Closes GH-11299. --- NEWS | 4 + ext/dom/parentnode.c | 188 +++++++++++++++++++--------------- ext/dom/tests/bug80602.phpt | 88 ++++++++-------- ext/dom/tests/bug80602_2.phpt | 88 ++++++++-------- ext/dom/tests/bug80602_3.phpt | 120 ++++++++++++++++++++++ ext/dom/tests/bug80602_4.phpt | 33 ++++++ ext/dom/tests/gh11288.phpt | 67 ++++++++++++ ext/dom/tests/gh11289.phpt | 28 +++++ ext/dom/tests/gh11290.phpt | 27 +++++ ext/dom/tests/gh9142.phpt | 20 ++++ 10 files changed, 490 insertions(+), 173 deletions(-) create mode 100644 ext/dom/tests/bug80602_3.phpt create mode 100644 ext/dom/tests/bug80602_4.phpt create mode 100644 ext/dom/tests/gh11288.phpt create mode 100644 ext/dom/tests/gh11289.phpt create mode 100644 ext/dom/tests/gh11290.phpt create mode 100644 ext/dom/tests/gh9142.phpt diff --git a/NEWS b/NEWS index 35e9fb124cfbe..93b363cfac79a 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,10 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? ????, PHP 8.1.21 +- DOM: + . Fixed bugs GH-11288 and GH-11289 and GH-11290 and GH-9142 (DOMExceptions + and segfaults with replaceWith). (nielsdos) + - Opcache: . Fix allocation loop in zend_shared_alloc_startup(). (nielsdos) . Access violation on smm_shared_globals with ALLOC_FALLBACK. (KoudelkaB) diff --git a/ext/dom/parentnode.c b/ext/dom/parentnode.c index cf823057d22ae..46c90a13e31d5 100644 --- a/ext/dom/parentnode.c +++ b/ext/dom/parentnode.c @@ -124,6 +124,23 @@ int dom_parent_node_child_element_count(dom_object *obj, zval *retval) } /* }}} */ +static bool dom_is_node_in_list(const zval *nodes, int nodesc, const xmlNodePtr node_to_find) +{ + for (int i = 0; i < nodesc; i++) { + if (Z_TYPE(nodes[i]) == IS_OBJECT) { + const zend_class_entry *ce = Z_OBJCE(nodes[i]); + + if (instanceof_function(ce, dom_node_class_entry)) { + if (dom_object_get_node(Z_DOMOBJ_P(nodes + i)) == node_to_find) { + return true; + } + } + } + } + + return false; +} + xmlNode* dom_zvals_to_fragment(php_libxml_ref_obj *document, xmlNode *contextNode, zval *nodes, int nodesc) { int i; @@ -177,17 +194,16 @@ xmlNode* dom_zvals_to_fragment(php_libxml_ref_obj *document, xmlNode *contextNod goto hierarchy_request_err; } - /* - * xmlNewDocText function will always returns same address to the second parameter if the parameters are greater than or equal to three. - * If it's text, that's fine, but if it's an object, it can cause invalid pointer because many new nodes point to the same memory address. - * So we must copy the new node to avoid this situation. - */ - if (nodesc > 1) { + /* Citing from the docs (https://gnome.pages.gitlab.gnome.org/libxml2/devhelp/libxml2-tree.html#xmlAddChild): + * "Add a new node to @parent, at the end of the child (or property) list merging adjacent TEXT nodes (in which case @cur is freed)". + * So we must take a copy if this situation arises to prevent a use-after-free. */ + bool will_free = newNode->type == XML_TEXT_NODE && fragment->last && fragment->last->type == XML_TEXT_NODE; + if (will_free) { newNode = xmlCopyNode(newNode, 1); } if (!xmlAddChild(fragment, newNode)) { - if (nodesc > 1) { + if (will_free) { xmlFreeNode(newNode); } goto hierarchy_request_err; @@ -303,25 +319,64 @@ void dom_parent_node_prepend(dom_object *context, zval *nodes, int nodesc) xmlFree(fragment); } +static void dom_pre_insert(xmlNodePtr insertion_point, xmlNodePtr parentNode, xmlNodePtr newchild, xmlNodePtr fragment) +{ + if (!insertion_point) { + /* Place it as last node */ + if (parentNode->children) { + /* There are children */ + fragment->last->prev = parentNode->last; + newchild->prev = parentNode->last->prev; + parentNode->last->next = newchild; + } else { + /* No children, because they moved out when they became a fragment */ + parentNode->children = newchild; + parentNode->last = newchild; + } + } else { + /* Insert fragment before insertion_point */ + fragment->last->next = insertion_point; + if (insertion_point->prev) { + insertion_point->prev->next = newchild; + newchild->prev = insertion_point->prev; + } + insertion_point->prev = newchild; + if (parentNode->children == insertion_point) { + parentNode->children = newchild; + } + } +} + void dom_parent_node_after(dom_object *context, zval *nodes, int nodesc) { + /* Spec link: https://dom.spec.whatwg.org/#dom-childnode-after */ + xmlNode *prevsib = dom_object_get_node(context); xmlNodePtr newchild, parentNode; - xmlNode *fragment, *nextsib; + xmlNode *fragment; xmlDoc *doc; - bool afterlastchild; - - int stricterror = dom_get_strict_error(context->document); - if (!prevsib->parent) { - php_dom_throw_error(NO_MODIFICATION_ALLOWED_ERR, stricterror); + /* Spec step 1 */ + parentNode = prevsib->parent; + /* Spec step 2 */ + if (!parentNode) { + int stricterror = dom_get_strict_error(context->document); + php_dom_throw_error(HIERARCHY_REQUEST_ERR, stricterror); return; } + /* Spec step 3: find first following child not in nodes; otherwise null */ + xmlNodePtr viable_next_sibling = prevsib->next; + while (viable_next_sibling) { + if (!dom_is_node_in_list(nodes, nodesc, viable_next_sibling)) { + break; + } + viable_next_sibling = viable_next_sibling->next; + } + doc = prevsib->doc; - parentNode = prevsib->parent; - nextsib = prevsib->next; - afterlastchild = (nextsib == NULL); + + /* Spec step 4: convert nodes into fragment */ fragment = dom_zvals_to_fragment(context->document, parentNode, nodes, nodesc); if (fragment == NULL) { @@ -331,40 +386,9 @@ void dom_parent_node_after(dom_object *context, zval *nodes, int nodesc) newchild = fragment->children; if (newchild) { - /* first node and last node are both both parameters to DOMElement::after() method so nextsib and prevsib are null. */ - if (!parentNode->children) { - prevsib = nextsib = NULL; - } else if (afterlastchild) { - /* - * The new node will be inserted after last node, prevsib is last node. - * The first node is the parameter to DOMElement::after() if parentNode->children == prevsib is true - * and prevsib does not change, otherwise prevsib is parentNode->last (first node). - */ - prevsib = parentNode->children == prevsib ? prevsib : parentNode->last; - } else { - /* - * The new node will be inserted after first node, prevsib is first node. - * The first node is not the parameter to DOMElement::after() if parentNode->children == prevsib is true - * and prevsib does not change otherwise prevsib is null to mean that parentNode->children is the new node. - */ - prevsib = parentNode->children == prevsib ? prevsib : NULL; - } - - if (prevsib) { - fragment->last->next = prevsib->next; - if (prevsib->next) { - prevsib->next->prev = fragment->last; - } - prevsib->next = newchild; - } else { - parentNode->children = newchild; - if (nextsib) { - fragment->last->next = nextsib; - nextsib->prev = fragment->last; - } - } + /* Step 5: place fragment into the parent before viable_next_sibling */ + dom_pre_insert(viable_next_sibling, parentNode, newchild, fragment); - newchild->prev = prevsib; dom_fragment_assign_parent_node(parentNode, fragment); dom_reconcile_ns(doc, newchild); } @@ -374,17 +398,34 @@ void dom_parent_node_after(dom_object *context, zval *nodes, int nodesc) void dom_parent_node_before(dom_object *context, zval *nodes, int nodesc) { + /* Spec link: https://dom.spec.whatwg.org/#dom-childnode-before */ + xmlNode *nextsib = dom_object_get_node(context); - xmlNodePtr newchild, prevsib, parentNode; - xmlNode *fragment, *afternextsib; + xmlNodePtr newchild, parentNode; + xmlNode *fragment; xmlDoc *doc; - bool beforefirstchild; - doc = nextsib->doc; - prevsib = nextsib->prev; - afternextsib = nextsib->next; + /* Spec step 1 */ parentNode = nextsib->parent; - beforefirstchild = !prevsib; + /* Spec step 2 */ + if (!parentNode) { + int stricterror = dom_get_strict_error(context->document); + php_dom_throw_error(HIERARCHY_REQUEST_ERR, stricterror); + return; + } + + /* Spec step 3: find first following child not in nodes; otherwise null */ + xmlNodePtr viable_previous_sibling = nextsib->prev; + while (viable_previous_sibling) { + if (!dom_is_node_in_list(nodes, nodesc, viable_previous_sibling)) { + break; + } + viable_previous_sibling = viable_previous_sibling->prev; + } + + doc = nextsib->doc; + + /* Spec step 4: convert nodes into fragment */ fragment = dom_zvals_to_fragment(context->document, parentNode, nodes, nodesc); if (fragment == NULL) { @@ -394,37 +435,14 @@ void dom_parent_node_before(dom_object *context, zval *nodes, int nodesc) newchild = fragment->children; if (newchild) { - /* first node and last node are both both parameters to DOMElement::before() method so nextsib is null. */ - if (!parentNode->children) { - nextsib = NULL; - } else if (beforefirstchild) { - /* - * The new node will be inserted before first node, nextsib is first node and afternextsib is last node. - * The first node is not the parameter to DOMElement::before() if parentNode->children == nextsib is true - * and nextsib does not change, otherwise nextsib is the last node. - */ - nextsib = parentNode->children == nextsib ? nextsib : afternextsib; - } else { - /* - * The new node will be inserted before last node, prevsib is first node and nestsib is last node. - * The first node is not the parameter to DOMElement::before() if parentNode->children == prevsib is true - * but last node may be, so use prevsib->next to determine the value of nextsib, otherwise nextsib does not change. - */ - nextsib = parentNode->children == prevsib ? prevsib->next : nextsib; - } - - if (parentNode->children == nextsib) { - parentNode->children = newchild; + /* Step 5: if viable_previous_sibling is null, set it to the parent's first child, otherwise viable_previous_sibling's next sibling */ + if (!viable_previous_sibling) { + viable_previous_sibling = parentNode->children; } else { - prevsib->next = newchild; - } - - fragment->last->next = nextsib; - if (nextsib) { - nextsib->prev = fragment->last; + viable_previous_sibling = viable_previous_sibling->next; } - - newchild->prev = prevsib; + /* Step 6: place fragment into the parent after viable_previous_sibling */ + dom_pre_insert(viable_previous_sibling, parentNode, newchild, fragment); dom_fragment_assign_parent_node(parentNode, fragment); dom_reconcile_ns(doc, newchild); diff --git a/ext/dom/tests/bug80602.phpt b/ext/dom/tests/bug80602.phpt index 9f041f686f516..844d829cb08d0 100644 --- a/ext/dom/tests/bug80602.phpt +++ b/ext/dom/tests/bug80602.phpt @@ -8,84 +8,84 @@ $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->firstChild; $target->before($target); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "1 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->lastChild; $target->before($target); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "2 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->firstChild; $target->before($doc->documentElement->lastChild); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "3 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->lastChild; $target->before($doc->documentElement->firstChild); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "4 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->firstChild; $target->before($target, $doc->documentElement->lastChild); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "5 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->firstChild; $target->before($doc->documentElement->lastChild, $target); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "6 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->lastChild; $target->before($target, $doc->documentElement->firstChild); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "7 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->lastChild; $target->before($doc->documentElement->firstChild, $target); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "8 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->firstChild; $target->before('bar','baz'); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "9 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->lastChild; $target->before('bar','baz'); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "10 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->firstChild; $target->before($target, 'bar','baz'); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "11 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->firstChild; $target->before('bar', $target, 'baz'); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "12 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->firstChild; $target->before('bar', 'baz', $target); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "13 ", $doc->saveXML($doc->documentElement).PHP_EOL; @@ -93,19 +93,19 @@ $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->lastChild; $target->before($target, 'bar','baz'); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "14 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->lastChild; $target->before('bar', $target, 'baz'); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "15 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->lastChild; $target->before('bar', 'baz', $target); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "16 ", $doc->saveXML($doc->documentElement).PHP_EOL; @@ -113,21 +113,21 @@ $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->firstChild; $target->before('bar', $target, $doc->documentElement->lastChild); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "17 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->firstChild; $target->before($target, 'bar', $doc->documentElement->lastChild); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "18 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->firstChild; $target->before($target, $doc->documentElement->lastChild, 'bar'); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "19 ", $doc->saveXML($doc->documentElement).PHP_EOL; @@ -136,43 +136,43 @@ $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->lastChild; $target->before('bar', $doc->documentElement->firstChild, $target); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "20 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->lastChild; $target->before($doc->documentElement->firstChild, 'bar', $target); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "21 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->lastChild; $target->before($doc->documentElement->firstChild, $target, 'bar'); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "22 ", $doc->saveXML($doc->documentElement).PHP_EOL; ?> --EXPECTF-- -foo -foo -foo -foo -foo -foo -foo -foo -barbazfoo -foobarbaz -foobarbaz -barfoobaz -barbazfoo -foobarbaz -foobarbaz -foobarbaz -barfoo -foobar -foobar -barfoo -foobar -foobar +1 foo +2 foo +3 foo +4 foo +5 foo +6 foo +7 foo +8 foo +9 barbazfoo +10 foobarbaz +11 foobarbaz +12 barfoobaz +13 barbazfoo +14 foobarbaz +15 foobarbaz +16 foobarbaz +17 barfoo +18 foobar +19 foobar +20 barfoo +21 foobar +22 foobar diff --git a/ext/dom/tests/bug80602_2.phpt b/ext/dom/tests/bug80602_2.phpt index 1151417c0f845..7c5070f51424c 100644 --- a/ext/dom/tests/bug80602_2.phpt +++ b/ext/dom/tests/bug80602_2.phpt @@ -8,84 +8,84 @@ $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->firstChild; $target->after($target); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "1 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->lastChild; $target->after($target); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "2 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->firstChild; $target->after($doc->documentElement->lastChild); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "3 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->lastChild; $target->after($doc->documentElement->firstChild); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "4 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->firstChild; $target->after($target, $doc->documentElement->lastChild); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "5 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->firstChild; $target->after($doc->documentElement->lastChild, $target); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "6 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->lastChild; $target->after($target, $doc->documentElement->firstChild); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "7 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->lastChild; $target->after($doc->documentElement->firstChild, $target); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "8 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->firstChild; $target->after('bar','baz'); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "9 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->lastChild; $target->after('bar','baz'); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "10 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->firstChild; $target->after($target, 'bar','baz'); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "11 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->firstChild; $target->after('bar', $target, 'baz'); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "12 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->firstChild; $target->after('bar', 'baz', $target); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "13 ", $doc->saveXML($doc->documentElement).PHP_EOL; @@ -93,19 +93,19 @@ $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->lastChild; $target->after($target, 'bar','baz'); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "14 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->lastChild; $target->after('bar', $target, 'baz'); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "15 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->lastChild; $target->after('bar', 'baz', $target); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "16 ", $doc->saveXML($doc->documentElement).PHP_EOL; @@ -113,21 +113,21 @@ $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->firstChild; $target->after('bar', $target, $doc->documentElement->lastChild); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "17 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->firstChild; $target->after($target, 'bar', $doc->documentElement->lastChild); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "18 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->firstChild; $target->after($target, $doc->documentElement->lastChild, 'bar'); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "19 ", $doc->saveXML($doc->documentElement).PHP_EOL; @@ -136,43 +136,43 @@ $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->lastChild; $target->after('bar', $doc->documentElement->firstChild, $target); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "20 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->lastChild; $target->after($doc->documentElement->firstChild, 'bar', $target); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "21 ", $doc->saveXML($doc->documentElement).PHP_EOL; $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->lastChild; $target->after($doc->documentElement->firstChild, $target, 'bar'); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "22 ", $doc->saveXML($doc->documentElement).PHP_EOL; ?> --EXPECTF-- -foo -foo -foo -foo -foo -foo -foo -foo -foobarbaz -foobarbaz -foobarbaz -barfoobaz -barbazfoo -foobarbaz -foobarbaz -foobarbaz -barfoo -foobar -foobar -barfoo -foobar -foobar +1 foo +2 foo +3 foo +4 foo +5 foo +6 foo +7 foo +8 foo +9 foobarbaz +10 foobarbaz +11 foobarbaz +12 barfoobaz +13 barbazfoo +14 foobarbaz +15 foobarbaz +16 foobarbaz +17 barfoo +18 foobar +19 foobar +20 barfoo +21 foobar +22 foobar diff --git a/ext/dom/tests/bug80602_3.phpt b/ext/dom/tests/bug80602_3.phpt new file mode 100644 index 0000000000000..f9bf67e778da5 --- /dev/null +++ b/ext/dom/tests/bug80602_3.phpt @@ -0,0 +1,120 @@ +--TEST-- +Bug #80602 (Segfault when using DOMChildNode::before()) - use-after-free variation +--FILE-- +loadXML('foo'); +$target = $doc->documentElement->lastChild; +$target->before('bar', $doc->documentElement->firstChild, 'baz'); +echo $doc->saveXML($doc->documentElement), "\n"; +var_dump($target); + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->lastChild; +// Note: after instead of before +$target->after('bar', $doc->documentElement->firstChild, 'baz'); +echo $doc->saveXML($doc->documentElement), "\n"; +var_dump($target); + +?> +--EXPECTF-- +barfoobaz +object(DOMElement)#3 (23) { + ["schemaTypeInfo"]=> + NULL + ["tagName"]=> + string(4) "last" + ["firstElementChild"]=> + NULL + ["lastElementChild"]=> + NULL + ["childElementCount"]=> + int(0) + ["previousElementSibling"]=> + NULL + ["nextElementSibling"]=> + NULL + ["nodeName"]=> + string(4) "last" + ["nodeValue"]=> + string(0) "" + ["nodeType"]=> + int(1) + ["parentNode"]=> + string(22) "(object value omitted)" + ["childNodes"]=> + string(22) "(object value omitted)" + ["firstChild"]=> + NULL + ["lastChild"]=> + NULL + ["previousSibling"]=> + string(22) "(object value omitted)" + ["nextSibling"]=> + NULL + ["attributes"]=> + string(22) "(object value omitted)" + ["ownerDocument"]=> + string(22) "(object value omitted)" + ["namespaceURI"]=> + NULL + ["prefix"]=> + string(0) "" + ["localName"]=> + string(4) "last" + ["baseURI"]=> + string(%d) %s + ["textContent"]=> + string(0) "" +} +barfoobaz +object(DOMElement)#2 (23) { + ["schemaTypeInfo"]=> + NULL + ["tagName"]=> + string(4) "last" + ["firstElementChild"]=> + NULL + ["lastElementChild"]=> + NULL + ["childElementCount"]=> + int(0) + ["previousElementSibling"]=> + NULL + ["nextElementSibling"]=> + NULL + ["nodeName"]=> + string(4) "last" + ["nodeValue"]=> + string(0) "" + ["nodeType"]=> + int(1) + ["parentNode"]=> + string(22) "(object value omitted)" + ["childNodes"]=> + string(22) "(object value omitted)" + ["firstChild"]=> + NULL + ["lastChild"]=> + NULL + ["previousSibling"]=> + NULL + ["nextSibling"]=> + string(22) "(object value omitted)" + ["attributes"]=> + string(22) "(object value omitted)" + ["ownerDocument"]=> + string(22) "(object value omitted)" + ["namespaceURI"]=> + NULL + ["prefix"]=> + string(0) "" + ["localName"]=> + string(4) "last" + ["baseURI"]=> + string(%d) %s + ["textContent"]=> + string(0) "" +} diff --git a/ext/dom/tests/bug80602_4.phpt b/ext/dom/tests/bug80602_4.phpt new file mode 100644 index 0000000000000..a1df8d10caa31 --- /dev/null +++ b/ext/dom/tests/bug80602_4.phpt @@ -0,0 +1,33 @@ +--TEST-- +Bug #80602 (Segfault when using DOMChildNode::before()) - after text merge variation +--FILE-- +loadXML('foobar'); +$foo = $doc->firstChild->firstChild; +$bar = $doc->firstChild->lastChild; + +$foo->after($bar); + +var_dump($doc->saveXML()); + +$foo->nodeValue = "x"; + +var_dump($doc->saveXML()); + +$bar->nodeValue = "y"; + +var_dump($doc->saveXML()); + +?> +--EXPECT-- +string(43) " +foobar +" +string(41) " +xbar +" +string(39) " +xy +" diff --git a/ext/dom/tests/gh11288.phpt b/ext/dom/tests/gh11288.phpt new file mode 100644 index 0000000000000..f70bea80d9085 --- /dev/null +++ b/ext/dom/tests/gh11288.phpt @@ -0,0 +1,67 @@ +--TEST-- +GH-11288 (Error: Couldn't fetch DOMElement introduced in 8.2.6, 8.1.19) +--FILE-- + + +Loremipsum + +HTML; + +$dom = new DOMDocument(); +$dom->loadHTML($html); + +$spans = iterator_to_array($dom->getElementsByTagName('span')->getIterator()); +foreach ($spans as $span) { + if ('unwrap_me' === $span->getAttribute('class')) { + $fragment = $dom->createDocumentFragment(); + $fragment->append(...$span->childNodes); + $span->parentNode?->replaceChild($fragment, $span); + } +} + +var_dump(str_replace("\n", "", $dom->saveHTML())); + +$html = << + +Loremipsum + +HTML; + +$dom = new DOMDocument(); +$dom->loadHTML($html); + +$spans = iterator_to_array($dom->getElementsByTagName('span')->getIterator()); +foreach ($spans as $span) { + if ('unwrap_me' === $span->getAttribute('class')) { + $span->replaceWith(...$span->childNodes); + } +} + +var_dump(str_replace("\n", "", $dom->saveHTML())); + +$html = << + +Loremipsum + +HTML; + +$dom = new DOMDocument(); +$dom->loadHTML($html); + +$spans = iterator_to_array($dom->getElementsByTagName('span')->getIterator()); +foreach ($spans as $span) { + if ('unwrap_me' === $span->getAttribute('class')) { + $span->replaceWith('abc'); + } +} + +var_dump(str_replace("\n", "", $dom->saveHTML())); +?> +--EXPECT-- +string(108) "Loremipsum" +string(108) "Loremipsum" +string(44) "abc" diff --git a/ext/dom/tests/gh11289.phpt b/ext/dom/tests/gh11289.phpt new file mode 100644 index 0000000000000..7771a486bd66b --- /dev/null +++ b/ext/dom/tests/gh11289.phpt @@ -0,0 +1,28 @@ +--TEST-- +GH-11289 (DOMException: Not Found Error introduced in 8.2.6, 8.1.19) +--FILE-- + + + +
+ + +HTML; + +$dom = new DOMDocument(); +$dom->loadHTML($html); + +$divs = iterator_to_array($dom->getElementsByTagName('div')->getIterator()); +foreach ($divs as $div) { + $fragment = $dom->createDocumentFragment(); + $fragment->appendXML('

Hi!

'); + $div->replaceWith(...$fragment->childNodes); +} + +var_dump(str_replace("\n", "", $dom->saveHTML())); +?> +--EXPECT-- +string(55) "

Hi!

" diff --git a/ext/dom/tests/gh11290.phpt b/ext/dom/tests/gh11290.phpt new file mode 100644 index 0000000000000..2900720301041 --- /dev/null +++ b/ext/dom/tests/gh11290.phpt @@ -0,0 +1,27 @@ +--TEST-- +GH-11290 (DOMElement::replaceWith causes crash) +--FILE-- + + + +

Loremipsumdolor

+ + +HTML; + +$dom = new DOMDocument(); +$dom->loadHTML($html); + +$spans = iterator_to_array($dom->getElementsByTagName('span')->getIterator()); +foreach ($spans as $span) { + if ('unwrap_me' === $span->getAttribute('class')) { + $span->replaceWith(...$span->childNodes); + } +} + +var_dump(str_replace("\n", "", $dom->saveHTML())); +?> +--EXPECT-- +string(67) "

Loremipsumdolor

" diff --git a/ext/dom/tests/gh9142.phpt b/ext/dom/tests/gh9142.phpt new file mode 100644 index 0000000000000..f72dfa823f38c --- /dev/null +++ b/ext/dom/tests/gh9142.phpt @@ -0,0 +1,20 @@ +--TEST-- +GH-9142 (DOMChildNode replaceWith() double-free error when replacing elements not separated by any whitespace) +--FILE-- +OneTwo'; + +($dom = new DOMDocument('1.0', 'UTF-8'))->loadHTML($document); + +foreach ((new DOMXPath($dom))->query('//var') as $var) { + $var->replaceWith($dom->createElement('p', $var->nodeValue)); +} + +var_dump($dom->saveHTML()); + +?> +--EXPECT-- +string(154) " +

One

Two

+" From 5b033b0def799cf89e4f750f6100c2c72e8aa550 Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Fri, 26 May 2023 00:30:58 +0200 Subject: [PATCH 10/54] Fix zend_jit_stop_counter_handlers() performance issues with protect_memory=1 The function repeatedly calls mprotect() which is extremely slow. In our community build, the Laravel tests went from ~6 minutes to ~4 hours. This issue only occurs with opcache.protect_memory=1. Closes GH-11323 --- ext/opcache/jit/zend_jit_trace.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/ext/opcache/jit/zend_jit_trace.c b/ext/opcache/jit/zend_jit_trace.c index b0a86318a4ec8..b1fbc7b6518cc 100644 --- a/ext/opcache/jit/zend_jit_trace.c +++ b/ext/opcache/jit/zend_jit_trace.c @@ -7175,8 +7175,6 @@ static void zend_jit_stop_hot_trace_counters(zend_op_array *op_array) uint32_t i; jit_extension = (zend_jit_op_array_trace_extension*)ZEND_FUNC_INFO(op_array); - zend_shared_alloc_lock(); - SHM_UNPROTECT(); for (i = 0; i < op_array->last; i++) { /* Opline with Jit-ed code handler is skipped. */ if (jit_extension->trace_info[i].trace_flags & @@ -7188,8 +7186,6 @@ static void zend_jit_stop_hot_trace_counters(zend_op_array *op_array) op_array->opcodes[i].handler = jit_extension->trace_info[i].orig_handler; } } - SHM_PROTECT(); - zend_shared_alloc_unlock(); } /* Get the tracing op_array. */ @@ -7228,6 +7224,9 @@ static void zend_jit_stop_persistent_script(zend_persistent_script *script) /* Get all scripts which are accelerated by JIT */ static void zend_jit_stop_counter_handlers(void) { + zend_shared_alloc_lock(); + /* mprotect has an extreme overhead, avoid calls to it for every function. */ + SHM_UNPROTECT(); for (uint32_t i = 0; i < ZCSG(hash).max_num_entries; i++) { zend_accel_hash_entry *cache_entry; for (cache_entry = ZCSG(hash).hash_table[i]; cache_entry; cache_entry = cache_entry->next) { @@ -7237,6 +7236,8 @@ static void zend_jit_stop_counter_handlers(void) zend_jit_stop_persistent_script(script); } } + SHM_PROTECT(); + zend_shared_alloc_unlock(); } static void zend_jit_blacklist_root_trace(const zend_op *opline, size_t offset) From b5a07a7501f9cc43170c9675f20a1dcf34b360e5 Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Fri, 26 May 2023 11:37:01 +0200 Subject: [PATCH 11/54] [skip ci] Fix race condition in readline test The var_dump can be preceded by the "Interactive shell" log. The var_dump does not add much to the test anyway, so just remove it. --- ext/readline/tests/bug77812-readline.phpt | 2 -- 1 file changed, 2 deletions(-) diff --git a/ext/readline/tests/bug77812-readline.phpt b/ext/readline/tests/bug77812-readline.phpt index a18686781718b..a2d6c212c536a 100644 --- a/ext/readline/tests/bug77812-readline.phpt +++ b/ext/readline/tests/bug77812-readline.phpt @@ -13,7 +13,6 @@ $php = getenv('TEST_PHP_EXECUTABLE'); $ini = getenv('TEST_PHP_EXTRA_ARGS'); $descriptorspec = [['pipe', 'r'], STDOUT, STDERR]; $proc = proc_open("$php $ini -a", $descriptorspec, $pipes); -var_dump($proc); fwrite($pipes[0], "echo << --EXPECTF-- -resource(%d) of type (process) Interactive shell php > echo << Date: Thu, 25 May 2023 20:26:28 +0200 Subject: [PATCH 12/54] Fix GCC 12 compilation on riscv64 Close GH-11321 --- NEWS | 3 +++ configure.ac | 10 ++++++++++ 2 files changed, 13 insertions(+) diff --git a/NEWS b/NEWS index 93b363cfac79a..3b0d012995477 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,9 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? ????, PHP 8.1.21 +- Core: + . Fixed build for the riscv64 architecture/GCC 12. (Daniil Gentili) + - DOM: . Fixed bugs GH-11288 and GH-11289 and GH-11290 and GH-9142 (DOMExceptions and segfaults with replaceWith). (nielsdos) diff --git a/configure.ac b/configure.ac index 661df89a03de5..baf8651f044ad 100644 --- a/configure.ac +++ b/configure.ac @@ -365,6 +365,16 @@ if test "$ac_cv_func_dlopen" = "yes"; then fi AC_CHECK_LIB(m, sin) +case $host_alias in + riscv64*) + AC_CHECK_LIB(atomic, __atomic_exchange_1, [ + PHP_ADD_LIBRARY(atomic) + ], [ + AC_MSG_ERROR([Problem with enabling atomic. Please check config.log for details.]) + ]) + ;; +esac + dnl Check for inet_aton in -lc, -lbind and -lresolv. PHP_CHECK_FUNC(inet_aton, resolv, bind) From f249958cd3d83220d04409ce231bf77ff6d68b9b Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Sat, 27 May 2023 19:16:39 +0200 Subject: [PATCH 13/54] [skip ci] Add more patterns to run-tests.php retry list CURL: 404: Page Not Found IMAP: Can't create a temporary mailbox: [ALREADYEXISTS] Mailbox already exists Sockets: socket_bind(): Unable to bind address [98]: Address already in use --- run-tests.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/run-tests.php b/run-tests.php index 14eb45eecd609..63ae57a35c8c0 100755 --- a/run-tests.php +++ b/run-tests.php @@ -2877,7 +2877,7 @@ function run_test(string $php, $file, array $env): string function error_may_be_retried(string $output): bool { - return preg_match('((timed out)|(connection refused))i', $output) === 1; + return preg_match('((timed out)|(connection refused)|(404: page not found)|(address already in use)|(mailbox already exists))i', $output) === 1; } /** From c473787abb486c3f25bd551252b36d617267738f Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Fri, 6 Jan 2023 19:51:49 +0100 Subject: [PATCH 14/54] Fix GH-10234: Setting DOMAttr::textContent results in an empty attribute value We can't directly call xmlNodeSetContent, because it might encode the string through xmlStringLenGetNodeList for types XML_DOCUMENT_FRAG_NODE, XML_ELEMENT_NODE, XML_ATTRIBUTE_NODE. In these cases we need to use a text node to avoid the encoding. For the other cases, we *can* rely on xmlNodeSetContent because it is either a no-op, or handles the content without encoding and clears the properties field if needed. The test was taken from the issue report, for the test: Co-authored-by: ThomasWeinert Closes GH-10245. --- NEWS | 2 + ext/dom/node.c | 19 ++++++-- ext/dom/tests/gh10234.phpt | 93 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 110 insertions(+), 4 deletions(-) create mode 100644 ext/dom/tests/gh10234.phpt diff --git a/NEWS b/NEWS index 3b0d012995477..40fb3e328b7d1 100644 --- a/NEWS +++ b/NEWS @@ -8,6 +8,8 @@ PHP NEWS - DOM: . Fixed bugs GH-11288 and GH-11289 and GH-11290 and GH-9142 (DOMExceptions and segfaults with replaceWith). (nielsdos) + . Fixed bug GH-10234 (Setting DOMAttr::textContent results in an empty + attribute value). (nielsdos) - Opcache: . Fix allocation loop in zend_shared_alloc_startup(). (nielsdos) diff --git a/ext/dom/node.c b/ext/dom/node.c index 880c8cfe3e794..b291ccc99a308 100644 --- a/ext/dom/node.c +++ b/ext/dom/node.c @@ -769,17 +769,28 @@ int dom_node_text_content_write(dom_object *obj, zval *newval) return FAILURE; } - if (nodep->type == XML_ELEMENT_NODE || nodep->type == XML_ATTRIBUTE_NODE) { + const xmlChar *xmlChars = (const xmlChar *) ZSTR_VAL(str); + int type = nodep->type; + + /* We can't directly call xmlNodeSetContent, because it might encode the string through + * xmlStringLenGetNodeList for types XML_DOCUMENT_FRAG_NODE, XML_ELEMENT_NODE, XML_ATTRIBUTE_NODE. + * See tree.c:xmlNodeSetContent in libxml. + * In these cases we need to use a text node to avoid the encoding. + * For the other cases, we *can* rely on xmlNodeSetContent because it is either a no-op, or handles + * the content without encoding. */ + if (type == XML_DOCUMENT_FRAG_NODE || type == XML_ELEMENT_NODE || type == XML_ATTRIBUTE_NODE) { if (nodep->children) { node_list_unlink(nodep->children); php_libxml_node_free_list((xmlNodePtr) nodep->children); nodep->children = NULL; } + + xmlNode *textNode = xmlNewText(xmlChars); + xmlAddChild(nodep, textNode); + } else { + xmlNodeSetContent(nodep, xmlChars); } - /* we have to use xmlNodeAddContent() to get the same behavior as with xmlNewText() */ - xmlNodeSetContent(nodep, (xmlChar *) ""); - xmlNodeAddContent(nodep, (xmlChar *) ZSTR_VAL(str)); zend_string_release_ex(str, 0); return SUCCESS; diff --git a/ext/dom/tests/gh10234.phpt b/ext/dom/tests/gh10234.phpt new file mode 100644 index 0000000000000..5edc8fc6c1ff1 --- /dev/null +++ b/ext/dom/tests/gh10234.phpt @@ -0,0 +1,93 @@ +--TEST-- +GH-10234 (Setting DOMAttr::textContent results in an empty attribute value.) +--EXTENSIONS-- +dom +--FILE-- +loadXML(''); +$attribute = $document->documentElement->getAttributeNode('attribute'); + +echo "-- Attribute tests --\n"; + +var_dump($document->saveHTML()); +var_dump($attribute->textContent); + +$attribute->textContent = 'new value'; +var_dump($attribute->textContent); +var_dump($document->saveHTML()); + +$attribute->textContent = 'hello & world'; +var_dump($attribute->textContent); +var_dump($document->saveHTML()); + +$attribute->textContent = 'hi'; +var_dump($attribute->textContent); +var_dump($document->saveHTML()); + +$attribute->textContent = 'quote "test"'; +var_dump($attribute->textContent); +var_dump($document->saveHTML()); + +$attribute->textContent = "quote 'test'"; +var_dump($attribute->textContent); +var_dump($document->saveHTML()); + +$attribute->textContent = "quote '\"test\"'"; +var_dump($attribute->textContent); +var_dump($document->saveHTML()); + +echo "-- Document element tests --\n"; + +$document->documentElement->textContent = 'hello & world'; +var_dump($document->documentElement->textContent); +var_dump($document->saveHTML()); + +$document->documentElement->textContent = 'hi'; +var_dump($document->documentElement->textContent); +var_dump($document->saveHTML()); + +$document->documentElement->textContent = 'quote "test"'; +var_dump($document->documentElement->textContent); +var_dump($document->saveHTML()); + +$document->documentElement->textContent = "quote 'test'"; +var_dump($document->documentElement->textContent); +var_dump($document->saveHTML()); +?> +--EXPECT-- +-- Attribute tests -- +string(38) " +" +string(5) "value" +string(9) "new value" +string(42) " +" +string(13) "hello & world" +string(50) " +" +string(9) "hi" +string(54) " +" +string(12) "quote "test"" +string(45) " +" +string(12) "quote 'test'" +string(45) " +" +string(14) "quote '"test"'" +string(57) " +" +-- Document element tests -- +string(13) "hello & world" +string(74) "hello & world +" +string(9) "hi" +string(78) "<b>hi</b> +" +string(12) "quote "test"" +string(69) "quote "test" +" +string(12) "quote 'test'" +string(69) "quote 'test' +" From 761b9a44f8f097f77d0f96d479c485e9b11e51d6 Mon Sep 17 00:00:00 2001 From: divinity76 Date: Mon, 29 May 2023 16:53:00 +0200 Subject: [PATCH 15/54] Fix return value in stub file for DOMNodeList::item Not explicitly documenting the possibility of returning DOMElement causes the Intelephense linter (a popular PHP linter with ~9 million downloads: https://marketplace.visualstudio.com/items?itemName=bmewburn.vscode-intelephense-client) to think this code is bad: $xp->query("whatever")->item(0)->getAttribute("foo"); DOMNode does not have getAttribute (while DOMElement does). Documenting the DOMElement return type should fix Intelephense's linter. Closes GH-11342. --- NEWS | 1 + ext/dom/php_dom.stub.php | 2 +- ext/dom/php_dom_arginfo.h | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 40fb3e328b7d1..143418e2a30d2 100644 --- a/NEWS +++ b/NEWS @@ -10,6 +10,7 @@ PHP NEWS and segfaults with replaceWith). (nielsdos) . Fixed bug GH-10234 (Setting DOMAttr::textContent results in an empty attribute value). (nielsdos) + . Fix return value in stub file for DOMNodeList::item. (divinity76) - Opcache: . Fix allocation loop in zend_shared_alloc_startup(). (nielsdos) diff --git a/ext/dom/php_dom.stub.php b/ext/dom/php_dom.stub.php index f26518c0ba8ec..45b54c21d6c2c 100644 --- a/ext/dom/php_dom.stub.php +++ b/ext/dom/php_dom.stub.php @@ -232,7 +232,7 @@ public function count(): int {} public function getIterator(): Iterator {} - /** @return DOMNode|DOMNameSpaceNode|null */ + /** @return DOMElement|DOMNode|DOMNameSpaceNode|null */ public function item(int $index) {} } diff --git a/ext/dom/php_dom_arginfo.h b/ext/dom/php_dom_arginfo.h index 2ac8ae45f2b26..d63b43e9b95f8 100644 --- a/ext/dom/php_dom_arginfo.h +++ b/ext/dom/php_dom_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 74698bea9c5e0635cf91345e8512b9677489510c */ + * Stub hash: a62e383b05df81ea245a7993215fb8ff4e1c7f9d */ ZEND_BEGIN_ARG_WITH_RETURN_OBJ_INFO_EX(arginfo_dom_import_simplexml, 0, 1, DOMElement, 0) ZEND_ARG_TYPE_INFO(0, node, IS_OBJECT, 0) From bce536067c803c47e33508b5c85798e0ba038d46 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Mon, 29 May 2023 16:16:01 +0200 Subject: [PATCH 16/54] Fix GH-11338: SplFileInfo empty getBasename with more than one slash Regressed in 13e4ce386bb7. Closes GH-11340. --- NEWS | 4 ++++ ext/spl/spl_directory.c | 4 +++- ext/spl/tests/gh11338.phpt | 47 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 ext/spl/tests/gh11338.phpt diff --git a/NEWS b/NEWS index 143418e2a30d2..e587b833339e6 100644 --- a/NEWS +++ b/NEWS @@ -16,6 +16,10 @@ PHP NEWS . Fix allocation loop in zend_shared_alloc_startup(). (nielsdos) . Access violation on smm_shared_globals with ALLOC_FALLBACK. (KoudelkaB) +- SPL: + . Fixed bug GH-11338 (SplFileInfo empty getBasename with more than one + slash). (nielsdos) + - Standard: . Fix access on NULL pointer in array_merge_recursive(). (ilutov) . Fix exception handling in array_multisort(). (ilutov) diff --git a/ext/spl/spl_directory.c b/ext/spl/spl_directory.c index aefa2aa933e51..b00a1e66568e0 100644 --- a/ext/spl/spl_directory.c +++ b/ext/spl/spl_directory.c @@ -432,7 +432,9 @@ static void spl_filesystem_info_set_filename(spl_filesystem_object *intern, zend path_len = ZSTR_LEN(path); if (path_len > 1 && IS_SLASH_AT(ZSTR_VAL(path), path_len-1)) { - path_len--; + do { + path_len--; + } while (path_len > 1 && IS_SLASH_AT(ZSTR_VAL(path), path_len - 1)); intern->file_name = zend_string_init(ZSTR_VAL(path), path_len, 0); } else { intern->file_name = zend_string_copy(path); diff --git a/ext/spl/tests/gh11338.phpt b/ext/spl/tests/gh11338.phpt new file mode 100644 index 0000000000000..0a59cea9e7468 --- /dev/null +++ b/ext/spl/tests/gh11338.phpt @@ -0,0 +1,47 @@ +--TEST-- +GH-11338 (SplFileInfo empty getBasename with more than on slash) +--FILE-- +getBasename()); + var_dump($file->getFilename()); +} + +test('/dir/anotherdir/basedir//'); +test('/dir/anotherdir/basedir/'); +test('/dir/anotherdir/basedir'); +test('/dir/anotherdir//basedir'); +test('///'); +test('//'); +test('/'); +test(''); + +?> +--EXPECT-- +Testing: '/dir/anotherdir/basedir//' +string(7) "basedir" +string(7) "basedir" +Testing: '/dir/anotherdir/basedir/' +string(7) "basedir" +string(7) "basedir" +Testing: '/dir/anotherdir/basedir' +string(7) "basedir" +string(7) "basedir" +Testing: '/dir/anotherdir//basedir' +string(7) "basedir" +string(7) "basedir" +Testing: '///' +string(0) "" +string(1) "/" +Testing: '//' +string(0) "" +string(1) "/" +Testing: '/' +string(0) "" +string(1) "/" +Testing: '' +string(0) "" +string(0) "" From 9c59d22a7bb4dd0b4cd0b138e6cea12686c1868d Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Mon, 29 May 2023 16:39:58 +0200 Subject: [PATCH 17/54] Fix GH-11336: php still tries to unlock the shared memory ZendSem with opcache.file_cache_only=1 but it was never locked I chose to check for the value of lock_file instead of checking the file_cache_only, because it is probably a little bit faster and we're going to access the lock_file variable anyway. It's also more generic. Closes GH-11341. --- NEWS | 2 ++ ext/opcache/ZendAccelerator.c | 4 ++++ ext/opcache/zend_shared_alloc.c | 2 +- 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index e587b833339e6..a0f4c4b10fe25 100644 --- a/NEWS +++ b/NEWS @@ -15,6 +15,8 @@ PHP NEWS - Opcache: . Fix allocation loop in zend_shared_alloc_startup(). (nielsdos) . Access violation on smm_shared_globals with ALLOC_FALLBACK. (KoudelkaB) + . Fixed bug GH-11336 (php still tries to unlock the shared memory ZendSem + with opcache.file_cache_only=1 but it was never locked). (nielsdos) - SPL: . Fixed bug GH-11338 (SplFileInfo empty getBasename with more than one diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c index 97b82378780b8..ed0602394ce97 100644 --- a/ext/opcache/ZendAccelerator.c +++ b/ext/opcache/ZendAccelerator.c @@ -354,6 +354,10 @@ static inline void accel_unlock_all(void) #ifdef ZEND_WIN32 accel_deactivate_sub(); #else + if (lock_file == -1) { + return; + } + struct flock mem_usage_unlock_all; mem_usage_unlock_all.l_type = F_UNLCK; diff --git a/ext/opcache/zend_shared_alloc.c b/ext/opcache/zend_shared_alloc.c index afe539bf987a7..37f6fea9199a7 100644 --- a/ext/opcache/zend_shared_alloc.c +++ b/ext/opcache/zend_shared_alloc.c @@ -52,7 +52,7 @@ zend_smm_shared_globals *smm_shared_globals; #ifdef ZTS static MUTEX_T zts_lock; #endif -int lock_file; +int lock_file = -1; static char lockfile_name[MAXPATHLEN]; #endif From 154c2510135bf7d4b96374fb34c7c0aa0410e143 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Mon, 29 May 2023 18:36:02 +0200 Subject: [PATCH 18/54] Fix spec compliance error for DOMDocument::getElementsByTagNameNS Spec link: https://dom.spec.whatwg.org/#concept-getelementsbytagnamens Spec says we should match any namespace when '*' is provided. This was however not the case: elements that didn't have a namespace were not returned. This patch fixes the error by modifying the namespace check. Closes GH-11343. --- NEWS | 2 + ext/dom/php_dom.c | 7 +- ...ementsByTagNameNS_match_any_namespace.phpt | 82 +++++++++++++++++++ 3 files changed, 90 insertions(+), 1 deletion(-) create mode 100644 ext/dom/tests/DOMDocument_getElementsByTagNameNS_match_any_namespace.phpt diff --git a/NEWS b/NEWS index a0f4c4b10fe25..1b40b1bceb49b 100644 --- a/NEWS +++ b/NEWS @@ -11,6 +11,8 @@ PHP NEWS . Fixed bug GH-10234 (Setting DOMAttr::textContent results in an empty attribute value). (nielsdos) . Fix return value in stub file for DOMNodeList::item. (divinity76) + . Fix spec compliance error with '*' namespace for + DOMDocument::getElementsByTagNameNS. (nielsdos) - Opcache: . Fix allocation loop in zend_shared_alloc_startup(). (nielsdos) diff --git a/ext/dom/php_dom.c b/ext/dom/php_dom.c index 01a206c0985bd..1883767d2e48b 100644 --- a/ext/dom/php_dom.c +++ b/ext/dom/php_dom.c @@ -1270,10 +1270,15 @@ xmlNode *dom_get_elements_by_tag_name_ns_raw(xmlNodePtr nodep, char *ns, char *l { xmlNodePtr ret = NULL; + /* Note: The spec says that ns == '' must be transformed to ns == NULL. In other words, they are equivalent. + * PHP however does not do this and internally uses the empty string everywhere when the user provides ns == NULL. + * This is because for PHP ns == NULL has another meaning: "match every namespace" instead of "match the empty namespace". */ + bool ns_match_any = ns == NULL || (ns[0] == '*' && ns[1] == '\0'); + while (nodep != NULL && (*cur <= index || index == -1)) { if (nodep->type == XML_ELEMENT_NODE) { if (xmlStrEqual(nodep->name, (xmlChar *)local) || xmlStrEqual((xmlChar *)"*", (xmlChar *)local)) { - if (ns == NULL || (!strcmp(ns, "") && nodep->ns == NULL) || (nodep->ns != NULL && (xmlStrEqual(nodep->ns->href, (xmlChar *)ns) || xmlStrEqual((xmlChar *)"*", (xmlChar *)ns)))) { + if (ns_match_any || (!strcmp(ns, "") && nodep->ns == NULL) || (nodep->ns != NULL && xmlStrEqual(nodep->ns->href, (xmlChar *)ns))) { if (*cur == index) { ret = nodep; break; diff --git a/ext/dom/tests/DOMDocument_getElementsByTagNameNS_match_any_namespace.phpt b/ext/dom/tests/DOMDocument_getElementsByTagNameNS_match_any_namespace.phpt new file mode 100644 index 0000000000000..888d1ef9b8057 --- /dev/null +++ b/ext/dom/tests/DOMDocument_getElementsByTagNameNS_match_any_namespace.phpt @@ -0,0 +1,82 @@ +--TEST-- +DOMDocument::getElementsByTagNameNS() match any namespace +--EXTENSIONS-- +dom +--FILE-- + + +Books of the other guy.. + + + + xinclude: book.xml not found + + + + This is another namespace + + + +EOD; +$dom = new DOMDocument; + +// load the XML string defined above +$dom->loadXML($xml); + +function test($namespace, $local) { + global $dom; + $namespace_str = $namespace !== NULL ? "'$namespace'" : "null"; + echo "-- getElementsByTagNameNS($namespace_str, '$local') --\n"; + foreach ($dom->getElementsByTagNameNS($namespace, $local) as $element) { + echo 'local name: \'', $element->localName, '\', prefix: \'', $element->prefix, "'\n"; + } +} + +// Should *also* include objects even without a namespace +test(null, '*'); +// Should *also* include objects even without a namespace +test('*', '*'); +// Should *only* include objects without a namespace +test('', '*'); +// Should *only* include objects with the specified namespace +test('http://www.w3.org/2001/XInclude', '*'); +// Should not give any output +test('', 'fallback'); +// Should not give any output, because the null namespace is the same as the empty namespace +test(null, 'fallback'); +// Should only output the include from the empty namespace +test(null, 'include'); + +?> +--EXPECT-- +-- getElementsByTagNameNS(null, '*') -- +local name: 'chapter', prefix: '' +local name: 'title', prefix: '' +local name: 'para', prefix: '' +local name: 'error', prefix: '' +local name: 'include', prefix: '' +-- getElementsByTagNameNS('*', '*') -- +local name: 'chapter', prefix: '' +local name: 'title', prefix: '' +local name: 'para', prefix: '' +local name: 'include', prefix: 'xi' +local name: 'fallback', prefix: 'xi' +local name: 'error', prefix: '' +local name: 'include', prefix: '' +-- getElementsByTagNameNS('', '*') -- +local name: 'chapter', prefix: '' +local name: 'title', prefix: '' +local name: 'para', prefix: '' +local name: 'error', prefix: '' +local name: 'include', prefix: '' +-- getElementsByTagNameNS('http://www.w3.org/2001/XInclude', '*') -- +local name: 'include', prefix: 'xi' +local name: 'fallback', prefix: 'xi' +-- getElementsByTagNameNS('', 'fallback') -- +-- getElementsByTagNameNS(null, 'fallback') -- +-- getElementsByTagNameNS(null, 'include') -- +local name: 'include', prefix: '' From b374ec399d85d2f7051b6f92324715e76b9b11ed Mon Sep 17 00:00:00 2001 From: nielsdos <7771979+nielsdos@users.noreply.github.com> Date: Mon, 29 May 2023 22:29:50 +0200 Subject: [PATCH 19/54] Fix DOMElement::append() and DOMElement::prepend() hierarchy checks We could end up in an invalid hierarchy, resulting in infinite loops and eventual crashes if we don't check for the DOM hierarchy validity. Closes GH-11344. --- NEWS | 2 + ext/dom/parentnode.c | 28 ++++++ .../DOMElement_append_hierarchy_test.phpt | 89 +++++++++++++++++++ .../DOMElement_prepend_hierarchy_test.phpt | 89 +++++++++++++++++++ 4 files changed, 208 insertions(+) create mode 100644 ext/dom/tests/DOMElement_append_hierarchy_test.phpt create mode 100644 ext/dom/tests/DOMElement_prepend_hierarchy_test.phpt diff --git a/NEWS b/NEWS index 1b40b1bceb49b..42c54e587de16 100644 --- a/NEWS +++ b/NEWS @@ -13,6 +13,8 @@ PHP NEWS . Fix return value in stub file for DOMNodeList::item. (divinity76) . Fix spec compliance error with '*' namespace for DOMDocument::getElementsByTagNameNS. (nielsdos) + . Fix DOMElement::append() and DOMElement::prepend() hierarchy checks. + (nielsdos) - Opcache: . Fix allocation loop in zend_shared_alloc_startup(). (nielsdos) diff --git a/ext/dom/parentnode.c b/ext/dom/parentnode.c index 46c90a13e31d5..c99a2a5a6622a 100644 --- a/ext/dom/parentnode.c +++ b/ext/dom/parentnode.c @@ -255,10 +255,33 @@ static void dom_fragment_assign_parent_node(xmlNodePtr parentNode, xmlNodePtr fr fragment->last = NULL; } +static zend_result dom_hierarchy_node_list(xmlNodePtr parentNode, zval *nodes, int nodesc) +{ + for (int i = 0; i < nodesc; i++) { + if (Z_TYPE(nodes[i]) == IS_OBJECT) { + const zend_class_entry *ce = Z_OBJCE(nodes[i]); + + if (instanceof_function(ce, dom_node_class_entry)) { + if (dom_hierarchy(parentNode, dom_object_get_node(Z_DOMOBJ_P(nodes + i))) != SUCCESS) { + return FAILURE; + } + } + } + } + + return SUCCESS; +} + void dom_parent_node_append(dom_object *context, zval *nodes, int nodesc) { xmlNode *parentNode = dom_object_get_node(context); xmlNodePtr newchild, prevsib; + + if (UNEXPECTED(dom_hierarchy_node_list(parentNode, nodes, nodesc) != SUCCESS)) { + php_dom_throw_error(HIERARCHY_REQUEST_ERR, dom_get_strict_error(context->document)); + return; + } + xmlNode *fragment = dom_zvals_to_fragment(context->document, parentNode, nodes, nodesc); if (fragment == NULL) { @@ -296,6 +319,11 @@ void dom_parent_node_prepend(dom_object *context, zval *nodes, int nodesc) return; } + if (UNEXPECTED(dom_hierarchy_node_list(parentNode, nodes, nodesc) != SUCCESS)) { + php_dom_throw_error(HIERARCHY_REQUEST_ERR, dom_get_strict_error(context->document)); + return; + } + xmlNodePtr newchild, nextsib; xmlNode *fragment = dom_zvals_to_fragment(context->document, parentNode, nodes, nodesc); diff --git a/ext/dom/tests/DOMElement_append_hierarchy_test.phpt b/ext/dom/tests/DOMElement_append_hierarchy_test.phpt new file mode 100644 index 0000000000000..2d70b10fe9f70 --- /dev/null +++ b/ext/dom/tests/DOMElement_append_hierarchy_test.phpt @@ -0,0 +1,89 @@ +--TEST-- +DOMElement::append() with hierarchy changes and errors +--EXTENSIONS-- +dom +--FILE-- +loadXML('

helloworld

'); + +echo "-- Append hello with world --\n"; +$dom = clone $dom_original; +$b_hello = $dom->firstChild->firstChild; +$b_world = $b_hello->nextSibling; +$b_hello->append($b_world); +var_dump($dom->saveHTML()); + +echo "-- Append hello with world's child --\n"; +$dom = clone $dom_original; +$b_hello = $dom->firstChild->firstChild; +$b_world = $b_hello->nextSibling; +$b_hello->append($b_world->firstChild); +var_dump($dom->saveHTML()); + +echo "-- Append world's child with hello --\n"; +$dom = clone $dom_original; +$b_hello = $dom->firstChild->firstChild; +$b_world = $b_hello->nextSibling; +$b_world->firstChild->append($b_hello); +var_dump($dom->saveHTML()); + +echo "-- Append hello with itself --\n"; +$dom = clone $dom_original; +$b_hello = $dom->firstChild->firstChild; +try { + $b_hello->append($b_hello); +} catch (\DOMException $e) { + echo $e->getMessage(), "\n"; +} +var_dump($dom->saveHTML()); + +echo "-- Append world's i tag with the parent --\n"; +$dom = clone $dom_original; +$b_hello = $dom->firstChild->firstChild; +$b_world = $b_hello->nextSibling; +try { + $b_world->firstChild->append($b_world); +} catch (\DOMException $e) { + echo $e->getMessage(), "\n"; +} +var_dump($dom->saveHTML()); + +echo "-- Append from another document --\n"; +$dom = clone $dom_original; +$dom2 = new DOMDocument; +$dom2->loadXML('

other

'); +try { + $dom->firstChild->firstChild->prepend($dom2->firstChild); +} catch (\DOMException $e) { + echo $e->getMessage(), "\n"; +} +var_dump($dom2->saveHTML()); +var_dump($dom->saveHTML()); + +?> +--EXPECT-- +-- Append hello with world -- +string(39) "

helloworld

+" +-- Append hello with world's child -- +string(39) "

helloworld

+" +-- Append world's child with hello -- +string(39) "

worldhello

+" +-- Append hello with itself -- +Hierarchy Request Error +string(39) "

helloworld

+" +-- Append world's i tag with the parent -- +Hierarchy Request Error +string(39) "

helloworld

+" +-- Append from another document -- +Wrong Document Error +string(13) "

other

+" +string(39) "

helloworld

+" diff --git a/ext/dom/tests/DOMElement_prepend_hierarchy_test.phpt b/ext/dom/tests/DOMElement_prepend_hierarchy_test.phpt new file mode 100644 index 0000000000000..4d9cf24a61828 --- /dev/null +++ b/ext/dom/tests/DOMElement_prepend_hierarchy_test.phpt @@ -0,0 +1,89 @@ +--TEST-- +DOMElement::prepend() with hierarchy changes and errors +--EXTENSIONS-- +dom +--FILE-- +loadXML('

helloworld

'); + +echo "-- Prepend hello with world --\n"; +$dom = clone $dom_original; +$b_hello = $dom->firstChild->firstChild; +$b_world = $b_hello->nextSibling; +$b_hello->prepend($b_world); +var_dump($dom->saveHTML()); + +echo "-- Prepend hello with world's child --\n"; +$dom = clone $dom_original; +$b_hello = $dom->firstChild->firstChild; +$b_world = $b_hello->nextSibling; +$b_hello->prepend($b_world->firstChild); +var_dump($dom->saveHTML()); + +echo "-- Prepend world's child with hello --\n"; +$dom = clone $dom_original; +$b_hello = $dom->firstChild->firstChild; +$b_world = $b_hello->nextSibling; +$b_world->firstChild->prepend($b_hello); +var_dump($dom->saveHTML()); + +echo "-- Prepend hello with itself --\n"; +$dom = clone $dom_original; +$b_hello = $dom->firstChild->firstChild; +try { + $b_hello->prepend($b_hello); +} catch (\DOMException $e) { + echo $e->getMessage(), "\n"; +} +var_dump($dom->saveHTML()); + +echo "-- Prepend world's i tag with the parent --\n"; +$dom = clone $dom_original; +$b_hello = $dom->firstChild->firstChild; +$b_world = $b_hello->nextSibling; +try { + $b_world->firstChild->prepend($b_world); +} catch (\DOMException $e) { + echo $e->getMessage(), "\n"; +} +var_dump($dom->saveHTML()); + +echo "-- Append from another document --\n"; +$dom = clone $dom_original; +$dom2 = new DOMDocument; +$dom2->loadXML('

other

'); +try { + $dom->firstChild->firstChild->prepend($dom2->firstChild); +} catch (\DOMException $e) { + echo $e->getMessage(), "\n"; +} +var_dump($dom2->saveHTML()); +var_dump($dom->saveHTML()); + +?> +--EXPECT-- +-- Prepend hello with world -- +string(39) "

worldhello

+" +-- Prepend hello with world's child -- +string(39) "

worldhello

+" +-- Prepend world's child with hello -- +string(39) "

helloworld

+" +-- Prepend hello with itself -- +Hierarchy Request Error +string(39) "

helloworld

+" +-- Prepend world's i tag with the parent -- +Hierarchy Request Error +string(39) "

helloworld

+" +-- Append from another document -- +Wrong Document Error +string(13) "

other

+" +string(39) "

helloworld

+" From c50172e8121246cd11df768dae605129a3b3b19c Mon Sep 17 00:00:00 2001 From: Yuya Hamada Date: Mon, 29 May 2023 11:22:44 +0900 Subject: [PATCH 20/54] Fix mb_strlen is wrong length for CP932 when 0x80. --- ext/mbstring/libmbfl/filters/mbfilter_cp932.c | 4 ++-- ext/mbstring/tests/mb_strlen.phpt | 8 ++++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/ext/mbstring/libmbfl/filters/mbfilter_cp932.c b/ext/mbstring/libmbfl/filters/mbfilter_cp932.c index 54f93f91fe207..c0732b7cf92be 100644 --- a/ext/mbstring/libmbfl/filters/mbfilter_cp932.c +++ b/ext/mbstring/libmbfl/filters/mbfilter_cp932.c @@ -65,7 +65,7 @@ static int mbfl_filt_conv_cp932_wchar_flush(mbfl_convert_filter *filter); -static const unsigned char mblen_table_sjis[] = { /* 0x80-0x9f,0xE0-0xFF */ +static const unsigned char mblen_table_sjis[] = { /* 0x81-0x9f,0xE0-0xFF */ 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, @@ -74,7 +74,7 @@ static const unsigned char mblen_table_sjis[] = { /* 0x80-0x9f,0xE0-0xFF */ 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, - 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, + 1, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, diff --git a/ext/mbstring/tests/mb_strlen.phpt b/ext/mbstring/tests/mb_strlen.phpt index 5ebfcd1aec065..81cacaf197763 100644 --- a/ext/mbstring/tests/mb_strlen.phpt +++ b/ext/mbstring/tests/mb_strlen.phpt @@ -35,6 +35,11 @@ print "-- Testing illegal bytes 0x80,0xFD-FF --\n"; print mb_strlen("\x80\xA1", 'SJIS') . "\n"; print mb_strlen("abc\xFD\xFE\xFF", 'SJIS') . "\n"; +echo "== CP932 ==\n"; +print mb_strlen("\x80\xA1", "CP932") . "\n"; +// 0xFD, 0xFE, 0xFF is reserved. +print mb_strlen("abc\xFD\xFE\xFF", 'CP932') . "\n"; + echo "== MacJapanese ==\n"; print mb_strlen("\x80\xA1", 'MacJapanese') . "\n"; print mb_strlen("abc\xFD\xFE\xFF", 'MacJapanese') . "\n"; @@ -91,6 +96,9 @@ try { -- Testing illegal bytes 0x80,0xFD-FF -- 2 6 +== CP932 == +2 +5 == MacJapanese == 2 6 From c6ae7a55b75f67a23b3b25c4c1573b7884b1ff4d Mon Sep 17 00:00:00 2001 From: James Lucas Date: Tue, 16 May 2023 10:37:42 +1000 Subject: [PATCH 21/54] Fix bug GH-11246 cli/get_set_process_title Fail to clobber_error only when the argv is a non-contiguous area Don't increment the end_of_error if a non-contiguous area is encountered in environ Closes GH-11247 --- NEWS | 4 ++++ sapi/cli/ps_title.c | 15 ++++++++------- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/NEWS b/NEWS index 42c54e587de16..894d23c2375ae 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,10 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? ????, PHP 8.1.21 +- CLI: + . Fixed bug GH-11246 (cli/get_set_process_title fails on MacOS). + (James Lucas) + - Core: . Fixed build for the riscv64 architecture/GCC 12. (Daniil Gentili) diff --git a/sapi/cli/ps_title.c b/sapi/cli/ps_title.c index 7b00348be67cc..01a8d05c4c1e9 100644 --- a/sapi/cli/ps_title.c +++ b/sapi/cli/ps_title.c @@ -167,19 +167,20 @@ char** save_ps_args(int argc, char** argv) end_of_area = argv[i] + strlen(argv[i]); } + if (non_contiguous_area != 0) { + goto clobber_error; + } + /* * check for contiguous environ strings following argv */ - for (i = 0; (non_contiguous_area == 0) && (environ[i] != NULL); i++) + for (i = 0; environ[i] != NULL; i++) { - if (end_of_area + 1 != environ[i]) - non_contiguous_area = 1; - end_of_area = environ[i] + strlen(environ[i]); + if (end_of_area + 1 == environ[i]) { + end_of_area = environ[i] + strlen(environ[i]); + } } - if (non_contiguous_area != 0) - goto clobber_error; - ps_buffer = argv[0]; ps_buffer_size = end_of_area - argv[0]; From 781277210553b9276939dc899790f39b93290268 Mon Sep 17 00:00:00 2001 From: nielsdos <7771979+nielsdos@users.noreply.github.com> Date: Tue, 30 May 2023 17:20:04 +0200 Subject: [PATCH 22/54] Fix GH-11347: Memory leak when calling a static method inside an xpath query It's a type confusion bug. `zend_make_callable` may change the function name of the fci to become an array, causing a crash in debug mode on `zval_ptr_dtor_str(&fci.function_name);` in `dom_xpath_ext_function_php`. On a production build it doesn't crash but only causes a leak, because the array elements are not destroyed, only the array container itself is. We can use the nogc variant because it cannot contain cycles, the potential array can only contain 2 strings. Closes GH-11350. --- NEWS | 2 ++ ext/dom/tests/gh11347.phpt | 26 ++++++++++++++++++++++++++ ext/dom/xpath.c | 2 +- 3 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 ext/dom/tests/gh11347.phpt diff --git a/NEWS b/NEWS index 894d23c2375ae..f2cc5b1be96a1 100644 --- a/NEWS +++ b/NEWS @@ -19,6 +19,8 @@ PHP NEWS DOMDocument::getElementsByTagNameNS. (nielsdos) . Fix DOMElement::append() and DOMElement::prepend() hierarchy checks. (nielsdos) + . Fixed bug GH-11347 (Memory leak when calling a static method inside an + xpath query). (nielsdos) - Opcache: . Fix allocation loop in zend_shared_alloc_startup(). (nielsdos) diff --git a/ext/dom/tests/gh11347.phpt b/ext/dom/tests/gh11347.phpt new file mode 100644 index 0000000000000..189231f925081 --- /dev/null +++ b/ext/dom/tests/gh11347.phpt @@ -0,0 +1,26 @@ +--TEST-- +GH-11347 (Memory leak when calling a static method inside an xpath query) +--EXTENSIONS-- +dom +--FILE-- +loadHTML('hello'); +$xpath = new DOMXpath($doc); +$xpath->registerNamespace("php", "http://php.net/xpath"); +$xpath->registerPHPFunctions(); +$xpath->query("//a[php:function('MyClass::dump', string(@href))]"); + +?> +Done +--EXPECT-- +string(15) "https://php.net/" +Done diff --git a/ext/dom/xpath.c b/ext/dom/xpath.c index 876d8b00dae0e..f546733a436d1 100644 --- a/ext/dom/xpath.c +++ b/ext/dom/xpath.c @@ -182,7 +182,7 @@ static void dom_xpath_ext_function_php(xmlXPathParserContextPtr ctxt, int nargs, } cleanup: zend_string_release_ex(callable, 0); - zval_ptr_dtor_str(&fci.function_name); + zval_ptr_dtor_nogc(&fci.function_name); if (fci.param_count > 0) { for (i = 0; i < nargs - 1; i++) { zval_ptr_dtor(&fci.params[i]); From b1d8e240e688cae810c83b364772bf140ac45f42 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sat, 3 Jun 2023 16:41:44 +0200 Subject: [PATCH 23/54] Fix bug #67440: append_node of a DOMDocumentFragment does not reconcile namespaces The test was amended from the original issue report. For the test: Co-authored-by: php@deep-freeze.ca The problem is that the regular dom_reconcile_ns() only works on a single node. We actually have to reconciliate the whole tree in case a fragment was added. This also required to move some code around such that this special case could be handled separately. Closes GH-11362. --- NEWS | 2 + ext/dom/node.c | 66 ++++++++++------ ext/dom/parentnode.c | 18 +++-- ext/dom/php_dom.c | 75 +++++++++++++----- ext/dom/php_dom.h | 1 + ext/dom/tests/bug67440.phpt | 151 ++++++++++++++++++++++++++++++++++++ 6 files changed, 264 insertions(+), 49 deletions(-) create mode 100644 ext/dom/tests/bug67440.phpt diff --git a/NEWS b/NEWS index f2cc5b1be96a1..8395e5233c864 100644 --- a/NEWS +++ b/NEWS @@ -21,6 +21,8 @@ PHP NEWS (nielsdos) . Fixed bug GH-11347 (Memory leak when calling a static method inside an xpath query). (nielsdos) + . Fixed bug #67440 (append_node of a DOMDocumentFragment does not reconcile + namespaces). (nielsdos) - Opcache: . Fix allocation loop in zend_shared_alloc_startup(). (nielsdos) diff --git a/ext/dom/node.c b/ext/dom/node.c index b291ccc99a308..bcf4ee487d38d 100644 --- a/ext/dom/node.c +++ b/ext/dom/node.c @@ -943,12 +943,20 @@ PHP_METHOD(DOMNode, insertBefore) return; } } + new_child = xmlAddPrevSibling(refp, child); + if (UNEXPECTED(NULL == new_child)) { + goto cannot_add; + } } else if (child->type == XML_DOCUMENT_FRAG_NODE) { + xmlNodePtr last = child->last; new_child = _php_dom_insert_fragment(parentp, refp->prev, refp, child, intern, childobj); - } - - if (new_child == NULL) { + dom_reconcile_ns_list(parentp->doc, new_child, last); + } else { new_child = xmlAddPrevSibling(refp, child); + if (UNEXPECTED(NULL == new_child)) { + goto cannot_add; + } + dom_reconcile_ns(parentp->doc, new_child); } } else { if (child->parent != NULL){ @@ -985,23 +993,28 @@ PHP_METHOD(DOMNode, insertBefore) return; } } + new_child = xmlAddChild(parentp, child); + if (UNEXPECTED(NULL == new_child)) { + goto cannot_add; + } } else if (child->type == XML_DOCUMENT_FRAG_NODE) { + xmlNodePtr last = child->last; new_child = _php_dom_insert_fragment(parentp, parentp->last, NULL, child, intern, childobj); - } - if (new_child == NULL) { + dom_reconcile_ns_list(parentp->doc, new_child, last); + } else { new_child = xmlAddChild(parentp, child); + if (UNEXPECTED(NULL == new_child)) { + goto cannot_add; + } + dom_reconcile_ns(parentp->doc, new_child); } } - if (NULL == new_child) { - zend_throw_error(NULL, "Cannot add newnode as the previous sibling of refnode"); - RETURN_THROWS(); - } - - dom_reconcile_ns(parentp->doc, new_child); - DOM_RET_OBJ(new_child, &ret, intern); - + return; +cannot_add: + zend_throw_error(NULL, "Cannot add newnode as the previous sibling of refnode"); + RETURN_THROWS(); } /* }}} end dom_node_insert_before */ @@ -1066,9 +1079,10 @@ PHP_METHOD(DOMNode, replaceChild) xmlUnlinkNode(oldchild); + xmlNodePtr last = newchild->last; newchild = _php_dom_insert_fragment(nodep, prevsib, nextsib, newchild, intern, newchildobj); if (newchild) { - dom_reconcile_ns(nodep->doc, newchild); + dom_reconcile_ns_list(nodep->doc, newchild, last); } } else if (oldchild != newchild) { xmlDtdPtr intSubset = xmlGetIntSubset(nodep->doc); @@ -1215,22 +1229,28 @@ PHP_METHOD(DOMNode, appendChild) php_libxml_node_free_resource((xmlNodePtr) lastattr); } } + new_child = xmlAddChild(nodep, child); + if (UNEXPECTED(new_child == NULL)) { + goto cannot_add; + } } else if (child->type == XML_DOCUMENT_FRAG_NODE) { + xmlNodePtr last = child->last; new_child = _php_dom_insert_fragment(nodep, nodep->last, NULL, child, intern, childobj); - } - - if (new_child == NULL) { + dom_reconcile_ns_list(nodep->doc, new_child, last); + } else { new_child = xmlAddChild(nodep, child); - if (new_child == NULL) { - // TODO Convert to Error? - php_error_docref(NULL, E_WARNING, "Couldn't append node"); - RETURN_FALSE; + if (UNEXPECTED(new_child == NULL)) { + goto cannot_add; } + dom_reconcile_ns(nodep->doc, new_child); } - dom_reconcile_ns(nodep->doc, new_child); - DOM_RET_OBJ(new_child, &ret, intern); + return; +cannot_add: + // TODO Convert to Error? + php_error_docref(NULL, E_WARNING, "Couldn't append node"); + RETURN_FALSE; } /* }}} end dom_node_append_child */ diff --git a/ext/dom/parentnode.c b/ext/dom/parentnode.c index c99a2a5a6622a..b7e8e3ba774e3 100644 --- a/ext/dom/parentnode.c +++ b/ext/dom/parentnode.c @@ -298,13 +298,14 @@ void dom_parent_node_append(dom_object *context, zval *nodes, int nodesc) parentNode->children = newchild; } - parentNode->last = fragment->last; + xmlNodePtr last = fragment->last; + parentNode->last = last; newchild->prev = prevsib; dom_fragment_assign_parent_node(parentNode, fragment); - dom_reconcile_ns(parentNode->doc, newchild); + dom_reconcile_ns_list(parentNode->doc, newchild, last); } xmlFree(fragment); @@ -335,13 +336,14 @@ void dom_parent_node_prepend(dom_object *context, zval *nodes, int nodesc) nextsib = parentNode->children; if (newchild) { + xmlNodePtr last = fragment->last; parentNode->children = newchild; fragment->last->next = nextsib; - nextsib->prev = fragment->last; + nextsib->prev = last; dom_fragment_assign_parent_node(parentNode, fragment); - dom_reconcile_ns(parentNode->doc, newchild); + dom_reconcile_ns_list(parentNode->doc, newchild, last); } xmlFree(fragment); @@ -414,11 +416,13 @@ void dom_parent_node_after(dom_object *context, zval *nodes, int nodesc) newchild = fragment->children; if (newchild) { + xmlNodePtr last = fragment->last; + /* Step 5: place fragment into the parent before viable_next_sibling */ dom_pre_insert(viable_next_sibling, parentNode, newchild, fragment); dom_fragment_assign_parent_node(parentNode, fragment); - dom_reconcile_ns(doc, newchild); + dom_reconcile_ns_list(doc, newchild, last); } xmlFree(fragment); @@ -463,6 +467,8 @@ void dom_parent_node_before(dom_object *context, zval *nodes, int nodesc) newchild = fragment->children; if (newchild) { + xmlNodePtr last = fragment->last; + /* Step 5: if viable_previous_sibling is null, set it to the parent's first child, otherwise viable_previous_sibling's next sibling */ if (!viable_previous_sibling) { viable_previous_sibling = parentNode->children; @@ -473,7 +479,7 @@ void dom_parent_node_before(dom_object *context, zval *nodes, int nodesc) dom_pre_insert(viable_previous_sibling, parentNode, newchild, fragment); dom_fragment_assign_parent_node(parentNode, fragment); - dom_reconcile_ns(doc, newchild); + dom_reconcile_ns_list(doc, newchild, last); } xmlFree(fragment); diff --git a/ext/dom/php_dom.c b/ext/dom/php_dom.c index 1883767d2e48b..df20093221f16 100644 --- a/ext/dom/php_dom.c +++ b/ext/dom/php_dom.c @@ -1385,38 +1385,73 @@ void dom_set_old_ns(xmlDoc *doc, xmlNs *ns) { } /* }}} end dom_set_old_ns */ -void dom_reconcile_ns(xmlDocPtr doc, xmlNodePtr nodep) /* {{{ */ +static void dom_reconcile_ns_internal(xmlDocPtr doc, xmlNodePtr nodep) { xmlNsPtr nsptr, nsdftptr, curns, prevns = NULL; - if (nodep->type == XML_ELEMENT_NODE) { - /* Following if block primarily used for inserting nodes created via createElementNS */ - if (nodep->nsDef != NULL) { - curns = nodep->nsDef; - while (curns) { - nsdftptr = curns->next; - if (curns->href != NULL) { - if((nsptr = xmlSearchNsByHref(doc, nodep->parent, curns->href)) && - (curns->prefix == NULL || xmlStrEqual(nsptr->prefix, curns->prefix))) { - curns->next = NULL; - if (prevns == NULL) { - nodep->nsDef = nsdftptr; - } else { - prevns->next = nsdftptr; - } - dom_set_old_ns(doc, curns); - curns = prevns; + /* Following if block primarily used for inserting nodes created via createElementNS */ + if (nodep->nsDef != NULL) { + curns = nodep->nsDef; + while (curns) { + nsdftptr = curns->next; + if (curns->href != NULL) { + if((nsptr = xmlSearchNsByHref(doc, nodep->parent, curns->href)) && + (curns->prefix == NULL || xmlStrEqual(nsptr->prefix, curns->prefix))) { + curns->next = NULL; + if (prevns == NULL) { + nodep->nsDef = nsdftptr; + } else { + prevns->next = nsdftptr; } + dom_set_old_ns(doc, curns); + curns = prevns; } - prevns = curns; - curns = nsdftptr; } + prevns = curns; + curns = nsdftptr; } + } +} + +void dom_reconcile_ns(xmlDocPtr doc, xmlNodePtr nodep) /* {{{ */ +{ + if (nodep->type == XML_ELEMENT_NODE) { + dom_reconcile_ns_internal(doc, nodep); xmlReconciliateNs(doc, nodep); } } /* }}} */ +static void dom_reconcile_ns_list_internal(xmlDocPtr doc, xmlNodePtr nodep, xmlNodePtr last) +{ + ZEND_ASSERT(nodep != NULL); + while (true) { + if (nodep->type == XML_ELEMENT_NODE) { + dom_reconcile_ns_internal(doc, nodep); + if (nodep->children) { + dom_reconcile_ns_list_internal(doc, nodep->children, nodep->last /* process the whole children list */); + } + } + if (nodep == last) { + break; + } + nodep = nodep->next; + } +} + +void dom_reconcile_ns_list(xmlDocPtr doc, xmlNodePtr nodep, xmlNodePtr last) +{ + dom_reconcile_ns_list_internal(doc, nodep, last); + /* Outside of the recursion above because xmlReconciliateNs() performs its own recursion. */ + while (true) { + xmlReconciliateNs(doc, nodep); + if (nodep == last) { + break; + } + nodep = nodep->next; + } +} + /* http://www.w3.org/TR/2004/REC-DOM-Level-3-Core-20040407/core.html#ID-DocCrElNS diff --git a/ext/dom/php_dom.h b/ext/dom/php_dom.h index fdfdd4e7a31ca..924d1397ca73a 100644 --- a/ext/dom/php_dom.h +++ b/ext/dom/php_dom.h @@ -110,6 +110,7 @@ int dom_check_qname(char *qname, char **localname, char **prefix, int uri_len, i xmlNsPtr dom_get_ns(xmlNodePtr node, char *uri, int *errorcode, char *prefix); void dom_set_old_ns(xmlDoc *doc, xmlNs *ns); void dom_reconcile_ns(xmlDocPtr doc, xmlNodePtr nodep); +void dom_reconcile_ns_list(xmlDocPtr doc, xmlNodePtr nodep, xmlNodePtr last); xmlNsPtr dom_get_nsdecl(xmlNode *node, xmlChar *localName); void dom_normalize (xmlNodePtr nodep); xmlNode *dom_get_elements_by_tag_name_ns_raw(xmlNodePtr nodep, char *ns, char *local, int *cur, int index); diff --git a/ext/dom/tests/bug67440.phpt b/ext/dom/tests/bug67440.phpt new file mode 100644 index 0000000000000..3e30f69b9ae4d --- /dev/null +++ b/ext/dom/tests/bug67440.phpt @@ -0,0 +1,151 @@ +--TEST-- +Bug #67440 (append_node of a DOMDocumentFragment does not reconcile namespaces) +--EXTENSIONS-- +dom +--FILE-- +loadXML(''); + $fragment = $document->createDocumentFragment(); + $fragment->appendChild($document->createTextNode("\n")); + $fragment->appendChild($document->createElementNS('http://example/ns', 'myns:childNode', '1')); + $fragment->appendChild($document->createTextNode("\n")); + $fragment->appendChild($document->createElementNS('http://example/ns', 'myns:childNode', '2')); + $fragment->appendChild($document->createTextNode("\n")); + return array($document, $fragment); +} + +function case1($method) { + list($document, $fragment) = createDocument(); + $document->documentElement->{$method}($fragment); + echo $document->saveXML(); +} + +function case2($method) { + list($document, $fragment) = createDocument(); + $childNodes = iterator_to_array($fragment->childNodes); + foreach ($childNodes as $childNode) { + $document->documentElement->{$method}($childNode); + } + echo $document->saveXML(); +} + +function case3($method) { + list($document, $fragment) = createDocument(); + $fragment->removeChild($fragment->firstChild); + $document->documentElement->{$method}($fragment); + echo $document->saveXML(); +} + +function case4($method) { + list($document, $fragment) = createDocument(); + $fragment->childNodes[1]->appendChild($document->createElementNS('http://example/ns2', 'myns2:childNode', '3')); + $document->documentElement->{$method}($fragment); + echo $document->saveXML(); +} + +echo "== appendChild ==\n"; +echo "-- fragment to document element --\n"; case1('appendChild'); echo "\n"; +echo "-- children manually document element --\n"; case2('appendChild'); echo "\n"; +echo "-- fragment to document where first element is not a text node --\n"; case3('appendChild'); echo "\n"; +echo "-- fragment with namespace declarations in children --\n"; case4('appendChild'); echo "\n"; + +echo "== insertBefore ==\n"; +echo "-- fragment to document element --\n"; case1('insertBefore'); echo "\n"; +echo "-- children manually document element --\n"; case2('insertBefore'); echo "\n"; +echo "-- fragment to document where first element is not a text node --\n"; case3('insertBefore'); echo "\n"; +echo "-- fragment with namespace declarations in children --\n"; case4('insertBefore'); echo "\n"; + +echo "== insertAfter ==\n"; +echo "-- fragment to document element --\n"; case1('insertBefore'); echo "\n"; +echo "-- children manually document element --\n"; case2('insertBefore'); echo "\n"; +echo "-- fragment to document where first element is not a text node --\n"; case3('insertBefore'); echo "\n"; +echo "-- fragment with namespace declarations in children --\n"; case4('insertBefore'); echo "\n"; + +?> +--EXPECT-- +== appendChild == +-- fragment to document element -- + + +1 +2 + + +-- children manually document element -- + + +1 +2 + + +-- fragment to document where first element is not a text node -- + +1 +2 + + +-- fragment with namespace declarations in children -- + + +13 +2 + + +== insertBefore == +-- fragment to document element -- + + +1 +2 + + +-- children manually document element -- + + +1 +2 + + +-- fragment to document where first element is not a text node -- + +1 +2 + + +-- fragment with namespace declarations in children -- + + +13 +2 + + +== insertAfter == +-- fragment to document element -- + + +1 +2 + + +-- children manually document element -- + + +1 +2 + + +-- fragment to document where first element is not a text node -- + +1 +2 + + +-- fragment with namespace declarations in children -- + + +13 +2 + From 23f70025270c040e0d378b210120d9824af10ea6 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sat, 3 Jun 2023 17:54:37 +0200 Subject: [PATCH 24/54] Fix bug #81642: DOMChildNode::replaceWith() bug when replacing a node with itself Closes GH-11363. --- NEWS | 2 ++ ext/dom/element.c | 5 ++- ext/dom/parentnode.c | 69 +++++++++++++++++++++++++++++++------ ext/dom/php_dom.h | 1 + ext/dom/tests/bug81642.phpt | 49 ++++++++++++++++++++++++++ 5 files changed, 112 insertions(+), 14 deletions(-) create mode 100644 ext/dom/tests/bug81642.phpt diff --git a/NEWS b/NEWS index 8395e5233c864..39605d0d3c4c0 100644 --- a/NEWS +++ b/NEWS @@ -23,6 +23,8 @@ PHP NEWS xpath query). (nielsdos) . Fixed bug #67440 (append_node of a DOMDocumentFragment does not reconcile namespaces). (nielsdos) + . Fixed bug #81642 (DOMChildNode::replaceWith() bug when replacing a node + with itself). (nielsdos) - Opcache: . Fix allocation loop in zend_shared_alloc_startup(). (nielsdos) diff --git a/ext/dom/element.c b/ext/dom/element.c index 19cef5834657a..78113d72776bd 100644 --- a/ext/dom/element.c +++ b/ext/dom/element.c @@ -1234,7 +1234,7 @@ PHP_METHOD(DOMElement, prepend) } /* }}} end DOMElement::prepend */ -/* {{{ URL: https://dom.spec.whatwg.org/#dom-parentnode-prepend +/* {{{ URL: https://dom.spec.whatwg.org/#dom-parentnode-replacechildren Since: DOM Living Standard (DOM4) */ PHP_METHOD(DOMElement, replaceWith) @@ -1251,8 +1251,7 @@ PHP_METHOD(DOMElement, replaceWith) id = ZEND_THIS; DOM_GET_OBJ(context, id, xmlNodePtr, intern); - dom_parent_node_after(intern, args, argc); - dom_child_node_remove(intern); + dom_child_replace_with(intern, args, argc); } /* }}} end DOMElement::prepend */ diff --git a/ext/dom/parentnode.c b/ext/dom/parentnode.c index b7e8e3ba774e3..a9dfda59622b7 100644 --- a/ext/dom/parentnode.c +++ b/ext/dom/parentnode.c @@ -485,35 +485,45 @@ void dom_parent_node_before(dom_object *context, zval *nodes, int nodesc) xmlFree(fragment); } -void dom_child_node_remove(dom_object *context) +static zend_result dom_child_removal_preconditions(const xmlNodePtr child, int stricterror) { - xmlNode *child = dom_object_get_node(context); - xmlNodePtr children; - int stricterror; - - stricterror = dom_get_strict_error(context->document); - if (dom_node_is_read_only(child) == SUCCESS || (child->parent != NULL && dom_node_is_read_only(child->parent) == SUCCESS)) { php_dom_throw_error(NO_MODIFICATION_ALLOWED_ERR, stricterror); - return; + return FAILURE; } if (!child->parent) { php_dom_throw_error(NOT_FOUND_ERR, stricterror); - return; + return FAILURE; } if (dom_node_children_valid(child->parent) == FAILURE) { - return; + return FAILURE; } - children = child->parent->children; + xmlNodePtr children = child->parent->children; if (!children) { php_dom_throw_error(NOT_FOUND_ERR, stricterror); + return FAILURE; + } + + return SUCCESS; +} + +void dom_child_node_remove(dom_object *context) +{ + xmlNode *child = dom_object_get_node(context); + xmlNodePtr children; + int stricterror; + + stricterror = dom_get_strict_error(context->document); + + if (UNEXPECTED(dom_child_removal_preconditions(child, stricterror) != SUCCESS)) { return; } + children = child->parent->children; while (children) { if (children == child) { xmlUnlinkNode(child); @@ -525,4 +535,41 @@ void dom_child_node_remove(dom_object *context) php_dom_throw_error(NOT_FOUND_ERR, stricterror); } +void dom_child_replace_with(dom_object *context, zval *nodes, int nodesc) +{ + xmlNodePtr child = dom_object_get_node(context); + xmlNodePtr parentNode = child->parent; + + int stricterror = dom_get_strict_error(context->document); + if (UNEXPECTED(dom_child_removal_preconditions(child, stricterror) != SUCCESS)) { + return; + } + + xmlNodePtr insertion_point = child->next; + + xmlNodePtr fragment = dom_zvals_to_fragment(context->document, parentNode, nodes, nodesc); + if (UNEXPECTED(fragment == NULL)) { + return; + } + + xmlNodePtr newchild = fragment->children; + xmlDocPtr doc = parentNode->doc; + + if (newchild) { + xmlNodePtr last = fragment->last; + + /* Unlink and free it unless it became a part of the fragment. */ + if (child->parent != fragment) { + xmlUnlinkNode(child); + } + + dom_pre_insert(insertion_point, parentNode, newchild, fragment); + + dom_fragment_assign_parent_node(parentNode, fragment); + dom_reconcile_ns_list(doc, newchild, last); + } + + xmlFree(fragment); +} + #endif diff --git a/ext/dom/php_dom.h b/ext/dom/php_dom.h index 924d1397ca73a..ac23d1fc25bb5 100644 --- a/ext/dom/php_dom.h +++ b/ext/dom/php_dom.h @@ -132,6 +132,7 @@ void dom_parent_node_append(dom_object *context, zval *nodes, int nodesc); void dom_parent_node_after(dom_object *context, zval *nodes, int nodesc); void dom_parent_node_before(dom_object *context, zval *nodes, int nodesc); void dom_child_node_remove(dom_object *context); +void dom_child_replace_with(dom_object *context, zval *nodes, int nodesc); #define DOM_GET_OBJ(__ptr, __id, __prtype, __intern) { \ __intern = Z_DOMOBJ_P(__id); \ diff --git a/ext/dom/tests/bug81642.phpt b/ext/dom/tests/bug81642.phpt new file mode 100644 index 0000000000000..7bf3dde50588e --- /dev/null +++ b/ext/dom/tests/bug81642.phpt @@ -0,0 +1,49 @@ +--TEST-- +Bug #81642 (DOMChildNode::replaceWith() bug when replacing a node with itself) +--EXTENSIONS-- +dom +--FILE-- +appendChild($target = $doc->createElement('test')); +$target->replaceWith($target); +var_dump($doc->saveXML()); + +// Replace with itself + another element +$doc = new DOMDocument(); +$doc->appendChild($target = $doc->createElement('test')); +$target->replaceWith($target, $doc->createElement('foo')); +var_dump($doc->saveXML()); + +// Replace with text node +$doc = new DOMDocument(); +$doc->appendChild($target = $doc->createElement('test')); +$target->replaceWith($target, 'foo'); +var_dump($doc->saveXML()); + +// Replace with text node variant 2 +$doc = new DOMDocument(); +$doc->appendChild($target = $doc->createElement('test')); +$target->replaceWith('bar', $target, 'foo'); +var_dump($doc->saveXML()); + +?> +--EXPECT-- +string(30) " + +" +string(37) " + + +" +string(34) " + +foo +" +string(38) " +bar + +foo +" From 0e34ac864a20bd03a35741db09f0bdf72ae56874 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 4 Jun 2023 15:13:37 +0200 Subject: [PATCH 25/54] Fix bug #77686: Removed elements are still returned by getElementById From the moment an ID is created, libxml2's behaviour is to cache that element, even if that element is not yet attached to the document. Similarly, only upon destruction of the element the ID is actually removed by libxml2. Since libxml2 has such behaviour deeply ingrained in the library, and uses the cache for various purposes, it seems like a bad idea and lost cause to fight it. Instead, we'll simply walk the tree upwards to check if the node is attached to the document. Closes GH-11369. --- NEWS | 2 ++ ext/dom/document.c | 21 ++++++++++++++++++- ext/dom/tests/bug77686.phpt | 40 +++++++++++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 ext/dom/tests/bug77686.phpt diff --git a/NEWS b/NEWS index 39605d0d3c4c0..122e4a48b86bd 100644 --- a/NEWS +++ b/NEWS @@ -25,6 +25,8 @@ PHP NEWS namespaces). (nielsdos) . Fixed bug #81642 (DOMChildNode::replaceWith() bug when replacing a node with itself). (nielsdos) + . Fixed bug #77686 (Removed elements are still returned by getElementById). + (nielsdos) - Opcache: . Fix allocation loop in zend_shared_alloc_startup(). (nielsdos) diff --git a/ext/dom/document.c b/ext/dom/document.c index c60198a3be110..93091df83a04f 100644 --- a/ext/dom/document.c +++ b/ext/dom/document.c @@ -1008,6 +1008,19 @@ PHP_METHOD(DOMDocument, getElementsByTagNameNS) } /* }}} end dom_document_get_elements_by_tag_name_ns */ +static bool php_dom_is_node_attached(const xmlNode *node) +{ + ZEND_ASSERT(node != NULL); + node = node->parent; + while (node != NULL) { + if (node->type == XML_DOCUMENT_NODE || node->type == XML_HTML_DOCUMENT_NODE) { + return true; + } + node = node->parent; + } + return false; +} + /* {{{ URL: http://www.w3.org/TR/2003/WD-DOM-Level-3-Core-20030226/DOM3-Core.html#core-ID-getElBId Since: DOM Level 2 */ @@ -1030,7 +1043,13 @@ PHP_METHOD(DOMDocument, getElementById) attrp = xmlGetID(docp, (xmlChar *) idname); - if (attrp && attrp->parent) { + /* From the moment an ID is created, libxml2's behaviour is to cache that element, even + * if that element is not yet attached to the document. Similarly, only upon destruction of + * the element the ID is actually removed by libxml2. Since libxml2 has such behaviour deeply + * ingrained in the library, and uses the cache for various purposes, it seems like a bad + * idea and lost cause to fight it. Instead, we'll simply walk the tree upwards to check + * if the node is attached to the document. */ + if (attrp && attrp->parent && php_dom_is_node_attached(attrp->parent)) { DOM_RET_OBJ((xmlNodePtr) attrp->parent, &ret, intern); } else { RETVAL_NULL(); diff --git a/ext/dom/tests/bug77686.phpt b/ext/dom/tests/bug77686.phpt new file mode 100644 index 0000000000000..ddd7c3364786c --- /dev/null +++ b/ext/dom/tests/bug77686.phpt @@ -0,0 +1,40 @@ +--TEST-- +Bug #77686 (Removed elements are still returned by getElementById) +--EXTENSIONS-- +dom +--FILE-- +loadHTML('before
hello
after'); +$body = $doc->getElementById('x'); +$div = $doc->getElementById('y'); +var_dump($doc->getElementById('y')->textContent); + +// Detached from document, should not find it anymore +$body->removeChild($div); +var_dump($doc->getElementById('y')); + +// Added again, should find it +$body->appendChild($div); +var_dump($doc->getElementById('y')->textContent); + +// Should find root element without a problem +var_dump($doc->getElementById('htmlelement')->textContent); + +// Created element but not yet attached, should not find it before it is added +$new_element = $doc->createElement('p'); +$new_element->textContent = 'my new text'; +$new_element->setAttribute('id', 'myp'); +var_dump($doc->getElementById('myp')); +$body->appendChild($new_element); +var_dump($doc->getElementById('myp')->textContent); + +?> +--EXPECT-- +string(5) "hello" +NULL +string(5) "hello" +string(16) "beforeafterhello" +NULL +string(11) "my new text" From 8f06febedf064f92193b3b0120671de973c94487 Mon Sep 17 00:00:00 2001 From: Dmitry Stogov Date: Tue, 6 Jun 2023 13:29:55 +0300 Subject: [PATCH 26/54] Fixed deoptimization info for interrupt handler --- ext/opcache/jit/zend_jit_trace.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ext/opcache/jit/zend_jit_trace.c b/ext/opcache/jit/zend_jit_trace.c index d2e3e1e1c3185..cae233c684eb3 100644 --- a/ext/opcache/jit/zend_jit_trace.c +++ b/ext/opcache/jit/zend_jit_trace.c @@ -6834,7 +6834,8 @@ static const void *zend_jit_trace(zend_jit_trace_rec *trace_buffer, uint32_t par if (!(t->flags & ZEND_JIT_TRACE_USES_INITIAL_IP) || (ra && zend_jit_trace_stack_needs_deoptimization(stack, op_array->last_var + op_array->T))) { - uint32_t exit_point = zend_jit_trace_get_exit_point(opline, ZEND_JIT_EXIT_TO_VM); + /* Deoptimize to the first instruction of the loop */ + uint32_t exit_point = zend_jit_trace_get_exit_point(trace_buffer[1].opline, ZEND_JIT_EXIT_TO_VM); timeout_exit_addr = zend_jit_trace_get_exit_addr(exit_point); if (!timeout_exit_addr) { From cced0ddf9d8f25c7f95f6331e5f89350f90edd40 Mon Sep 17 00:00:00 2001 From: nielsdos <7771979+nielsdos@users.noreply.github.com> Date: Mon, 5 Jun 2023 22:13:35 +0200 Subject: [PATCH 27/54] Fix test failure for init_fcall_003.phpt without opcache If opcache isn't loaded, then opcache_invalidate() will fail. Reproducible when you compile PHP without opcache, or run PHP without opcache loaded, and try to run this test. Closes GH-11378. --- ext/opcache/tests/jit/init_fcall_003.phpt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ext/opcache/tests/jit/init_fcall_003.phpt b/ext/opcache/tests/jit/init_fcall_003.phpt index f37344cbce4a9..180f0745c16c6 100644 --- a/ext/opcache/tests/jit/init_fcall_003.phpt +++ b/ext/opcache/tests/jit/init_fcall_003.phpt @@ -11,6 +11,8 @@ opcache.jit_hot_loop=64 opcache.jit_hot_func=127 opcache.jit_hot_return=8 opcache.jit_hot_side_exit=8 +--EXTENSIONS-- +opcache --FILE-- Date: Sun, 16 Apr 2023 15:05:03 +0200 Subject: [PATCH 28/54] Fix missing randomness check and insufficient random bytes for SOAP HTTP Digest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If php_random_bytes_throw fails, the nonce will be uninitialized, but still sent to the server. The client nonce is intended to protect against a malicious server. See section 5.10 and 5.12 of RFC 7616 [1], and bullet point 2 below. Tim pointed out that even though it's the MD5 of the nonce that gets sent, enumerating 31 bits is trivial. So we have still a stack information leak of 31 bits. Furthermore, Tim found the following issues: * The small size of cnonce might cause the server to erroneously reject a request due to a repeated (cnonce, nc) pair. As per the birthday problem 31 bits of randomness will return a duplication with 50% chance after less than 55000 requests and nc always starts counting at 1. * The cnonce is intended to protect the client and password against a malicious server that returns a constant server nonce where the server precomputed a rainbow table between passwords and correct client response. As storage is fairly cheap, a server could precompute the client responses for (a subset of) client nonces and still have a chance of reversing the client response with the same probability as the cnonce duplication. Precomputing the rainbow table for all 2^31 cnonces increases the rainbow table size by factor 2 billion, which is infeasible. But precomputing it for 2^14 cnonces only increases the table size by factor 16k and the server would still have a 10% chance of successfully reversing a password with a single client request. This patch fixes the issues by increasing the nonce size, and checking the return value of php_random_bytes_throw(). In the process we also get rid of the MD5 hashing of the nonce. [1] RFC 7616: https://www.rfc-editor.org/rfc/rfc7616 Co-authored-by: Tim Düsterhus --- ext/soap/php_http.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/ext/soap/php_http.c b/ext/soap/php_http.c index 1da286ad875f8..e796dba9619ac 100644 --- a/ext/soap/php_http.c +++ b/ext/soap/php_http.c @@ -664,18 +664,23 @@ int make_http_soap_request(zval *this_ptr, if ((digest = zend_hash_str_find(Z_OBJPROP_P(this_ptr), "_digest", sizeof("_digest")-1)) != NULL) { if (Z_TYPE_P(digest) == IS_ARRAY) { char HA1[33], HA2[33], response[33], cnonce[33], nc[9]; - zend_long nonce; + unsigned char nonce[16]; PHP_MD5_CTX md5ctx; unsigned char hash[16]; - php_random_bytes_throw(&nonce, sizeof(nonce)); - nonce &= 0x7fffffff; + if (UNEXPECTED(php_random_bytes_throw(&nonce, sizeof(nonce)) != SUCCESS)) { + ZEND_ASSERT(EG(exception)); + php_stream_close(stream); + convert_to_null(Z_CLIENT_HTTPURL_P(this_ptr)); + convert_to_null(Z_CLIENT_HTTPSOCKET_P(this_ptr)); + convert_to_null(Z_CLIENT_USE_PROXY_P(this_ptr)); + smart_str_free(&soap_headers_z); + smart_str_free(&soap_headers); + return FALSE; + } - PHP_MD5Init(&md5ctx); - snprintf(cnonce, sizeof(cnonce), ZEND_LONG_FMT, nonce); - PHP_MD5Update(&md5ctx, (unsigned char*)cnonce, strlen(cnonce)); - PHP_MD5Final(hash, &md5ctx); - make_digest(cnonce, hash); + php_hash_bin2hex(cnonce, nonce, sizeof(nonce)); + cnonce[32] = 0; if ((tmp = zend_hash_str_find(Z_ARRVAL_P(digest), "nc", sizeof("nc")-1)) != NULL && Z_TYPE_P(tmp) == IS_LONG) { From 05724482637904235b95082d06e0dc01965c73d0 Mon Sep 17 00:00:00 2001 From: Remi Collet Date: Tue, 6 Jun 2023 18:05:22 +0200 Subject: [PATCH 29/54] Fix GH-11382 add missing hash header for bin2hex --- ext/soap/php_http.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ext/soap/php_http.c b/ext/soap/php_http.c index e796dba9619ac..77ed21d4f0f4e 100644 --- a/ext/soap/php_http.c +++ b/ext/soap/php_http.c @@ -20,6 +20,7 @@ #include "ext/standard/base64.h" #include "ext/standard/md5.h" #include "ext/standard/php_random.h" +#include "ext/hash/php_hash.h" static char *get_http_header_value_nodup(char *headers, char *type, size_t *len); static char *get_http_header_value(char *headers, char *type); From b720ab99f8de31e878e1707f0e232f28fc6655c5 Mon Sep 17 00:00:00 2001 From: Pierrick Charron Date: Tue, 6 Jun 2023 17:59:43 -0400 Subject: [PATCH 30/54] Update NEWS --- NEWS | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 9b41aeb1ed7d5..ff0d216d435ce 100644 --- a/NEWS +++ b/NEWS @@ -1,8 +1,14 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| -?? ??? ????, PHP 8.0.29 +?? ??? ????, PHP 8.0.30 +08 Jun 2023, PHP 8.0.29 + +- Soap: + . Fixed bug GHSA-76gg-c692-v2mw (Missing error check and insufficient random + bytes in HTTP Digest authentication for SOAP). (nielsdos, timwolla) + 14 Feb 2023, PHP 8.0.28 - Core: From 5604f7ae22cbc8f0539aa49421201348895f3401 Mon Sep 17 00:00:00 2001 From: Pierrick Charron Date: Tue, 6 Jun 2023 18:06:13 -0400 Subject: [PATCH 31/54] Update NEWS --- NEWS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS b/NEWS index 122e4a48b86bd..33d789b6b1714 100644 --- a/NEWS +++ b/NEWS @@ -89,6 +89,8 @@ PHP NEWS done). (peter279k) - Soap: + . Fixed bug GHSA-76gg-c692-v2mw (Missing error check and insufficient random + bytes in HTTP Digest authentication for SOAP). (nielsdos, timwolla) . Fixed bug GH-8426 (make test fail while soap extension build). (nielsdos) - SPL: From 269d6c5942896617c1bb51d143c25f4ffe1c6259 Mon Sep 17 00:00:00 2001 From: Pierrick Charron Date: Tue, 6 Jun 2023 18:10:06 -0400 Subject: [PATCH 32/54] Update NEWS --- NEWS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS b/NEWS index 68a2d8cb71544..2551047462e89 100644 --- a/NEWS +++ b/NEWS @@ -103,6 +103,8 @@ PHP NEWS done). (peter279k) - Soap: + . Fixed bug GHSA-76gg-c692-v2mw (Missing error check and insufficient random + bytes in HTTP Digest authentication for SOAP). (nielsdos, timwolla) . Fixed bug GH-8426 (make test fail while soap extension build). (nielsdos) - SPL: From 06d68738b78bd7a469931ca035f3dd0cce805623 Mon Sep 17 00:00:00 2001 From: Dmitry Stogov Date: Thu, 8 Jun 2023 14:55:18 +0300 Subject: [PATCH 33/54] Keep consistent EG(current_execute_data) after return from generator (#11380) --- Zend/zend_vm_def.h | 4 ++++ Zend/zend_vm_execute.h | 12 ++++++++++++ 2 files changed, 16 insertions(+) diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h index a2db0e30da7ac..646dab4ae685a 100644 --- a/Zend/zend_vm_def.h +++ b/Zend/zend_vm_def.h @@ -4549,6 +4549,8 @@ ZEND_VM_HANDLER(161, ZEND_GENERATOR_RETURN, CONST|TMP|VAR|CV, ANY, SPEC(OBSERVER ZEND_OBSERVER_FCALL_END(generator->execute_data, &generator->retval); + EG(current_execute_data) = EX(prev_execute_data); + /* Close the generator to free up resources */ zend_generator_close(generator, 1); @@ -7837,6 +7839,7 @@ ZEND_VM_HELPER(zend_dispatch_try_catch_finally_helper, ANY, ANY, uint32_t try_ca cleanup_live_vars(execute_data, op_num, 0); if (UNEXPECTED((EX_CALL_INFO() & ZEND_CALL_GENERATOR) != 0)) { zend_generator *generator = zend_get_running_generator(EXECUTE_DATA_C); + EG(current_execute_data) = EX(prev_execute_data); zend_generator_close(generator, 1); ZEND_VM_RETURN(); } else { @@ -7930,6 +7933,7 @@ ZEND_VM_HANDLER(150, ZEND_USER_OPCODE, ANY, ANY) case ZEND_USER_OPCODE_RETURN: if (UNEXPECTED((EX_CALL_INFO() & ZEND_CALL_GENERATOR) != 0)) { zend_generator *generator = zend_get_running_generator(EXECUTE_DATA_C); + EG(current_execute_data) = EX(prev_execute_data); zend_generator_close(generator, 1); ZEND_VM_RETURN(); } else { diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h index 388d19e3d692b..5675f89412159 100644 --- a/Zend/zend_vm_execute.h +++ b/Zend/zend_vm_execute.h @@ -3061,6 +3061,7 @@ static zend_never_inline ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_dispatch_try cleanup_live_vars(execute_data, op_num, 0); if (UNEXPECTED((EX_CALL_INFO() & ZEND_CALL_GENERATOR) != 0)) { zend_generator *generator = zend_get_running_generator(EXECUTE_DATA_C); + EG(current_execute_data) = EX(prev_execute_data); zend_generator_close(generator, 1); ZEND_VM_RETURN(); } else { @@ -3154,6 +3155,7 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_USER_OPCODE_SPEC_HANDLER(ZEND_ case ZEND_USER_OPCODE_RETURN: if (UNEXPECTED((EX_CALL_INFO() & ZEND_CALL_GENERATOR) != 0)) { zend_generator *generator = zend_get_running_generator(EXECUTE_DATA_C); + EG(current_execute_data) = EX(prev_execute_data); zend_generator_close(generator, 1); ZEND_VM_RETURN(); } else { @@ -4517,6 +4519,8 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_GENERATOR_RETURN_SPEC_CONST_HA } } + EG(current_execute_data) = EX(prev_execute_data); + /* Close the generator to free up resources */ zend_generator_close(generator, 1); @@ -4562,6 +4566,8 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_GENERATOR_RETURN_SPEC_OBSERVER zend_observer_fcall_end(generator->execute_data, &generator->retval); + EG(current_execute_data) = EX(prev_execute_data); + /* Close the generator to free up resources */ zend_generator_close(generator, 1); @@ -18954,6 +18960,8 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_GENERATOR_RETURN_SPEC_TMP_HAND } } + EG(current_execute_data) = EX(prev_execute_data); + /* Close the generator to free up resources */ zend_generator_close(generator, 1); @@ -21612,6 +21620,8 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_GENERATOR_RETURN_SPEC_VAR_HAND } } + EG(current_execute_data) = EX(prev_execute_data); + /* Close the generator to free up resources */ zend_generator_close(generator, 1); @@ -38480,6 +38490,8 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_GENERATOR_RETURN_SPEC_CV_HANDL } } + EG(current_execute_data) = EX(prev_execute_data); + /* Close the generator to free up resources */ zend_generator_close(generator, 1); From 709540ccdc5a3fc25fcdfceba322e6ad3aa3ce6f Mon Sep 17 00:00:00 2001 From: Bob Weinand Date: Fri, 9 Jun 2023 14:00:53 +0200 Subject: [PATCH 34/54] Fix add/remove observer API with multiple observers installed Depending on the order in which observers were installed, some observers might have been executed twice after removal of another observer. Also, adding an observer could produce a bogus pointer. --- Zend/zend_observer.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Zend/zend_observer.c b/Zend/zend_observer.c index 79929bfdd80e5..2cb4db914758a 100644 --- a/Zend/zend_observer.c +++ b/Zend/zend_observer.c @@ -158,9 +158,8 @@ static bool zend_observer_remove_handler(void **first_handler, void *old_handler } else { if (cur_handler != last_handler) { memmove(cur_handler, cur_handler + 1, sizeof(cur_handler) * (last_handler - cur_handler)); - } else { - *last_handler = NULL; } + *last_handler = NULL; } return true; } @@ -196,7 +195,7 @@ ZEND_API void zend_observer_add_end_handler(zend_function *function, zend_observ if (*end_handler != ZEND_OBSERVER_NOT_OBSERVED) { // there's no space for new handlers, then it's forbidden to call this function ZEND_ASSERT(end_handler[registered_observers - 1] == NULL); - memmove(end_handler + 1, end_handler, registered_observers - 1); + memmove(end_handler + 1, end_handler, sizeof(end_handler) * (registered_observers - 1)); } *end_handler = end; } From fd09728bb6a5c8f7c7320ae1e60a2db48c765ce6 Mon Sep 17 00:00:00 2001 From: James Lucas Date: Fri, 28 Apr 2023 11:02:49 +1000 Subject: [PATCH 35/54] Fix bug GH-9356: Incomplete SAN validation of IPv6 address IPv6 addresses are valid entries in subjectAltNames. Certificate Authorities may issue certificates including IPv6 addresses except if they fall within addresses in the RFC 4193 range. Google and CloudFlare provide IPv6 addresses in their DNS over HTTPS services. Internal CAs do not have those restrictions and can issue Unique local addresses in certificates. Closes GH-11145 --- NEWS | 4 ++ ext/openssl/tests/san_ipv6_peer_matching.phpt | 69 +++++++++++++++++++ ext/openssl/xp_ssl.c | 47 +++++++++++-- 3 files changed, 116 insertions(+), 4 deletions(-) create mode 100644 ext/openssl/tests/san_ipv6_peer_matching.phpt diff --git a/NEWS b/NEWS index 33d789b6b1714..8eae4ccaa5e7c 100644 --- a/NEWS +++ b/NEWS @@ -34,6 +34,10 @@ PHP NEWS . Fixed bug GH-11336 (php still tries to unlock the shared memory ZendSem with opcache.file_cache_only=1 but it was never locked). (nielsdos) +- OpenSSL: + . Fixed bug GH-9356 Incomplete validation of IPv6 Address fields in + subjectAltNames (James Lucas, Jakub Zelenka). + - SPL: . Fixed bug GH-11338 (SplFileInfo empty getBasename with more than one slash). (nielsdos) diff --git a/ext/openssl/tests/san_ipv6_peer_matching.phpt b/ext/openssl/tests/san_ipv6_peer_matching.phpt new file mode 100644 index 0000000000000..81966025d3969 --- /dev/null +++ b/ext/openssl/tests/san_ipv6_peer_matching.phpt @@ -0,0 +1,69 @@ +--TEST-- +IPv6 Peer verification matches SAN names +--EXTENSIONS-- +openssl +--SKIPIF-- + +--FILE-- + [ + 'local_cert' => '%s', + ]]); + + $server = stream_socket_server($serverUri, $errno, $errstr, $serverFlags, $serverCtx); + phpt_notify(); + + @stream_socket_accept($server, 1); + @stream_socket_accept($server, 1); +CODE; +$serverCode = sprintf($serverCode, $certFile); + +$clientCode = <<<'CODE' + $serverUri = "ssl://[::1]:64324"; + $clientFlags = STREAM_CLIENT_CONNECT; + $clientCtx = stream_context_create(['ssl' => [ + 'verify_peer' => false, + ]]); + + phpt_wait(); + + stream_context_set_option($clientCtx, 'ssl', 'peer_name', '2001:db8:85a3:8d3:1319:8a2e:370:7348'); + var_dump(stream_socket_client($serverUri, $errno, $errstr, 1, $clientFlags, $clientCtx)); + + stream_context_set_option($clientCtx, 'ssl', 'peer_name', '2001:db8:85a3:8d3:1319:8a2e:370:7349'); + var_dump(stream_socket_client($serverUri, $errno, $errstr, 1, $clientFlags, $clientCtx)); +CODE; + +include 'CertificateGenerator.inc'; +$certificateGenerator = new CertificateGenerator(); +$certificateGenerator->saveNewCertAsFileWithKey(null, $certFile, null, $san); + +include 'ServerClientTestCase.inc'; +ServerClientTestCase::getInstance()->run($clientCode, $serverCode); +?> +--CLEAN-- + +--EXPECTF-- +resource(%d) of type (stream) + +Warning: stream_socket_client(): Unable to locate peer certificate CN in %s on line %d + +Warning: stream_socket_client(): Failed to enable crypto in %s on line %d + +Warning: stream_socket_client(): Unable to connect to ssl://[::1]:64324 (Unknown error) in %s on line %d +bool(false) diff --git a/ext/openssl/xp_ssl.c b/ext/openssl/xp_ssl.c index 9aac4a0b70a28..5b3ad2c1f8863 100644 --- a/ext/openssl/xp_ssl.c +++ b/ext/openssl/xp_ssl.c @@ -39,6 +39,7 @@ #ifdef PHP_WIN32 #include "win32/winutil.h" #include "win32/time.h" +#include #include /* These are from Wincrypt.h, they conflict with OpenSSL */ #undef X509_NAME @@ -46,6 +47,10 @@ #undef X509_EXTENSIONS #endif +#ifdef HAVE_ARPA_INET_H +#include +#endif + /* Flags for determining allowed stream crypto methods */ #define STREAM_CRYPTO_IS_CLIENT (1<<0) #define STREAM_CRYPTO_METHOD_SSLv2 (1<<1) @@ -110,6 +115,21 @@ #define PHP_X509_NAME_ENTRY_TO_UTF8(ne, i, out) \ ASN1_STRING_to_UTF8(&out, X509_NAME_ENTRY_get_data(X509_NAME_get_entry(ne, i))) +/* Used for IPv6 Address peer verification */ +#define EXPAND_IPV6_ADDRESS(_str, _bytes) \ + do { \ + snprintf(_str, 40, "%X:%X:%X:%X:%X:%X:%X:%X", \ + _bytes[0] << 8 | _bytes[1], \ + _bytes[2] << 8 | _bytes[3], \ + _bytes[4] << 8 | _bytes[5], \ + _bytes[6] << 8 | _bytes[7], \ + _bytes[8] << 8 | _bytes[9], \ + _bytes[10] << 8 | _bytes[11], \ + _bytes[12] << 8 | _bytes[13], \ + _bytes[14] << 8 | _bytes[15] \ + ); \ + } while(0) + #if PHP_OPENSSL_API_VERSION < 0x10100 static RSA *php_openssl_tmp_rsa_cb(SSL *s, int is_export, int keylength); #endif @@ -421,6 +441,18 @@ static bool php_openssl_matches_san_list(X509 *peer, const char *subject_name) / GENERAL_NAMES *alt_names = X509_get_ext_d2i(peer, NID_subject_alt_name, 0, 0); int alt_name_count = sk_GENERAL_NAME_num(alt_names); +#if defined(HAVE_IPV6) && defined(HAVE_INET_PTON) + /* detect if subject name is an IPv6 address and expand once if required */ + char subject_name_ipv6_expanded[40]; + unsigned char ipv6[16]; + bool subject_name_is_ipv6 = false; + subject_name_ipv6_expanded[0] = 0; + if (inet_pton(AF_INET6, subject_name, &ipv6)) { + EXPAND_IPV6_ADDRESS(subject_name_ipv6_expanded, ipv6); + subject_name_is_ipv6 = true; + } +#endif + for (i = 0; i < alt_name_count; i++) { GENERAL_NAME *san = sk_GENERAL_NAME_value(alt_names, i); @@ -459,10 +491,17 @@ static bool php_openssl_matches_san_list(X509 *peer, const char *subject_name) / return 1; } } - /* No, we aren't bothering to check IPv6 addresses. Why? - * Because IP SAN names are officially deprecated and are - * not allowed by CAs starting in 2015. Deal with it. - */ +#if defined(HAVE_IPV6) && defined(HAVE_INET_PTON) + else if (san->d.ip->length == 16 && subject_name_is_ipv6) { + ipbuffer[0] = 0; + EXPAND_IPV6_ADDRESS(ipbuffer, san->d.iPAddress->data); + if (strcasecmp((const char*)subject_name_ipv6_expanded, (const char*)ipbuffer) == 0) { + sk_GENERAL_NAME_pop_free(alt_names, GENERAL_NAME_free); + + return 1; + } + } +#endif } } From 3fc013b2e24bcb38805975be4ae913a925075d41 Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Fri, 9 Jun 2023 16:48:00 +0100 Subject: [PATCH 36/54] Fix CS and checking for IPv6 SAN verify --- ext/openssl/xp_ssl.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/ext/openssl/xp_ssl.c b/ext/openssl/xp_ssl.c index 5b3ad2c1f8863..6890810125cef 100644 --- a/ext/openssl/xp_ssl.c +++ b/ext/openssl/xp_ssl.c @@ -115,6 +115,7 @@ #define PHP_X509_NAME_ENTRY_TO_UTF8(ne, i, out) \ ASN1_STRING_to_UTF8(&out, X509_NAME_ENTRY_get_data(X509_NAME_get_entry(ne, i))) +#if defined(HAVE_IPV6) && defined(HAVE_INET_PTON) /* Used for IPv6 Address peer verification */ #define EXPAND_IPV6_ADDRESS(_str, _bytes) \ do { \ @@ -129,6 +130,8 @@ _bytes[14] << 8 | _bytes[15] \ ); \ } while(0) +#define HAVE_IPV6_SAN 1 +#endif #if PHP_OPENSSL_API_VERSION < 0x10100 static RSA *php_openssl_tmp_rsa_cb(SSL *s, int is_export, int keylength); @@ -441,16 +444,17 @@ static bool php_openssl_matches_san_list(X509 *peer, const char *subject_name) / GENERAL_NAMES *alt_names = X509_get_ext_d2i(peer, NID_subject_alt_name, 0, 0); int alt_name_count = sk_GENERAL_NAME_num(alt_names); -#if defined(HAVE_IPV6) && defined(HAVE_INET_PTON) +#ifdef HAVE_IPV6_SAN /* detect if subject name is an IPv6 address and expand once if required */ - char subject_name_ipv6_expanded[40]; - unsigned char ipv6[16]; - bool subject_name_is_ipv6 = false; - subject_name_ipv6_expanded[0] = 0; + char subject_name_ipv6_expanded[40]; + unsigned char ipv6[16]; + bool subject_name_is_ipv6 = false; + subject_name_ipv6_expanded[0] = 0; + if (inet_pton(AF_INET6, subject_name, &ipv6)) { - EXPAND_IPV6_ADDRESS(subject_name_ipv6_expanded, ipv6); - subject_name_is_ipv6 = true; - } + EXPAND_IPV6_ADDRESS(subject_name_ipv6_expanded, ipv6); + subject_name_is_ipv6 = true; + } #endif for (i = 0; i < alt_name_count; i++) { @@ -491,7 +495,7 @@ static bool php_openssl_matches_san_list(X509 *peer, const char *subject_name) / return 1; } } -#if defined(HAVE_IPV6) && defined(HAVE_INET_PTON) +#ifdef HAVE_IPV6_SAN else if (san->d.ip->length == 16 && subject_name_is_ipv6) { ipbuffer[0] = 0; EXPAND_IPV6_ADDRESS(ipbuffer, san->d.iPAddress->data); From f2d673fb18cc6a6c88bf588f39fd1aa9dcfec964 Mon Sep 17 00:00:00 2001 From: nielsdos <7771979+nielsdos@users.noreply.github.com> Date: Thu, 8 Jun 2023 21:22:20 +0200 Subject: [PATCH 37/54] Fix #70359 and #78577: segfaults with DOMNameSpaceNode * Fix type confusion and parent reference * Manually manage the lifetime of the parent * Add regression tests * Break out to a helper, and apply the use-after-free fix to xpath Closes GH-11402. --- NEWS | 3 + ext/dom/element.c | 25 ++---- ext/dom/php_dom.c | 61 ++++++++++++-- ext/dom/php_dom.h | 13 +++ ext/dom/tests/bug70359.phpt | 83 +++++++++++++++++++ ext/dom/tests/bug78577.phpt | 33 ++++++++ ext/dom/tests/xpath_domnamespacenode.phpt | 2 +- .../xpath_domnamespacenode_advanced.phpt | 75 +++++++++++++++++ ext/dom/xpath.c | 54 +++++------- 9 files changed, 292 insertions(+), 57 deletions(-) create mode 100644 ext/dom/tests/bug70359.phpt create mode 100644 ext/dom/tests/bug78577.phpt create mode 100644 ext/dom/tests/xpath_domnamespacenode_advanced.phpt diff --git a/NEWS b/NEWS index 8eae4ccaa5e7c..139c696374456 100644 --- a/NEWS +++ b/NEWS @@ -27,6 +27,9 @@ PHP NEWS with itself). (nielsdos) . Fixed bug #77686 (Removed elements are still returned by getElementById). (nielsdos) + . Fixed bug #70359 (print_r() on DOMAttr causes Segfault in + php_libxml_node_free_list()). (nielsdos) + . Fixed bug #78577 (Crash in DOMNameSpace debug info handlers). (nielsdos) - Opcache: . Fix allocation loop in zend_shared_alloc_startup(). (nielsdos) diff --git a/ext/dom/element.c b/ext/dom/element.c index 78113d72776bd..f84caa629cc66 100644 --- a/ext/dom/element.c +++ b/ext/dom/element.c @@ -150,6 +150,7 @@ int dom_element_schema_type_info_read(dom_object *obj, zval *retval) /* }}} */ +/* Note: the object returned is not necessarily a node, but can be an attribute or a namespace declaration. */ static xmlNodePtr dom_get_dom1_attribute(xmlNodePtr elem, xmlChar *name) /* {{{ */ { int len; @@ -376,25 +377,13 @@ PHP_METHOD(DOMElement, getAttributeNode) } if (attrp->type == XML_NAMESPACE_DECL) { - xmlNsPtr curns; - xmlNodePtr nsparent; - - nsparent = attrp->_private; - curns = xmlNewNs(NULL, attrp->name, NULL); - if (attrp->children) { - curns->prefix = xmlStrdup((xmlChar *) attrp->children); - } - if (attrp->children) { - attrp = xmlNewDocNode(nodep->doc, NULL, (xmlChar *) attrp->children, attrp->name); - } else { - attrp = xmlNewDocNode(nodep->doc, NULL, (xmlChar *)"xmlns", attrp->name); - } - attrp->type = XML_NAMESPACE_DECL; - attrp->parent = nsparent; - attrp->ns = curns; + xmlNsPtr original = (xmlNsPtr) attrp; + /* Keep parent alive, because we're a fake child. */ + GC_ADDREF(&intern->std); + (void) php_dom_create_fake_namespace_decl(nodep, original, return_value, intern); + } else { + DOM_RET_OBJ((xmlNodePtr) attrp, &ret, intern); } - - DOM_RET_OBJ((xmlNodePtr) attrp, &ret, intern); } /* }}} end dom_element_get_attribute_node */ diff --git a/ext/dom/php_dom.c b/ext/dom/php_dom.c index df20093221f16..9e0bb1f3d1d02 100644 --- a/ext/dom/php_dom.c +++ b/ext/dom/php_dom.c @@ -61,6 +61,7 @@ PHP_DOM_EXPORT zend_class_entry *dom_namespace_node_class_entry; zend_object_handlers dom_object_handlers; zend_object_handlers dom_nnodemap_object_handlers; +zend_object_handlers dom_object_namespace_node_handlers; #ifdef LIBXML_XPATH_ENABLED zend_object_handlers dom_xpath_object_handlers; #endif @@ -86,6 +87,9 @@ static HashTable dom_xpath_prop_handlers; #endif /* }}} */ +static zend_object *dom_objects_namespace_node_new(zend_class_entry *class_type); +static void dom_object_namespace_node_free_storage(zend_object *object); + typedef int (*dom_read_t)(dom_object *obj, zval *retval); typedef int (*dom_write_t)(dom_object *obj, zval *newval); @@ -570,6 +574,10 @@ PHP_MINIT_FUNCTION(dom) dom_nnodemap_object_handlers.read_dimension = dom_nodelist_read_dimension; dom_nnodemap_object_handlers.has_dimension = dom_nodelist_has_dimension; + memcpy(&dom_object_namespace_node_handlers, &dom_object_handlers, sizeof(zend_object_handlers)); + dom_object_namespace_node_handlers.offset = XtOffsetOf(dom_object_namespace_node, dom.std); + dom_object_namespace_node_handlers.free_obj = dom_object_namespace_node_free_storage; + zend_hash_init(&classes, 0, NULL, NULL, 1); dom_domexception_class_entry = register_class_DOMException(zend_ce_exception); @@ -604,7 +612,7 @@ PHP_MINIT_FUNCTION(dom) zend_hash_add_ptr(&classes, dom_node_class_entry->name, &dom_node_prop_handlers); dom_namespace_node_class_entry = register_class_DOMNameSpaceNode(); - dom_namespace_node_class_entry->create_object = dom_objects_new; + dom_namespace_node_class_entry->create_object = dom_objects_namespace_node_new; zend_hash_init(&dom_namespace_node_prop_handlers, 0, NULL, dom_dtor_prop_handler, 1); dom_register_prop_handler(&dom_namespace_node_prop_handlers, "nodeName", sizeof("nodeName")-1, dom_node_node_name_read, NULL); @@ -1001,10 +1009,8 @@ void dom_namednode_iter(dom_object *basenode, int ntype, dom_object *intern, xml } /* }}} */ -static dom_object* dom_objects_set_class(zend_class_entry *class_type) /* {{{ */ +static void dom_objects_set_class_ex(zend_class_entry *class_type, dom_object *intern) { - dom_object *intern = zend_object_alloc(sizeof(dom_object), class_type); - zend_class_entry *base_class = class_type; while ((base_class->type != ZEND_INTERNAL_CLASS || base_class->info.internal.module->module_number != dom_module_entry.module_number) && base_class->parent != NULL) { base_class = base_class->parent; @@ -1014,10 +1020,14 @@ static dom_object* dom_objects_set_class(zend_class_entry *class_type) /* {{{ */ zend_object_std_init(&intern->std, class_type); object_properties_init(&intern->std, class_type); +} +static dom_object* dom_objects_set_class(zend_class_entry *class_type) +{ + dom_object *intern = zend_object_alloc(sizeof(dom_object), class_type); + dom_objects_set_class_ex(class_type, intern); return intern; } -/* }}} */ /* {{{ dom_objects_new */ zend_object *dom_objects_new(zend_class_entry *class_type) @@ -1028,6 +1038,25 @@ zend_object *dom_objects_new(zend_class_entry *class_type) } /* }}} */ +static zend_object *dom_objects_namespace_node_new(zend_class_entry *class_type) +{ + dom_object_namespace_node *intern = zend_object_alloc(sizeof(dom_object_namespace_node), class_type); + dom_objects_set_class_ex(class_type, &intern->dom); + intern->dom.std.handlers = &dom_object_namespace_node_handlers; + return &intern->dom.std; +} + +static void dom_object_namespace_node_free_storage(zend_object *object) +{ + dom_object_namespace_node *intern = php_dom_namespace_node_obj_from_obj(object); + if (intern->parent_intern != NULL) { + zval tmp; + ZVAL_OBJ(&tmp, &intern->parent_intern->std); + zval_ptr_dtor(&tmp); + } + dom_objects_free_storage(object); +} + #ifdef LIBXML_XPATH_ENABLED /* {{{ zend_object dom_xpath_objects_new(zend_class_entry *class_type) */ zend_object *dom_xpath_objects_new(zend_class_entry *class_type) @@ -1550,6 +1579,28 @@ xmlNsPtr dom_get_nsdecl(xmlNode *node, xmlChar *localName) { } /* }}} end dom_get_nsdecl */ +/* Note: Assumes the additional lifetime was already added in the caller. */ +xmlNodePtr php_dom_create_fake_namespace_decl(xmlNodePtr nodep, xmlNsPtr original, zval *return_value, dom_object *parent_intern) +{ + xmlNodePtr attrp; + xmlNsPtr curns = xmlNewNs(NULL, original->href, NULL); + if (original->prefix) { + curns->prefix = xmlStrdup(original->prefix); + attrp = xmlNewDocNode(nodep->doc, NULL, (xmlChar *) original->prefix, original->href); + } else { + attrp = xmlNewDocNode(nodep->doc, NULL, (xmlChar *)"xmlns", original->href); + } + attrp->type = XML_NAMESPACE_DECL; + attrp->parent = nodep; + attrp->ns = curns; + + php_dom_create_object(attrp, return_value, parent_intern); + /* This object must exist, because we just created an object for it via php_dom_create_object(). */ + dom_object *obj = ((php_libxml_node_ptr *)attrp->_private)->_private; + php_dom_namespace_node_obj_from_obj(&obj->std)->parent_intern = parent_intern; + return attrp; +} + static zval *dom_nodelist_read_dimension(zend_object *object, zval *offset, int type, zval *rv) /* {{{ */ { zval offset_copy; diff --git a/ext/dom/php_dom.h b/ext/dom/php_dom.h index ac23d1fc25bb5..6ed382b6f84af 100644 --- a/ext/dom/php_dom.h +++ b/ext/dom/php_dom.h @@ -93,6 +93,18 @@ typedef struct { HashPosition pos; } php_dom_iterator; +typedef struct { + /* This may be a fake object that isn't actually in the children list of the parent. + * This is because some namespace declaration nodes aren't stored on the parent in libxml2, so we have to fake it. + * We could use a zval for this, but since this is always going to be an object let's save space... */ + dom_object *parent_intern; + dom_object dom; +} dom_object_namespace_node; + +static inline dom_object_namespace_node *php_dom_namespace_node_obj_from_obj(zend_object *obj) { + return (dom_object_namespace_node*)((char*)(obj) - XtOffsetOf(dom_object_namespace_node, dom.std)); +} + #include "domexception.h" dom_object *dom_object_get_data(xmlNodePtr obj); @@ -126,6 +138,7 @@ xmlNode *php_dom_libxml_hash_iter(xmlHashTable *ht, int index); xmlNode *php_dom_libxml_notation_iter(xmlHashTable *ht, int index); zend_object_iterator *php_dom_get_iterator(zend_class_entry *ce, zval *object, int by_ref); void dom_set_doc_classmap(php_libxml_ref_obj *document, zend_class_entry *basece, zend_class_entry *ce); +xmlNodePtr php_dom_create_fake_namespace_decl(xmlNodePtr nodep, xmlNsPtr original, zval *return_value, dom_object *parent_intern); void dom_parent_node_prepend(dom_object *context, zval *nodes, int nodesc); void dom_parent_node_append(dom_object *context, zval *nodes, int nodesc); diff --git a/ext/dom/tests/bug70359.phpt b/ext/dom/tests/bug70359.phpt new file mode 100644 index 0000000000000..b0a5ae57a3232 --- /dev/null +++ b/ext/dom/tests/bug70359.phpt @@ -0,0 +1,83 @@ +--TEST-- +Bug #70359 (print_r() on DOMAttr causes Segfault in php_libxml_node_free_list()) +--EXTENSIONS-- +dom +--FILE-- +loadXML(<< + +XML); +$spaceNode = $dom->documentElement->getAttributeNode('xmlns'); +print_r($spaceNode); + +echo "-- Test with parent and non-ns attribute --\n"; + +$dom = new DOMDocument(); +$dom->loadXML(<< + + + +XML); +$spaceNode = $dom->documentElement->firstElementChild->getAttributeNode('myattrib'); +var_dump($spaceNode->nodeType); +var_dump($spaceNode->nodeValue); + +$dom->documentElement->firstElementChild->remove(); +try { + print_r($spaceNode->parentNode); +} catch (\Error $e) { + echo $e->getMessage(), "\n"; +} + +echo "-- Test with parent and ns attribute --\n"; + +$dom = new DOMDocument(); +$dom->loadXML(<< + + + +XML); +$spaceNode = $dom->documentElement->firstElementChild->getAttributeNode('xmlns:xsi'); +print_r($spaceNode); + +$dom->documentElement->firstElementChild->remove(); +var_dump($spaceNode->parentNode->nodeName); // Shouldn't crash + +?> +--EXPECT-- +-- Test without parent -- +DOMNameSpaceNode Object +( + [nodeName] => xmlns + [nodeValue] => http://www.sitemaps.org/schemas/sitemap/0.9 + [nodeType] => 18 + [prefix] => + [localName] => xmlns + [namespaceURI] => http://www.sitemaps.org/schemas/sitemap/0.9 + [ownerDocument] => (object value omitted) + [parentNode] => (object value omitted) +) +-- Test with parent and non-ns attribute -- +int(2) +string(3) "bar" +Couldn't fetch DOMAttr. Node no longer exists +-- Test with parent and ns attribute -- +DOMNameSpaceNode Object +( + [nodeName] => xmlns:xsi + [nodeValue] => fooooooooooooooooooooo + [nodeType] => 18 + [prefix] => xsi + [localName] => xsi + [namespaceURI] => fooooooooooooooooooooo + [ownerDocument] => (object value omitted) + [parentNode] => (object value omitted) +) +string(3) "url" diff --git a/ext/dom/tests/bug78577.phpt b/ext/dom/tests/bug78577.phpt new file mode 100644 index 0000000000000..2631efc1e206c --- /dev/null +++ b/ext/dom/tests/bug78577.phpt @@ -0,0 +1,33 @@ +--TEST-- +Bug #78577 (Crash in DOMNameSpace debug info handlers) +--EXTENSIONS-- +dom +--FILE-- +loadXML(''); + +$attr = $doc->documentElement->getAttributeNode('xmlns'); +var_dump($attr); + +?> +--EXPECT-- +object(DOMNameSpaceNode)#3 (8) { + ["nodeName"]=> + string(5) "xmlns" + ["nodeValue"]=> + string(19) "http://php.net/test" + ["nodeType"]=> + int(18) + ["prefix"]=> + string(0) "" + ["localName"]=> + string(5) "xmlns" + ["namespaceURI"]=> + string(19) "http://php.net/test" + ["ownerDocument"]=> + string(22) "(object value omitted)" + ["parentNode"]=> + string(22) "(object value omitted)" +} diff --git a/ext/dom/tests/xpath_domnamespacenode.phpt b/ext/dom/tests/xpath_domnamespacenode.phpt index f0bfbed10dda6..97059c18e54da 100644 --- a/ext/dom/tests/xpath_domnamespacenode.phpt +++ b/ext/dom/tests/xpath_domnamespacenode.phpt @@ -17,7 +17,7 @@ var_dump($nodes->item(0)); ?> --EXPECT-- -object(DOMNameSpaceNode)#3 (8) { +object(DOMNameSpaceNode)#4 (8) { ["nodeName"]=> string(9) "xmlns:xml" ["nodeValue"]=> diff --git a/ext/dom/tests/xpath_domnamespacenode_advanced.phpt b/ext/dom/tests/xpath_domnamespacenode_advanced.phpt new file mode 100644 index 0000000000000..bbc49dc54652d --- /dev/null +++ b/ext/dom/tests/xpath_domnamespacenode_advanced.phpt @@ -0,0 +1,75 @@ +--TEST-- +DOMXPath::query() can return DOMNodeList with DOMNameSpaceNode items - advanced variation +--EXTENSIONS-- +dom +--FILE-- +loadXML(<<<'XML' + + Hello PHP! + +XML); + +$xpath = new DOMXPath($dom); +$query = '//namespace::*'; + +echo "-- All namespace attributes --\n"; + +foreach ($xpath->query($query) as $attribute) { + echo $attribute->nodeName . ' = ' . $attribute->nodeValue . PHP_EOL; + var_dump($attribute->parentNode->tagName); +} + +echo "-- All namespace attributes with removal attempt --\n"; + +foreach ($xpath->query($query) as $attribute) { + echo "Before: ", $attribute->parentNode->tagName, "\n"; + // Second & third attempt should fail because it's no longer in the document + try { + $attribute->parentNode->remove(); + } catch (\DOMException $e) { + echo $e->getMessage(), "\n"; + } + // However, it should not cause a use-after-free + echo "After: ", $attribute->parentNode->tagName, "\n"; +} + +?> +--EXPECT-- +-- All namespace attributes -- +xmlns:xml = http://www.w3.org/XML/1998/namespace +string(4) "root" +xmlns:bar = http://example.com/bar +string(4) "root" +xmlns:foo = http://example.com/foo +string(4) "root" +xmlns:xml = http://www.w3.org/XML/1998/namespace +string(5) "child" +xmlns:bar = http://example.com/bar +string(5) "child" +xmlns:foo = http://example.com/foo +string(5) "child" +xmlns:baz = http://example.com/baz +string(5) "child" +-- All namespace attributes with removal attempt -- +Before: root +After: root +Before: root +Not Found Error +After: root +Before: root +Not Found Error +After: root +Before: child +After: child +Before: child +Not Found Error +After: child +Before: child +Not Found Error +After: child +Before: child +Not Found Error +After: child diff --git a/ext/dom/xpath.c b/ext/dom/xpath.c index f546733a436d1..62e11f6b99bfb 100644 --- a/ext/dom/xpath.c +++ b/ext/dom/xpath.c @@ -101,24 +101,18 @@ static void dom_xpath_ext_function_php(xmlXPathParserContextPtr ctxt, int nargs, zval child; /* not sure, if we need this... it's copied from xpath.c */ if (node->type == XML_NAMESPACE_DECL) { - xmlNsPtr curns; - xmlNodePtr nsparent; - - nsparent = node->_private; - curns = xmlNewNs(NULL, node->name, NULL); - if (node->children) { - curns->prefix = xmlStrdup((xmlChar *) node->children); - } - if (node->children) { - node = xmlNewDocNode(node->doc, NULL, (xmlChar *) node->children, node->name); - } else { - node = xmlNewDocNode(node->doc, NULL, (xmlChar *) "xmlns", node->name); - } - node->type = XML_NAMESPACE_DECL; - node->parent = nsparent; - node->ns = curns; + xmlNodePtr nsparent = node->_private; + xmlNsPtr original = (xmlNsPtr) node; + + /* Make sure parent dom object exists, so we can take an extra reference. */ + zval parent_zval; /* don't destroy me, my lifetime is transfered to the fake namespace decl */ + php_dom_create_object(nsparent, &parent_zval, &intern->dom); + dom_object *parent_intern = Z_DOMOBJ_P(&parent_zval); + + node = php_dom_create_fake_namespace_decl(nsparent, original, &child, parent_intern); + } else { + php_dom_create_object(node, &child, &intern->dom); } - php_dom_create_object(node, &child, &intern->dom); add_next_index_zval(&fci.params[i], &child); } } else { @@ -421,24 +415,18 @@ static void php_xpath_eval(INTERNAL_FUNCTION_PARAMETERS, int type) /* {{{ */ zval child; if (node->type == XML_NAMESPACE_DECL) { - xmlNsPtr curns; - xmlNodePtr nsparent; + xmlNodePtr nsparent = node->_private; + xmlNsPtr original = (xmlNsPtr) node; - nsparent = node->_private; - curns = xmlNewNs(NULL, node->name, NULL); - if (node->children) { - curns->prefix = xmlStrdup((xmlChar *) node->children); - } - if (node->children) { - node = xmlNewDocNode(docp, NULL, (xmlChar *) node->children, node->name); - } else { - node = xmlNewDocNode(docp, NULL, (xmlChar *) "xmlns", node->name); - } - node->type = XML_NAMESPACE_DECL; - node->parent = nsparent; - node->ns = curns; + /* Make sure parent dom object exists, so we can take an extra reference. */ + zval parent_zval; /* don't destroy me, my lifetime is transfered to the fake namespace decl */ + php_dom_create_object(nsparent, &parent_zval, &intern->dom); + dom_object *parent_intern = Z_DOMOBJ_P(&parent_zval); + + node = php_dom_create_fake_namespace_decl(nsparent, original, &child, parent_intern); + } else { + php_dom_create_object(node, &child, &intern->dom); } - php_dom_create_object(node, &child, &intern->dom); add_next_index_zval(&retval, &child); } } else { From e309fd84610802c67413fb48284e85495034e7a9 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sat, 10 Jun 2023 22:48:16 +0200 Subject: [PATCH 38/54] Fix lifetime issue with getAttributeNodeNS() It's the same issue that I fixed previously in GH-11402, but in a different place. Closes GH-11422. --- NEWS | 1 + ext/dom/element.c | 20 ++++--------------- ...ifetime_parentNode_getAttributeNodeNS.phpt | 20 +++++++++++++++++++ 3 files changed, 25 insertions(+), 16 deletions(-) create mode 100644 ext/dom/tests/bug_lifetime_parentNode_getAttributeNodeNS.phpt diff --git a/NEWS b/NEWS index 139c696374456..776745d073a38 100644 --- a/NEWS +++ b/NEWS @@ -30,6 +30,7 @@ PHP NEWS . Fixed bug #70359 (print_r() on DOMAttr causes Segfault in php_libxml_node_free_list()). (nielsdos) . Fixed bug #78577 (Crash in DOMNameSpace debug info handlers). (nielsdos) + . Fix lifetime issue with getAttributeNodeNS(). (nielsdos) - Opcache: . Fix allocation loop in zend_shared_alloc_startup(). (nielsdos) diff --git a/ext/dom/element.c b/ext/dom/element.c index f84caa629cc66..44c576a07363f 100644 --- a/ext/dom/element.c +++ b/ext/dom/element.c @@ -787,7 +787,7 @@ Since: DOM Level 2 PHP_METHOD(DOMElement, getAttributeNodeNS) { zval *id; - xmlNodePtr elemp, fakeAttrp; + xmlNodePtr elemp; xmlAttrPtr attrp; dom_object *intern; size_t uri_len, name_len; @@ -808,21 +808,9 @@ PHP_METHOD(DOMElement, getAttributeNodeNS) xmlNsPtr nsptr; nsptr = dom_get_nsdecl(elemp, (xmlChar *)name); if (nsptr != NULL) { - xmlNsPtr curns; - curns = xmlNewNs(NULL, nsptr->href, NULL); - if (nsptr->prefix) { - curns->prefix = xmlStrdup((xmlChar *) nsptr->prefix); - } - if (nsptr->prefix) { - fakeAttrp = xmlNewDocNode(elemp->doc, NULL, (xmlChar *) nsptr->prefix, nsptr->href); - } else { - fakeAttrp = xmlNewDocNode(elemp->doc, NULL, (xmlChar *)"xmlns", nsptr->href); - } - fakeAttrp->type = XML_NAMESPACE_DECL; - fakeAttrp->parent = elemp; - fakeAttrp->ns = curns; - - DOM_RET_OBJ(fakeAttrp, &ret, intern); + /* Keep parent alive, because we're a fake child. */ + GC_ADDREF(&intern->std); + (void) php_dom_create_fake_namespace_decl(elemp, nsptr, return_value, intern); } else { RETURN_NULL(); } diff --git a/ext/dom/tests/bug_lifetime_parentNode_getAttributeNodeNS.phpt b/ext/dom/tests/bug_lifetime_parentNode_getAttributeNodeNS.phpt new file mode 100644 index 0000000000000..3c53e08d4db76 --- /dev/null +++ b/ext/dom/tests/bug_lifetime_parentNode_getAttributeNodeNS.phpt @@ -0,0 +1,20 @@ +--TEST-- +Lifetime issue with parentNode on getAttributeNodeNS() +--EXTENSIONS-- +dom +--FILE-- + + + +'; + +$xml=new DOMDocument(); +$xml->loadXML($xmlString); +$ns2 = $xml->documentElement->getAttributeNodeNS("http://www.w3.org/2000/xmlns/", "ns2"); +$ns2->parentNode->remove(); +var_dump($ns2->parentNode->localName); + +?> +--EXPECT-- +string(4) "root" From 10d94aca4c5ee7a101ed39bc395bcc1bb9d68507 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 11 Jun 2023 23:44:58 +0200 Subject: [PATCH 39/54] Fix "invalid state error" with cloned namespace declarations Closes GH-11429. --- NEWS | 1 + ext/dom/php_dom.c | 57 +++++++++++++++++++++------ ext/dom/tests/clone_nodes.phpt | 72 ++++++++++++++++++++++++++++++++++ 3 files changed, 117 insertions(+), 13 deletions(-) create mode 100644 ext/dom/tests/clone_nodes.phpt diff --git a/NEWS b/NEWS index 776745d073a38..cad4653438930 100644 --- a/NEWS +++ b/NEWS @@ -31,6 +31,7 @@ PHP NEWS php_libxml_node_free_list()). (nielsdos) . Fixed bug #78577 (Crash in DOMNameSpace debug info handlers). (nielsdos) . Fix lifetime issue with getAttributeNodeNS(). (nielsdos) + . Fix "invalid state error" with cloned namespace declarations. (nielsdos) - Opcache: . Fix allocation loop in zend_shared_alloc_startup(). (nielsdos) diff --git a/ext/dom/php_dom.c b/ext/dom/php_dom.c index 9e0bb1f3d1d02..454dc54d8e211 100644 --- a/ext/dom/php_dom.c +++ b/ext/dom/php_dom.c @@ -89,6 +89,7 @@ static HashTable dom_xpath_prop_handlers; static zend_object *dom_objects_namespace_node_new(zend_class_entry *class_type); static void dom_object_namespace_node_free_storage(zend_object *object); +static xmlNodePtr php_dom_create_fake_namespace_decl_node_ptr(xmlNodePtr nodep, xmlNsPtr original); typedef int (*dom_read_t)(dom_object *obj, zval *retval); typedef int (*dom_write_t)(dom_object *obj, zval *newval); @@ -477,6 +478,19 @@ PHP_FUNCTION(dom_import_simplexml) static dom_object* dom_objects_set_class(zend_class_entry *class_type); +static void dom_update_refcount_after_clone(dom_object *original, xmlNodePtr original_node, dom_object *clone, xmlNodePtr cloned_node) +{ + /* If we cloned a document then we must create new doc proxy */ + if (cloned_node->doc == original_node->doc) { + clone->document = original->document; + } + php_libxml_increment_doc_ref((php_libxml_node_object *)clone, cloned_node->doc); + php_libxml_increment_node_ptr((php_libxml_node_object *)clone, cloned_node, (void *)clone); + if (original->document != clone->document) { + dom_copy_doc_props(original->document, clone->document); + } +} + static zend_object *dom_objects_store_clone_obj(zend_object *zobject) /* {{{ */ { dom_object *intern = php_dom_obj_from_obj(zobject); @@ -489,15 +503,7 @@ static zend_object *dom_objects_store_clone_obj(zend_object *zobject) /* {{{ */ if (node != NULL) { xmlNodePtr cloned_node = xmlDocCopyNode(node, node->doc, 1); if (cloned_node != NULL) { - /* If we cloned a document then we must create new doc proxy */ - if (cloned_node->doc == node->doc) { - clone->document = intern->document; - } - php_libxml_increment_doc_ref((php_libxml_node_object *)clone, cloned_node->doc); - php_libxml_increment_node_ptr((php_libxml_node_object *)clone, cloned_node, (void *)clone); - if (intern->document != clone->document) { - dom_copy_doc_props(intern->document, clone->document); - } + dom_update_refcount_after_clone(intern, node, clone, cloned_node); } } @@ -509,6 +515,26 @@ static zend_object *dom_objects_store_clone_obj(zend_object *zobject) /* {{{ */ } /* }}} */ +static zend_object *dom_object_namespace_node_clone_obj(zend_object *zobject) +{ + dom_object_namespace_node *intern = php_dom_namespace_node_obj_from_obj(zobject); + zend_object *clone = dom_objects_namespace_node_new(intern->dom.std.ce); + dom_object_namespace_node *clone_intern = php_dom_namespace_node_obj_from_obj(clone); + + xmlNodePtr original_node = dom_object_get_node(&intern->dom); + ZEND_ASSERT(original_node->type == XML_NAMESPACE_DECL); + xmlNodePtr cloned_node = php_dom_create_fake_namespace_decl_node_ptr(original_node->parent, original_node->ns); + + if (intern->parent_intern) { + clone_intern->parent_intern = intern->parent_intern; + GC_ADDREF(&clone_intern->parent_intern->std); + } + dom_update_refcount_after_clone(&intern->dom, original_node, &clone_intern->dom, cloned_node); + + zend_objects_clone_members(clone, &intern->dom.std); + return clone; +} + static void dom_copy_prop_handler(zval *zv) /* {{{ */ { dom_prop_handler *hnd = Z_PTR_P(zv); @@ -577,6 +603,7 @@ PHP_MINIT_FUNCTION(dom) memcpy(&dom_object_namespace_node_handlers, &dom_object_handlers, sizeof(zend_object_handlers)); dom_object_namespace_node_handlers.offset = XtOffsetOf(dom_object_namespace_node, dom.std); dom_object_namespace_node_handlers.free_obj = dom_object_namespace_node_free_storage; + dom_object_namespace_node_handlers.clone_obj = dom_object_namespace_node_clone_obj; zend_hash_init(&classes, 0, NULL, NULL, 1); @@ -1579,8 +1606,7 @@ xmlNsPtr dom_get_nsdecl(xmlNode *node, xmlChar *localName) { } /* }}} end dom_get_nsdecl */ -/* Note: Assumes the additional lifetime was already added in the caller. */ -xmlNodePtr php_dom_create_fake_namespace_decl(xmlNodePtr nodep, xmlNsPtr original, zval *return_value, dom_object *parent_intern) +static xmlNodePtr php_dom_create_fake_namespace_decl_node_ptr(xmlNodePtr nodep, xmlNsPtr original) { xmlNodePtr attrp; xmlNsPtr curns = xmlNewNs(NULL, original->href, NULL); @@ -1593,11 +1619,16 @@ xmlNodePtr php_dom_create_fake_namespace_decl(xmlNodePtr nodep, xmlNsPtr origina attrp->type = XML_NAMESPACE_DECL; attrp->parent = nodep; attrp->ns = curns; + return attrp; +} +/* Note: Assumes the additional lifetime was already added in the caller. */ +xmlNodePtr php_dom_create_fake_namespace_decl(xmlNodePtr nodep, xmlNsPtr original, zval *return_value, dom_object *parent_intern) +{ + xmlNodePtr attrp = php_dom_create_fake_namespace_decl_node_ptr(nodep, original); php_dom_create_object(attrp, return_value, parent_intern); /* This object must exist, because we just created an object for it via php_dom_create_object(). */ - dom_object *obj = ((php_libxml_node_ptr *)attrp->_private)->_private; - php_dom_namespace_node_obj_from_obj(&obj->std)->parent_intern = parent_intern; + php_dom_namespace_node_obj_from_obj(Z_OBJ_P(return_value))->parent_intern = parent_intern; return attrp; } diff --git a/ext/dom/tests/clone_nodes.phpt b/ext/dom/tests/clone_nodes.phpt new file mode 100644 index 0000000000000..1841c702caf8d --- /dev/null +++ b/ext/dom/tests/clone_nodes.phpt @@ -0,0 +1,72 @@ +--TEST-- +Clone nodes +--EXTENSIONS-- +dom +--FILE-- +loadXML(''); + +$attr = $doc->documentElement->getAttributeNode('xmlns'); +var_dump($attr); + +$attrClone = clone $attr; +var_dump($attrClone->nodeValue); +var_dump($attrClone->parentNode->nodeName); + +unset($doc); +unset($attr); + +var_dump($attrClone->nodeValue); +var_dump($attrClone->parentNode->nodeName); + +echo "-- Clone DOMNode --\n"; + +$doc = new DOMDocument; +$doc->loadXML(''); + +$bar = $doc->documentElement->firstChild; +$barClone = clone $bar; +$bar->remove(); +unset($bar); + +var_dump($barClone->nodeName); + +$doc->firstElementChild->remove(); +unset($doc); + +var_dump($barClone->nodeName); +var_dump($barClone->parentNode); + +?> +--EXPECT-- +-- Clone DOMNameSpaceNode -- +object(DOMNameSpaceNode)#3 (8) { + ["nodeName"]=> + string(5) "xmlns" + ["nodeValue"]=> + string(19) "http://php.net/test" + ["nodeType"]=> + int(18) + ["prefix"]=> + string(0) "" + ["localName"]=> + string(5) "xmlns" + ["namespaceURI"]=> + string(19) "http://php.net/test" + ["ownerDocument"]=> + string(22) "(object value omitted)" + ["parentNode"]=> + string(22) "(object value omitted)" +} +string(19) "http://php.net/test" +string(3) "foo" +string(19) "http://php.net/test" +string(3) "foo" +-- Clone DOMNode -- +string(3) "bar" +string(3) "bar" +NULL From a8a3b99e00747f3a1198c526674c9dad513a203f Mon Sep 17 00:00:00 2001 From: nielsdos <7771979+nielsdos@users.noreply.github.com> Date: Mon, 12 Jun 2023 23:58:34 +0200 Subject: [PATCH 40/54] Fix GH-11433: Unable to set CURLOPT_ACCEPT_ENCODING to NULL Closes GH-11446. --- NEWS | 4 ++ ext/curl/interface.c | 2 +- .../curl_setopt_CURLOPT_ACCEPT_ENCODING.phpt | 38 +++++++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 ext/curl/tests/curl_setopt_CURLOPT_ACCEPT_ENCODING.phpt diff --git a/NEWS b/NEWS index cad4653438930..6baaae22e8629 100644 --- a/NEWS +++ b/NEWS @@ -9,6 +9,10 @@ PHP NEWS - Core: . Fixed build for the riscv64 architecture/GCC 12. (Daniil Gentili) +- Curl: + . Fixed bug GH-11433 (Unable to set CURLOPT_ACCEPT_ENCODING to NULL). + (nielsdos) + - DOM: . Fixed bugs GH-11288 and GH-11289 and GH-11290 and GH-9142 (DOMExceptions and segfaults with replaceWith). (nielsdos) diff --git a/ext/curl/interface.c b/ext/curl/interface.c index 025c876ad5bcd..807b27cb78c90 100644 --- a/ext/curl/interface.c +++ b/ext/curl/interface.c @@ -2493,7 +2493,6 @@ static int _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue, bool i case CURLOPT_TLSAUTH_TYPE: case CURLOPT_TLSAUTH_PASSWORD: case CURLOPT_TLSAUTH_USERNAME: - case CURLOPT_ACCEPT_ENCODING: case CURLOPT_TRANSFER_ENCODING: case CURLOPT_DNS_SERVERS: case CURLOPT_MAIL_AUTH: @@ -2553,6 +2552,7 @@ static int _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue, bool i case CURLOPT_RANGE: case CURLOPT_FTP_ACCOUNT: case CURLOPT_RTSP_SESSION_ID: + case CURLOPT_ACCEPT_ENCODING: #if LIBCURL_VERSION_NUM >= 0x072100 /* Available since 7.33.0 */ case CURLOPT_DNS_INTERFACE: case CURLOPT_DNS_LOCAL_IP4: diff --git a/ext/curl/tests/curl_setopt_CURLOPT_ACCEPT_ENCODING.phpt b/ext/curl/tests/curl_setopt_CURLOPT_ACCEPT_ENCODING.phpt new file mode 100644 index 0000000000000..c170308c2e981 --- /dev/null +++ b/ext/curl/tests/curl_setopt_CURLOPT_ACCEPT_ENCODING.phpt @@ -0,0 +1,38 @@ +--TEST-- +Test curl_setopt() with CURLOPT_ACCEPT_ENCODING +--EXTENSIONS-- +curl +--FILE-- + +--EXPECTF-- +GET /get.inc?test= HTTP/1.1 +Host: %s +Accept: */* +Accept-Encoding: gzip + +GET /get.inc?test= HTTP/1.1 +Host: %s +Accept: */* From 4fcb3e0d343bb2a4bd405fce816020986e040434 Mon Sep 17 00:00:00 2001 From: Peter Date: Tue, 13 Jun 2023 01:19:11 +0800 Subject: [PATCH 41/54] Fix cross-compilation check in phar generation for FreeBSD FreeBSD's shell is very POSIX strict. This patch makes sure it works correctly under FreeBSD too. Closes GH-11441. --- NEWS | 3 +++ ext/phar/Makefile.frag | 6 +++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/NEWS b/NEWS index 6baaae22e8629..2335b5d7d397a 100644 --- a/NEWS +++ b/NEWS @@ -47,6 +47,9 @@ PHP NEWS . Fixed bug GH-9356 Incomplete validation of IPv6 Address fields in subjectAltNames (James Lucas, Jakub Zelenka). +- Phar: + . Fix cross-compilation check in phar generation for FreeBSD. (peter279k) + - SPL: . Fixed bug GH-11338 (SplFileInfo empty getBasename with more than one slash). (nielsdos) diff --git a/ext/phar/Makefile.frag b/ext/phar/Makefile.frag index e5646b2029261..7a867dd7df28f 100644 --- a/ext/phar/Makefile.frag +++ b/ext/phar/Makefile.frag @@ -35,7 +35,7 @@ TEST_PHP_EXECUTABLE_RES = $(shell echo "$(TEST_PHP_EXECUTABLE)" | grep -c 'Exec $(builddir)/phar.php: $(srcdir)/build_precommand.php $(srcdir)/phar/*.inc $(srcdir)/phar/*.php $(SAPI_CLI_PATH) -@(echo "Generating phar.php"; \ - if [ $(TEST_PHP_EXECUTABLE_RES) -ne 1 ]; then \ + if [ "$(TEST_PHP_EXECUTABLE_RES)" != 1 ]; then \ $(PHP_PHARCMD_EXECUTABLE) $(PHP_PHARCMD_SETTINGS) $(srcdir)/build_precommand.php > $(builddir)/phar.php; \ else \ echo "Skipping phar.php generating during cross compilation"; \ @@ -43,7 +43,7 @@ $(builddir)/phar.php: $(srcdir)/build_precommand.php $(srcdir)/phar/*.inc $(srcd $(builddir)/phar.phar: $(builddir)/phar.php $(builddir)/phar/phar.inc $(srcdir)/phar/*.inc $(srcdir)/phar/*.php $(SAPI_CLI_PATH) -@(echo "Generating phar.phar"; \ - if [ $(TEST_PHP_EXECUTABLE_RES) -ne 1 ]; then \ + if [ "$(TEST_PHP_EXECUTABLE_RES)" != 1 ]; then \ rm -f $(builddir)/phar.phar; \ rm -f $(srcdir)/phar.phar; \ $(PHP_PHARCMD_EXECUTABLE) $(PHP_PHARCMD_SETTINGS) $(builddir)/phar.php pack -f $(builddir)/phar.phar -a pharcommand -c auto -x \\.svn -p 0 -s $(srcdir)/phar/phar.php -h sha1 -b "$(PHP_PHARCMD_BANG)" $(srcdir)/phar/; \ @@ -53,7 +53,7 @@ $(builddir)/phar.phar: $(builddir)/phar.php $(builddir)/phar/phar.inc $(srcdir)/ fi) install-pharcmd: pharcmd - @(if [ $(TEST_PHP_EXECUTABLE_RES) -ne 1 ]; then \ + @(if [ "$(TEST_PHP_EXECUTABLE_RES)" != 1 ]; then \ $(mkinstalldirs) $(INSTALL_ROOT)$(bindir); \ $(INSTALL) $(builddir)/phar.phar $(INSTALL_ROOT)$(bindir)/$(program_prefix)phar$(program_suffix).phar; \ rm -f $(INSTALL_ROOT)$(bindir)/$(program_prefix)phar$(program_suffix); \ From 7ade242e28a8139f38f585b08fd21711b8b8823d Mon Sep 17 00:00:00 2001 From: Mikhail Galanin Date: Thu, 25 May 2023 13:22:57 +0100 Subject: [PATCH 42/54] sapi/fpm: add "pcntl" when running test depending pcntl_sigprocmask() If "pcntl" is built as a shared module, the extension will not load automatically when we spawn the FPM --- sapi/fpm/tests/reload-uses-sigkill-as-last-measure.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sapi/fpm/tests/reload-uses-sigkill-as-last-measure.phpt b/sapi/fpm/tests/reload-uses-sigkill-as-last-measure.phpt index 3e5d19c429b8e..339022b513e31 100644 --- a/sapi/fpm/tests/reload-uses-sigkill-as-last-measure.phpt +++ b/sapi/fpm/tests/reload-uses-sigkill-as-last-measure.phpt @@ -35,7 +35,7 @@ pcntl_sigprocmask(SIG_BLOCK, [SIGQUIT, SIGTERM]); EOT; $tester = new FPM\Tester($cfg, $code); -$tester->start(); +$tester->start(extensions: ['pcntl']); $tester->expectLogStartNotices(); $tester->request()->expectEmptyBody(); $tester->signal('USR2'); From 9b184663967a2201cbcf3edbfb00f09ba0b1fdc0 Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Thu, 15 Jun 2023 17:36:00 +0100 Subject: [PATCH 43/54] FPM: Add "pcntl" when running another test depending on pcntl --- sapi/fpm/tests/bug77023-pm-dynamic-blocking-sigquit.phpt | 2 +- sapi/fpm/tests/reload-uses-sigkill-as-last-measure.phpt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sapi/fpm/tests/bug77023-pm-dynamic-blocking-sigquit.phpt b/sapi/fpm/tests/bug77023-pm-dynamic-blocking-sigquit.phpt index cb33a2f6635a7..458948f676590 100644 --- a/sapi/fpm/tests/bug77023-pm-dynamic-blocking-sigquit.phpt +++ b/sapi/fpm/tests/bug77023-pm-dynamic-blocking-sigquit.phpt @@ -36,7 +36,7 @@ EOT; $tester = new FPM\Tester($cfg, $code); -$tester->start(); +$tester->start(extensions: ['pcntl']); $tester->expectLogStartNotices(); $tester->multiRequest(2); $tester->status([ diff --git a/sapi/fpm/tests/reload-uses-sigkill-as-last-measure.phpt b/sapi/fpm/tests/reload-uses-sigkill-as-last-measure.phpt index 339022b513e31..64dd8e736b603 100644 --- a/sapi/fpm/tests/reload-uses-sigkill-as-last-measure.phpt +++ b/sapi/fpm/tests/reload-uses-sigkill-as-last-measure.phpt @@ -1,5 +1,5 @@ --TEST-- -If SIGQUIT and SIGTERM during reloading fail, SIGKILL should be sent +FPM: If SIGQUIT and SIGTERM during reloading fail, SIGKILL should be sent --EXTENSIONS-- pcntl --SKIPIF-- From b30be40b86b62fc681c432fd96840d8e57e172a5 Mon Sep 17 00:00:00 2001 From: nielsdos <7771979+nielsdos@users.noreply.github.com> Date: Wed, 14 Jun 2023 21:49:31 +0200 Subject: [PATCH 44/54] Fix bug #55294 and #47530 and #47847: namespace reconciliation issues We'll use the DOM wrapper version of libxml2 instead of the regular one. It's conforming to the behaviour we expect of DOM. Most of this patch is tests. I based and extended the tests on the code attached with the aforementioned bug reports. Therefore the credits for the tests: Co-authored-by: hilse at web dot de Co-authored-by: robin2008 at altruists dot org Co-authored-by: sgunderson at bigfoot dot com We'll also change the searching point of the internal reconciliation to start at the top of the added tree to avoid redundant work now that the function is changed. Closes GH-11454. --- NEWS | 2 + ext/dom/php_dom.c | 33 +++++--- ext/dom/tests/bug47530.phpt | 152 ++++++++++++++++++++++++++++++++++++ ext/dom/tests/bug47847.phpt | 27 +++++++ ext/dom/tests/bug55294.phpt | 29 +++++++ 5 files changed, 233 insertions(+), 10 deletions(-) create mode 100644 ext/dom/tests/bug47530.phpt create mode 100644 ext/dom/tests/bug47847.phpt create mode 100644 ext/dom/tests/bug55294.phpt diff --git a/NEWS b/NEWS index 2335b5d7d397a..b89fb82ac442c 100644 --- a/NEWS +++ b/NEWS @@ -36,6 +36,8 @@ PHP NEWS . Fixed bug #78577 (Crash in DOMNameSpace debug info handlers). (nielsdos) . Fix lifetime issue with getAttributeNodeNS(). (nielsdos) . Fix "invalid state error" with cloned namespace declarations. (nielsdos) + . Fixed bug #55294 and #47530 and #47847 (various namespace reconciliation + issues). (nielsdos) - Opcache: . Fix allocation loop in zend_shared_alloc_startup(). (nielsdos) diff --git a/ext/dom/php_dom.c b/ext/dom/php_dom.c index 454dc54d8e211..4a6ab2fee9e98 100644 --- a/ext/dom/php_dom.c +++ b/ext/dom/php_dom.c @@ -1441,7 +1441,7 @@ void dom_set_old_ns(xmlDoc *doc, xmlNs *ns) { } /* }}} end dom_set_old_ns */ -static void dom_reconcile_ns_internal(xmlDocPtr doc, xmlNodePtr nodep) +static void dom_reconcile_ns_internal(xmlDocPtr doc, xmlNodePtr nodep, xmlNodePtr search_parent) { xmlNsPtr nsptr, nsdftptr, curns, prevns = NULL; @@ -1451,7 +1451,7 @@ static void dom_reconcile_ns_internal(xmlDocPtr doc, xmlNodePtr nodep) while (curns) { nsdftptr = curns->next; if (curns->href != NULL) { - if((nsptr = xmlSearchNsByHref(doc, nodep->parent, curns->href)) && + if((nsptr = xmlSearchNsByHref(doc, search_parent, curns->href)) && (curns->prefix == NULL || xmlStrEqual(nsptr->prefix, curns->prefix))) { curns->next = NULL; if (prevns == NULL) { @@ -1469,23 +1469,34 @@ static void dom_reconcile_ns_internal(xmlDocPtr doc, xmlNodePtr nodep) } } +static void dom_libxml_reconcile_ensure_namespaces_are_declared(xmlNodePtr nodep) +{ + /* Put on stack to avoid allocation. + * Although libxml2 currently does not use this for the reconciliation, it still + * makes sense to do this just in case libxml2's internal change in the future. */ + xmlDOMWrapCtxt dummy_ctxt = {0}; + xmlDOMWrapReconcileNamespaces(&dummy_ctxt, nodep, /* options */ 0); +} + void dom_reconcile_ns(xmlDocPtr doc, xmlNodePtr nodep) /* {{{ */ { + /* Although the node type will be checked by the libxml2 API, + * we still want to do the internal reconciliation conditionally. */ if (nodep->type == XML_ELEMENT_NODE) { - dom_reconcile_ns_internal(doc, nodep); - xmlReconciliateNs(doc, nodep); + dom_reconcile_ns_internal(doc, nodep, nodep->parent); + dom_libxml_reconcile_ensure_namespaces_are_declared(nodep); } } /* }}} */ -static void dom_reconcile_ns_list_internal(xmlDocPtr doc, xmlNodePtr nodep, xmlNodePtr last) +static void dom_reconcile_ns_list_internal(xmlDocPtr doc, xmlNodePtr nodep, xmlNodePtr last, xmlNodePtr search_parent) { ZEND_ASSERT(nodep != NULL); while (true) { if (nodep->type == XML_ELEMENT_NODE) { - dom_reconcile_ns_internal(doc, nodep); + dom_reconcile_ns_internal(doc, nodep, search_parent); if (nodep->children) { - dom_reconcile_ns_list_internal(doc, nodep->children, nodep->last /* process the whole children list */); + dom_reconcile_ns_list_internal(doc, nodep->children, nodep->last /* process the whole children list */, search_parent); } } if (nodep == last) { @@ -1497,10 +1508,12 @@ static void dom_reconcile_ns_list_internal(xmlDocPtr doc, xmlNodePtr nodep, xmlN void dom_reconcile_ns_list(xmlDocPtr doc, xmlNodePtr nodep, xmlNodePtr last) { - dom_reconcile_ns_list_internal(doc, nodep, last); - /* Outside of the recursion above because xmlReconciliateNs() performs its own recursion. */ + dom_reconcile_ns_list_internal(doc, nodep, last, nodep->parent); + /* The loop is outside of the recursion in the above call because + * dom_libxml_reconcile_ensure_namespaces_are_declared() performs its own recursion. */ while (true) { - xmlReconciliateNs(doc, nodep); + /* The internal libxml2 call will already check the node type, no need for us to do it here. */ + dom_libxml_reconcile_ensure_namespaces_are_declared(nodep); if (nodep == last) { break; } diff --git a/ext/dom/tests/bug47530.phpt b/ext/dom/tests/bug47530.phpt new file mode 100644 index 0000000000000..0fb990e0e7bff --- /dev/null +++ b/ext/dom/tests/bug47530.phpt @@ -0,0 +1,152 @@ +--TEST-- +Bug #47530 (Importing objects into document fragments creates bogus "default" namespace) +--EXTENSIONS-- +dom +--FILE-- +loadXML(''); + $root = $doc->documentElement; + $frag = $doc->createDocumentFragment(); + $frag->appendChild($doc->importNode($root->firstChild)); + $root->appendChild($frag); + echo $doc->saveXML(); +} + +function test_document_fragment_without_import() { + $doc = new DOMDocument; + $doc->loadXML(''); + $frag = $doc->createDocumentFragment(); + $frag->appendChild($doc->createElementNS('https://php.net/bar', 'bar')); + $frag->appendChild($doc->createElementNS('', 'bar')); + $element = $doc->documentElement->firstChild; + $element->appendChild($frag); + unset($frag); // Free fragment, should not break getting the namespaceURI below + echo $doc->saveXML(); + unset($doc); + var_dump($element->firstChild->tagName); + var_dump($element->firstChild->namespaceURI); +} + +function test_document_import() { + $xml = << + +
+

Test-Text

+
+ +XML; + + $dom = new DOMDocument(); + $dom->loadXML($xml); + + $dom2 = new DOMDocument(); + $importedNode = $dom2->importNode($dom->documentElement, true); + $dom2->appendChild($importedNode); + + echo $dom2->saveXML(); +} + +function test_partial_document_import() { + $xml = << + +
+

Test-Text

+ More test text + Even more test text +
+ +XML; + + $dom = new DOMDocument(); + $dom->loadXML($xml); + + $dom2 = new DOMDocument(); + $dom2->loadXML(''); + $importedNode = $dom2->importNode($dom->documentElement, true); + $dom2->documentElement->appendChild($importedNode); + + // Freeing the original document shouldn't break the other document + unset($importedNode); + unset($dom); + + echo $dom2->saveXML(); +} + +function test_document_import_with_attributes() { + $dom = new DOMDocument(); + $dom->loadXML('

'); + $dom2 = new DOMDocument(); + $dom2->loadXML('
'); + $dom2->documentElement->appendChild($dom2->importNode($dom->documentElement->firstChild)); + echo $dom2->saveXML(), "\n"; + + $dom2->documentElement->firstChild->appendChild($dom2->importNode($dom->documentElement->firstChild->nextSibling)); + echo $dom2->saveXML(), "\n"; +} + +function test_appendChild_with_shadowing() { + $dom = new DOMDocument(); + $dom->loadXML(''); + + $a = $dom->documentElement->firstElementChild; + $b = $a->nextSibling; + $b->remove(); + $a->appendChild($b); + + echo $dom->saveXML(), "\n"; +} + +echo "-- Test document fragment with import --\n"; +test_document_fragment_with_import(); +echo "-- Test document fragment without import --\n"; +test_document_fragment_without_import(); +echo "-- Test document import --\n"; +test_document_import(); +echo "-- Test partial document import --\n"; +test_partial_document_import(); +echo "-- Test document import with attributes --\n"; +test_document_import_with_attributes(); +echo "-- Test appendChild with shadowing --\n"; +test_appendChild_with_shadowing(); + +?> +--EXPECT-- +-- Test document fragment with import -- + + +-- Test document fragment without import -- + + +string(7) "foo:bar" +string(19) "https://php.net/bar" +-- Test document import -- + + +
+

Test-Text

+
+ +-- Test partial document import -- + + +
+

Test-Text

+ More test text + Even more test text +
+ +-- Test document import with attributes -- + +

+ + +

+ +-- Test appendChild with shadowing -- + + diff --git a/ext/dom/tests/bug47847.phpt b/ext/dom/tests/bug47847.phpt new file mode 100644 index 0000000000000..324bf9508d5ce --- /dev/null +++ b/ext/dom/tests/bug47847.phpt @@ -0,0 +1,27 @@ +--TEST-- +Bug #47847 (importNode loses the namespace of an XML element) +--EXTENSIONS-- +dom +--FILE-- +loadXML(<< + + + + + +XML); + +$aDOM = new DOMDocument(); +$imported = $aDOM->importNode($fromdom->documentElement->firstElementChild, true); +$aDOM->appendChild($imported); + +echo $aDOM->saveXML(); +?> +--EXPECT-- + + + + diff --git a/ext/dom/tests/bug55294.phpt b/ext/dom/tests/bug55294.phpt new file mode 100644 index 0000000000000..19534955029bc --- /dev/null +++ b/ext/dom/tests/bug55294.phpt @@ -0,0 +1,29 @@ +--TEST-- +Bug #55294 (DOMDocument::importNode shifts namespaces when "default" namespace exists) +--EXTENSIONS-- +dom +--FILE-- +loadXML(<< + + + + +EOXML +); + +$bDOM = new DOMDocument(); +$node = $bDOM->importNode($aDOM->getElementsByTagNameNS('http://example.com/A', 'B')->item(0), true); +$bDOM->appendChild($node); + +echo $bDOM->saveXML(), "\n"; + +?> +--EXPECT-- + + + + From 29a96e09b295f78d249423f9e0dd3c4becdd96fe Mon Sep 17 00:00:00 2001 From: nielsdos <7771979+nielsdos@users.noreply.github.com> Date: Wed, 14 Jun 2023 20:37:03 +0200 Subject: [PATCH 45/54] Fix GH-11451: Invalid associative array containing duplicate keys It used the "add_new" variant which assumes the key doesn't already exist. But in case of duplicate keys we have to take the last result. Closes GH-11453. --- NEWS | 4 ++++ ext/sqlite3/sqlite3.c | 4 +++- ext/sqlite3/tests/gh11451.phpt | 25 +++++++++++++++++++++++++ 3 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 ext/sqlite3/tests/gh11451.phpt diff --git a/NEWS b/NEWS index 19fd47a300069..3e0eee0d5e5c5 100644 --- a/NEWS +++ b/NEWS @@ -63,6 +63,10 @@ PHP NEWS . Fix access on NULL pointer in array_merge_recursive(). (ilutov) . Fix exception handling in array_multisort(). (ilutov) +- SQLite3: + . Fixed bug GH-11451 (Invalid associative array containing duplicate + keys). (nielsdos) + 08 Jun 2023, PHP 8.2.7 - Core: diff --git a/ext/sqlite3/sqlite3.c b/ext/sqlite3/sqlite3.c index d97f8b5fa53e3..2b9e18d12f1f0 100644 --- a/ext/sqlite3/sqlite3.c +++ b/ext/sqlite3/sqlite3.c @@ -1982,7 +1982,9 @@ PHP_METHOD(SQLite3Result, fetchArray) Z_ADDREF(data); } } - zend_symtable_add_new(Z_ARR_P(return_value), result_obj->column_names[i], &data); + /* Note: we can't use the "add_new" variant here instead of "update" because + * when the same column name is encountered, the last result should be taken. */ + zend_symtable_update(Z_ARR_P(return_value), result_obj->column_names[i], &data); } } break; diff --git a/ext/sqlite3/tests/gh11451.phpt b/ext/sqlite3/tests/gh11451.phpt new file mode 100644 index 0000000000000..e518c39b18b6d --- /dev/null +++ b/ext/sqlite3/tests/gh11451.phpt @@ -0,0 +1,25 @@ +--TEST-- +GH-11451 (Invalid associative array containing duplicate keys) +--EXTENSIONS-- +sqlite3 +--FILE-- +query('SELECT 1 AS key, 2 AS key') + ->fetchArray(SQLITE3_ASSOC)); + +var_dump((new SQLite3(':memory:')) + ->query('SELECT 0 AS dummy, 1 AS key, 2 AS key') + ->fetchArray(SQLITE3_ASSOC)); +?> +--EXPECT-- +array(1) { + ["key"]=> + int(2) +} +array(2) { + ["dummy"]=> + int(0) + ["key"]=> + int(2) +} From 1c85278fbe44c8196e9e98268aef2f303365c430 Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Fri, 16 Jun 2023 12:04:05 +0200 Subject: [PATCH 46/54] [skip ci] Don't suppress setup-slapd.sh output This step frequently fails in CI. We would like to now why to see if we can improve stability. --- .github/actions/setup-x64/action.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/actions/setup-x64/action.yml b/.github/actions/setup-x64/action.yml index bb014bfe9de11..6cec51d4c8079 100644 --- a/.github/actions/setup-x64/action.yml +++ b/.github/actions/setup-x64/action.yml @@ -17,7 +17,7 @@ runs: docker exec sql1 /opt/mssql-tools/bin/sqlcmd -S 127.0.0.1 -U SA -P "" -Q "create login pdo_test with password='password', check_policy=off; create user pdo_test for login pdo_test; grant alter, control to pdo_test;" sudo locale-gen de_DE - ./.github/scripts/setup-slapd.sh &>/dev/null + ./.github/scripts/setup-slapd.sh sudo cp ext/snmp/tests/snmpd.conf /etc/snmp sudo cp ext/snmp/tests/bigtest /etc/snmp From 7eb3e9cd173fbdd39eefa791aab610858e76399d Mon Sep 17 00:00:00 2001 From: nielsdos <7771979+nielsdos@users.noreply.github.com> Date: Sat, 17 Jun 2023 00:37:48 +0200 Subject: [PATCH 47/54] Fix GH-11404: DOMDocument::savexml and friends ommit xmlns="" declaration for null namespace, creating incorrect xml representation of the DOM The NULL namespace is only correct when there is no default namespace override. When there is, we need to manually set it to the empty string namespace. Closes GH-11428. --- NEWS | 2 + ext/dom/document.c | 4 + ext/dom/element.c | 4 + ext/dom/node.c | 16 ++-- ext/dom/php_dom.c | 38 +++++++++ ext/dom/tests/bug47530.phpt | 2 +- ext/dom/tests/gh11404.phpt | 160 ++++++++++++++++++++++++++++++++++++ 7 files changed, 216 insertions(+), 10 deletions(-) create mode 100644 ext/dom/tests/gh11404.phpt diff --git a/NEWS b/NEWS index b89fb82ac442c..f0cbc5a6c978b 100644 --- a/NEWS +++ b/NEWS @@ -38,6 +38,8 @@ PHP NEWS . Fix "invalid state error" with cloned namespace declarations. (nielsdos) . Fixed bug #55294 and #47530 and #47847 (various namespace reconciliation issues). (nielsdos) + . Fixed bug GH-11404 (DOMDocument::saveXML and friends omit xmlns="" + declaration for null namespace). (nielsdos) - Opcache: . Fix allocation loop in zend_shared_alloc_startup(). (nielsdos) diff --git a/ext/dom/document.c b/ext/dom/document.c index 93091df83a04f..199c39b5ff503 100644 --- a/ext/dom/document.c +++ b/ext/dom/document.c @@ -878,6 +878,10 @@ PHP_METHOD(DOMDocument, createElementNS) if (errorcode == 0) { if (xmlValidateName((xmlChar *) localname, 0) == 0) { + /* https://dom.spec.whatwg.org/#validate-and-extract: demands us to set an empty string uri to NULL */ + if (uri_len == 0) { + uri = NULL; + } nodep = xmlNewDocNode(docp, NULL, (xmlChar *) localname, (xmlChar *) value); if (nodep != NULL && uri != NULL) { nsptr = xmlSearchNsByHref(nodep->doc, nodep, (xmlChar *) uri); diff --git a/ext/dom/element.c b/ext/dom/element.c index 44c576a07363f..79786ed907808 100644 --- a/ext/dom/element.c +++ b/ext/dom/element.c @@ -56,6 +56,10 @@ PHP_METHOD(DOMElement, __construct) if (uri_len > 0) { errorcode = dom_check_qname(name, &localname, &prefix, uri_len, name_len); if (errorcode == 0) { + /* https://dom.spec.whatwg.org/#validate-and-extract: demands us to set an empty string uri to NULL */ + if (uri_len == 0) { + uri = NULL; + } nodep = xmlNewNode (NULL, (xmlChar *)localname); if (nodep != NULL && uri != NULL) { nsptr = dom_get_ns(nodep, uri, &errorcode, prefix); diff --git a/ext/dom/node.c b/ext/dom/node.c index bcf4ee487d38d..dc8c96b85ff7f 100644 --- a/ext/dom/node.c +++ b/ext/dom/node.c @@ -531,7 +531,6 @@ Since: DOM Level 2 int dom_node_namespace_uri_read(dom_object *obj, zval *retval) { xmlNode *nodep = dom_object_get_node(obj); - char *str = NULL; if (nodep == NULL) { php_dom_throw_error(INVALID_STATE_ERR, 1); @@ -543,20 +542,19 @@ int dom_node_namespace_uri_read(dom_object *obj, zval *retval) case XML_ATTRIBUTE_NODE: case XML_NAMESPACE_DECL: if (nodep->ns != NULL) { - str = (char *) nodep->ns->href; + char *str = (char *) nodep->ns->href; + /* https://dom.spec.whatwg.org/#concept-attribute: namespaceUri is "null or a non-empty string" */ + if (str != NULL && str[0] != '\0') { + ZVAL_STRING(retval, str); + return SUCCESS; + } } break; default: - str = NULL; break; } - if (str != NULL) { - ZVAL_STRING(retval, str); - } else { - ZVAL_NULL(retval); - } - + ZVAL_NULL(retval); return SUCCESS; } diff --git a/ext/dom/php_dom.c b/ext/dom/php_dom.c index 4a6ab2fee9e98..1cd035b62f51f 100644 --- a/ext/dom/php_dom.c +++ b/ext/dom/php_dom.c @@ -1478,6 +1478,22 @@ static void dom_libxml_reconcile_ensure_namespaces_are_declared(xmlNodePtr nodep xmlDOMWrapReconcileNamespaces(&dummy_ctxt, nodep, /* options */ 0); } +static bool dom_must_replace_namespace_by_empty_default(xmlDocPtr doc, xmlNodePtr nodep) +{ + xmlNsPtr default_ns = xmlSearchNs(doc, nodep->parent, NULL); + return default_ns != NULL && default_ns->href != NULL && default_ns->href[0] != '\0'; +} + +static void dom_replace_namespace_by_empty_default(xmlDocPtr doc, xmlNodePtr nodep) +{ + ZEND_ASSERT(nodep->ns == NULL); + /* The node uses the default empty namespace, but the current default namespace is non-empty. + * We can't unconditionally do this because otherwise libxml2 creates an xmlns="" declaration. + * Note: there's no point searching the oldNs list, because we haven't found it in the tree anyway. + * Ideally this would be pre-allocated but unfortunately libxml2 doesn't offer such a functionality. */ + xmlSetNs(nodep, xmlNewNs(nodep, (const xmlChar *) "", NULL)); +} + void dom_reconcile_ns(xmlDocPtr doc, xmlNodePtr nodep) /* {{{ */ { /* Although the node type will be checked by the libxml2 API, @@ -1485,6 +1501,10 @@ void dom_reconcile_ns(xmlDocPtr doc, xmlNodePtr nodep) /* {{{ */ if (nodep->type == XML_ELEMENT_NODE) { dom_reconcile_ns_internal(doc, nodep, nodep->parent); dom_libxml_reconcile_ensure_namespaces_are_declared(nodep); + /* Check nodep->ns first to avoid an expensive lookup. */ + if (nodep->ns == NULL && dom_must_replace_namespace_by_empty_default(doc, nodep)) { + dom_replace_namespace_by_empty_default(doc, nodep); + } } } /* }}} */ @@ -1508,12 +1528,30 @@ static void dom_reconcile_ns_list_internal(xmlDocPtr doc, xmlNodePtr nodep, xmlN void dom_reconcile_ns_list(xmlDocPtr doc, xmlNodePtr nodep, xmlNodePtr last) { + bool did_compute_must_replace_namespace_by_empty_default = false; + bool must_replace_namespace_by_empty_default = false; + dom_reconcile_ns_list_internal(doc, nodep, last, nodep->parent); + /* The loop is outside of the recursion in the above call because * dom_libxml_reconcile_ensure_namespaces_are_declared() performs its own recursion. */ while (true) { /* The internal libxml2 call will already check the node type, no need for us to do it here. */ dom_libxml_reconcile_ensure_namespaces_are_declared(nodep); + + /* We don't have to handle the children, because if their ns's are NULL they'll just take on the default + * which should've been reconciled before. */ + if (nodep->ns == NULL) { + /* This is an optimistic approach: we assume that most of the time we don't need the result of the computation. */ + if (!did_compute_must_replace_namespace_by_empty_default) { + did_compute_must_replace_namespace_by_empty_default = true; + must_replace_namespace_by_empty_default = dom_must_replace_namespace_by_empty_default(doc, nodep); + } + if (must_replace_namespace_by_empty_default) { + dom_replace_namespace_by_empty_default(doc, nodep); + } + } + if (nodep == last) { break; } diff --git a/ext/dom/tests/bug47530.phpt b/ext/dom/tests/bug47530.phpt index 0fb990e0e7bff..ab0b030c94be9 100644 --- a/ext/dom/tests/bug47530.phpt +++ b/ext/dom/tests/bug47530.phpt @@ -121,7 +121,7 @@ test_appendChild_with_shadowing(); -- Test document fragment without import -- - + string(7) "foo:bar" string(19) "https://php.net/bar" -- Test document import -- diff --git a/ext/dom/tests/gh11404.phpt b/ext/dom/tests/gh11404.phpt new file mode 100644 index 0000000000000..6c868de508c6e --- /dev/null +++ b/ext/dom/tests/gh11404.phpt @@ -0,0 +1,160 @@ +--TEST-- +GH-11404: DOMDocument::savexml and friends ommit xmlns="" declaration for null namespace, creating incorrect xml representation of the DOM +--EXTENSIONS-- +dom +--FILE-- +createElement('a'); + $nodeB = $dom->createElementNS(null, 'b'); + $nodeC = $dom->createElementNS('', 'c'); + $nodeD = $dom->createElement('d'); + $nodeD->setAttributeNS('some:ns', 'x:attrib', 'val'); + $nodeE = $dom->createElementNS('some:ns', 'e'); + // And these two respect the default ns. + $nodeE->setAttributeNS(null, 'attrib1', 'val'); + $nodeE->setAttributeNS('', 'attrib2', 'val'); + + $dom->documentElement->appendChild($nodeA); + $dom->documentElement->appendChild($nodeB); + $dom->documentElement->appendChild($nodeC); + $dom->documentElement->appendChild($nodeD); + $dom->documentElement->appendChild($nodeE); + + var_dump($nodeA->namespaceURI); + var_dump($nodeB->namespaceURI); + var_dump($nodeC->namespaceURI); + var_dump($nodeD->namespaceURI); + var_dump($nodeE->namespaceURI); + + echo $dom->saveXML(); + + // Create a subtree without using a fragment + $subtree = $dom->createElement('subtree'); + $subtree->appendChild($dom->createElementNS('some:ns', 'subtreechild1')); + $subtree->firstElementChild->appendChild($dom->createElement('subtreechild2')); + $dom->documentElement->appendChild($subtree); + + echo $dom->saveXML(); + + // Create a subtree with the use of a fragment + $subtree = $dom->createDocumentFragment(); + $subtree->appendChild($child3 = $dom->createElement('child3')); + $child3->appendChild($dom->createElement('child4')); + $subtree->appendChild($dom->createElement('child5')); + $dom->documentElement->appendChild($subtree); + + echo $dom->saveXML(); +} + +$dom1 = new DOMDocument; +$dom1->loadXML(''); +testAppendAndAttributes($dom1); + +echo "-- Test append and attributes: without default namespace variation --\n"; + +$dom1 = new DOMDocument; +$dom1->loadXML(''); +testAppendAndAttributes($dom1); + +echo "-- Test import --\n"; + +function testImport(?string $href, string $toBeImported) { + $dom1 = new DOMDocument; + $decl = $href === NULL ? '' : "xmlns=\"$href\""; + $dom1->loadXML(''); + + $dom2 = new DOMDocument; + $dom2->loadXML('' . $toBeImported); + + $dom1->documentElement->append( + $imported = $dom1->importNode($dom2->documentElement, true) + ); + + var_dump($imported->namespaceURI); + + echo $dom1->saveXML(); +} + +testImport(null, ''); +testImport('', ''); +testImport('some:ns', ''); +testImport('', '
'); +testImport('some:ns', '
'); + +echo "-- Namespace URI comparison --\n"; + +$dom1 = new DOMDocument; +$dom1->loadXML('
'); +var_dump($dom1->firstElementChild->namespaceURI); +var_dump($dom1->firstElementChild->firstElementChild->namespaceURI); + +$dom1 = new DOMDocument; +$dom1->appendChild($dom1->createElementNS('a:b', 'parent')); +$dom1->firstElementChild->appendChild($dom1->createElementNS('a:b', 'child1')); +$dom1->firstElementChild->appendChild($second = $dom1->createElement('child2')); +var_dump($dom1->firstElementChild->namespaceURI); +var_dump($dom1->firstElementChild->firstElementChild->namespaceURI); +var_dump($second->namespaceURI); +echo $dom1->saveXML(); + +$dom1 = new DOMDocument; +$dom1->loadXML(''); +var_dump($dom1->firstElementChild->namespaceURI); +$dom1->firstElementChild->appendChild($dom1->createElementNS('a:b', 'tag')); +var_dump($dom1->firstElementChild->firstElementChild->namespaceURI); +?> +--EXPECT-- +-- Test append and attributes: with default namespace variation -- +NULL +NULL +NULL +NULL +string(7) "some:ns" + + + + + + +-- Test append and attributes: without default namespace variation -- +NULL +NULL +NULL +NULL +string(7) "some:ns" + + + + + + +-- Test import -- +NULL + + +NULL + + +NULL + + +NULL + +
+string(7) "some:ns" + +
+-- Namespace URI comparison -- +string(3) "a:b" +string(3) "a:b" +string(3) "a:b" +string(3) "a:b" +NULL + + +string(3) "a:b" +string(3) "a:b" From f194cdf85231f4f861f6dff7ad8b784e78503f64 Mon Sep 17 00:00:00 2001 From: David CARLIER Date: Thu, 8 Jun 2023 22:52:49 +0100 Subject: [PATCH 48/54] ext/pgsql: fix PGtrace invalid free issue. disable trace when closing the connection, is a no op if there is no stream attached to it. Close GH-11403 --- NEWS | 3 +++ ext/pgsql/pgsql.c | 1 + ext/pgsql/tests/pg_trace.phpt | 20 ++++++++++++++++++++ 3 files changed, 24 insertions(+) create mode 100644 ext/pgsql/tests/pg_trace.phpt diff --git a/NEWS b/NEWS index f0cbc5a6c978b..16dbfb22c3b99 100644 --- a/NEWS +++ b/NEWS @@ -51,6 +51,9 @@ PHP NEWS . Fixed bug GH-9356 Incomplete validation of IPv6 Address fields in subjectAltNames (James Lucas, Jakub Zelenka). +- PGSQL: + . Fixed intermittent segfault with pg_trace. (David Carlier) + - Phar: . Fix cross-compilation check in phar generation for FreeBSD. (peter279k) diff --git a/ext/pgsql/pgsql.c b/ext/pgsql/pgsql.c index cbed6b7db18e2..66f09382fc32e 100644 --- a/ext/pgsql/pgsql.c +++ b/ext/pgsql/pgsql.c @@ -172,6 +172,7 @@ static void pgsql_link_free(pgsql_link_handle *link) PQclear(res); } if (!link->persistent) { + PQuntrace(link->conn); PQfinish(link->conn); } PGG(num_links)--; diff --git a/ext/pgsql/tests/pg_trace.phpt b/ext/pgsql/tests/pg_trace.phpt new file mode 100644 index 0000000000000..89b6027f854ea --- /dev/null +++ b/ext/pgsql/tests/pg_trace.phpt @@ -0,0 +1,20 @@ +--TEST-- +pg_trace +--EXTENSIONS-- +pgsql +--SKIPIF-- + +--FILE-- + +--EXPECTF-- +bool(true) From 9f7d88802e693c61e569988ef8a0b015d32c7668 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sat, 17 Jun 2023 20:52:52 +0200 Subject: [PATCH 49/54] Fix #80332: Completely broken array access functionality with DOMNamedNodeMap The problem is the usage of zval_get_long(). In particular, if the string is non-numeric the result of zval_get_long() will be 0 without giving an error or warning. This is misleading for users: users get the impression that they can use strings to access the map because it coincidentally works for the first item (which is at index 0). Of course, this fails with any other index which causes confusion and bugs. This patch adds proper support for using string offsets while accessing the map. It does so by detecting if it's a non-numeric string, and then using the getNamedItem() method instead of item(). I had to split up the array access implementation code for DOMNodeList and DOMNamedNodeMap first to be able to do this. Closes GH-11468. --- NEWS | 2 + ext/dom/namednodemap.c | 122 ++++++++++++++++++---------------- ext/dom/nodelist.c | 50 +++++++------- ext/dom/php_dom.c | 109 ++++++++++++++++++++++++------ ext/dom/php_dom.h | 9 +++ ext/dom/tests/bug67949.phpt | 2 +- ext/dom/tests/bug80332_1.phpt | 84 +++++++++++++++++++++++ ext/dom/tests/bug80332_2.phpt | 47 +++++++++++++ 8 files changed, 318 insertions(+), 107 deletions(-) create mode 100644 ext/dom/tests/bug80332_1.phpt create mode 100644 ext/dom/tests/bug80332_2.phpt diff --git a/NEWS b/NEWS index 16dbfb22c3b99..dd949084cce9c 100644 --- a/NEWS +++ b/NEWS @@ -40,6 +40,8 @@ PHP NEWS issues). (nielsdos) . Fixed bug GH-11404 (DOMDocument::saveXML and friends omit xmlns="" declaration for null namespace). (nielsdos) + . Fixed bug #80332 (Completely broken array access functionality with + DOMNamedNodeMap). (nielsdos) - Opcache: . Fix allocation loop in zend_shared_alloc_startup(). (nielsdos) diff --git a/ext/dom/namednodemap.c b/ext/dom/namednodemap.c index 99103ce30b7ad..31b915243c5f7 100644 --- a/ext/dom/namednodemap.c +++ b/ext/dom/namednodemap.c @@ -31,7 +31,7 @@ * Since: */ -static int get_namednodemap_length(dom_object *obj) +int php_dom_get_namednodemap_length(dom_object *obj) { dom_nnodemap_object *objmap = (dom_nnodemap_object *) obj->ptr; if (!objmap) { @@ -65,95 +65,74 @@ URL: http://www.w3.org/TR/2003/WD-DOM-Level-3-Core-20030226/DOM3-Core.html#core- */ int dom_namednodemap_length_read(dom_object *obj, zval *retval) { - ZVAL_LONG(retval, get_namednodemap_length(obj)); + ZVAL_LONG(retval, php_dom_get_namednodemap_length(obj)); return SUCCESS; } /* }}} */ -/* {{{ URL: http://www.w3.org/TR/2003/WD-DOM-Level-3-Core-20030226/DOM3-Core.html#core-ID-1074577549 -Since: -*/ -PHP_METHOD(DOMNamedNodeMap, getNamedItem) +xmlNodePtr php_dom_named_node_map_get_named_item(dom_nnodemap_object *objmap, const char *named, bool may_transform) { - zval *id; - int ret; - size_t namedlen=0; - dom_object *intern; xmlNodePtr itemnode = NULL; - char *named; - - dom_nnodemap_object *objmap; - xmlNodePtr nodep; - xmlNotation *notep = NULL; - - id = ZEND_THIS; - if (zend_parse_parameters(ZEND_NUM_ARGS(), "s", &named, &namedlen) == FAILURE) { - RETURN_THROWS(); - } - - intern = Z_DOMOBJ_P(id); - - objmap = (dom_nnodemap_object *)intern->ptr; - if (objmap != NULL) { if ((objmap->nodetype == XML_NOTATION_NODE) || objmap->nodetype == XML_ENTITY_NODE) { if (objmap->ht) { if (objmap->nodetype == XML_ENTITY_NODE) { - itemnode = (xmlNodePtr)xmlHashLookup(objmap->ht, (xmlChar *) named); + itemnode = (xmlNodePtr)xmlHashLookup(objmap->ht, (const xmlChar *) named); } else { - notep = (xmlNotation *)xmlHashLookup(objmap->ht, (xmlChar *) named); + xmlNotationPtr notep = xmlHashLookup(objmap->ht, (const xmlChar *) named); if (notep) { - itemnode = create_notation(notep->name, notep->PublicID, notep->SystemID); + if (may_transform) { + itemnode = create_notation(notep->name, notep->PublicID, notep->SystemID); + } else { + itemnode = (xmlNodePtr) notep; + } } } } } else { - nodep = dom_object_get_node(objmap->baseobj); + xmlNodePtr nodep = dom_object_get_node(objmap->baseobj); if (nodep) { - itemnode = (xmlNodePtr)xmlHasProp(nodep, (xmlChar *) named); + itemnode = (xmlNodePtr)xmlHasProp(nodep, (const xmlChar *) named); } } } + return itemnode; +} +void php_dom_named_node_map_get_named_item_into_zval(dom_nnodemap_object *objmap, const char *named, zval *return_value) +{ + int ret; + xmlNodePtr itemnode = php_dom_named_node_map_get_named_item(objmap, named, true); if (itemnode) { DOM_RET_OBJ(itemnode, &ret, objmap->baseobj); - return; } else { - RETVAL_NULL(); + RETURN_NULL(); } } -/* }}} end dom_namednodemap_get_named_item */ -/* {{{ URL: http://www.w3.org/TR/2003/WD-DOM-Level-3-Core-20030226/DOM3-Core.html#core-ID-349467F9 +/* {{{ URL: http://www.w3.org/TR/2003/WD-DOM-Level-3-Core-20030226/DOM3-Core.html#core-ID-1074577549 Since: */ -PHP_METHOD(DOMNamedNodeMap, item) +PHP_METHOD(DOMNamedNodeMap, getNamedItem) { - zval *id; - zend_long index; - int ret; - dom_object *intern; - xmlNodePtr itemnode = NULL; - - dom_nnodemap_object *objmap; - xmlNodePtr nodep, curnode; - int count; + size_t namedlen; + char *named; - id = ZEND_THIS; - if (zend_parse_parameters(ZEND_NUM_ARGS(), "l", &index) == FAILURE) { - RETURN_THROWS(); - } - if (index < 0 || ZEND_LONG_INT_OVFL(index)) { - zend_argument_value_error(1, "must be between 0 and %d", INT_MAX); + if (zend_parse_parameters(ZEND_NUM_ARGS(), "s", &named, &namedlen) == FAILURE) { RETURN_THROWS(); } - intern = Z_DOMOBJ_P(id); - - objmap = (dom_nnodemap_object *)intern->ptr; + zval *id = ZEND_THIS; + dom_nnodemap_object *objmap = Z_DOMOBJ_P(id)->ptr; + php_dom_named_node_map_get_named_item_into_zval(objmap, named, return_value); +} +/* }}} end dom_namednodemap_get_named_item */ +xmlNodePtr php_dom_named_node_map_get_item(dom_nnodemap_object *objmap, zend_long index) +{ + xmlNodePtr itemnode = NULL; if (objmap != NULL) { if ((objmap->nodetype == XML_NOTATION_NODE) || objmap->nodetype == XML_ENTITY_NODE) { @@ -165,10 +144,10 @@ PHP_METHOD(DOMNamedNodeMap, item) } } } else { - nodep = dom_object_get_node(objmap->baseobj); + xmlNodePtr nodep = dom_object_get_node(objmap->baseobj); if (nodep) { - curnode = (xmlNodePtr)nodep->properties; - count = 0; + xmlNodePtr curnode = (xmlNodePtr)nodep->properties; + zend_long count = 0; while (count < index && curnode != NULL) { count++; curnode = (xmlNodePtr)curnode->next; @@ -177,13 +156,38 @@ PHP_METHOD(DOMNamedNodeMap, item) } } } + return itemnode; +} +void php_dom_named_node_map_get_item_into_zval(dom_nnodemap_object *objmap, zend_long index, zval *return_value) +{ + int ret; + xmlNodePtr itemnode = php_dom_named_node_map_get_item(objmap, index); if (itemnode) { DOM_RET_OBJ(itemnode, &ret, objmap->baseobj); - return; + } else { + RETURN_NULL(); + } +} + +/* {{{ URL: http://www.w3.org/TR/2003/WD-DOM-Level-3-Core-20030226/DOM3-Core.html#core-ID-349467F9 +Since: +*/ +PHP_METHOD(DOMNamedNodeMap, item) +{ + zend_long index; + if (zend_parse_parameters(ZEND_NUM_ARGS(), "l", &index) == FAILURE) { + RETURN_THROWS(); + } + if (index < 0 || ZEND_LONG_INT_OVFL(index)) { + zend_argument_value_error(1, "must be between 0 and %d", INT_MAX); + RETURN_THROWS(); } - RETVAL_NULL(); + zval *id = ZEND_THIS; + dom_object *intern = Z_DOMOBJ_P(id); + dom_nnodemap_object *objmap = intern->ptr; + php_dom_named_node_map_get_item_into_zval(objmap, index, return_value); } /* }}} end dom_namednodemap_item */ @@ -254,7 +258,7 @@ PHP_METHOD(DOMNamedNodeMap, count) } intern = Z_DOMOBJ_P(id); - RETURN_LONG(get_namednodemap_length(intern)); + RETURN_LONG(php_dom_get_namednodemap_length(intern)); } /* }}} end dom_namednodemap_count */ diff --git a/ext/dom/nodelist.c b/ext/dom/nodelist.c index b03ebe1acd90a..20f6320a54607 100644 --- a/ext/dom/nodelist.c +++ b/ext/dom/nodelist.c @@ -31,7 +31,7 @@ * Since: */ -static int get_nodelist_length(dom_object *obj) +int php_dom_get_nodelist_length(dom_object *obj) { dom_nnodemap_object *objmap = (dom_nnodemap_object *) obj->ptr; if (!objmap) { @@ -82,7 +82,7 @@ URL: http://www.w3.org/TR/2003/WD-DOM-Level-3-Core-20030226/DOM3-Core.html#ID-20 */ int dom_nodelist_length_read(dom_object *obj, zval *retval) { - ZVAL_LONG(retval, get_nodelist_length(obj)); + ZVAL_LONG(retval, php_dom_get_nodelist_length(obj)); return SUCCESS; } @@ -99,36 +99,15 @@ PHP_METHOD(DOMNodeList, count) } intern = Z_DOMOBJ_P(id); - RETURN_LONG(get_nodelist_length(intern)); + RETURN_LONG(php_dom_get_nodelist_length(intern)); } /* }}} end dom_nodelist_count */ -/* }}} */ - -/* {{{ URL: http://www.w3.org/TR/2003/WD-DOM-Level-3-Core-20030226/DOM3-Core.html#ID-844377136 -Since: -*/ -PHP_METHOD(DOMNodeList, item) +void php_dom_nodelist_get_item_into_zval(dom_nnodemap_object *objmap, zend_long index, zval *return_value) { - zval *id; - zend_long index; int ret; - dom_object *intern; xmlNodePtr itemnode = NULL; - - dom_nnodemap_object *objmap; - xmlNodePtr nodep, curnode; - int count = 0; - - id = ZEND_THIS; - if (zend_parse_parameters(ZEND_NUM_ARGS(), "l", &index) == FAILURE) { - RETURN_THROWS(); - } - if (index >= 0) { - intern = Z_DOMOBJ_P(id); - - objmap = (dom_nnodemap_object *)intern->ptr; if (objmap != NULL) { if (objmap->ht) { if (objmap->nodetype == XML_ENTITY_NODE) { @@ -145,10 +124,11 @@ PHP_METHOD(DOMNodeList, item) return; } } else if (objmap->baseobj) { - nodep = dom_object_get_node(objmap->baseobj); + xmlNodePtr nodep = dom_object_get_node(objmap->baseobj); if (nodep) { + int count = 0; if (objmap->nodetype == XML_ATTRIBUTE_NODE || objmap->nodetype == XML_ELEMENT_NODE) { - curnode = nodep->children; + xmlNodePtr curnode = nodep->children; while (count < index && curnode != NULL) { count++; curnode = curnode->next; @@ -175,6 +155,22 @@ PHP_METHOD(DOMNodeList, item) RETVAL_NULL(); } + +/* {{{ URL: http://www.w3.org/TR/2003/WD-DOM-Level-3-Core-20030226/DOM3-Core.html#ID-844377136 +Since: +*/ +PHP_METHOD(DOMNodeList, item) +{ + zend_long index; + if (zend_parse_parameters(ZEND_NUM_ARGS(), "l", &index) == FAILURE) { + RETURN_THROWS(); + } + + zval *id = ZEND_THIS; + dom_object *intern = Z_DOMOBJ_P(id); + dom_nnodemap_object *objmap = intern->ptr; + php_dom_nodelist_get_item_into_zval(objmap, index, return_value); +} /* }}} end dom_nodelist_item */ ZEND_METHOD(DOMNodeList, getIterator) diff --git a/ext/dom/php_dom.c b/ext/dom/php_dom.c index 1cd035b62f51f..be0534eb644e4 100644 --- a/ext/dom/php_dom.c +++ b/ext/dom/php_dom.c @@ -61,6 +61,7 @@ PHP_DOM_EXPORT zend_class_entry *dom_namespace_node_class_entry; zend_object_handlers dom_object_handlers; zend_object_handlers dom_nnodemap_object_handlers; +zend_object_handlers dom_nodelist_object_handlers; zend_object_handlers dom_object_namespace_node_handlers; #ifdef LIBXML_XPATH_ENABLED zend_object_handlers dom_xpath_object_handlers; @@ -90,6 +91,7 @@ static HashTable dom_xpath_prop_handlers; static zend_object *dom_objects_namespace_node_new(zend_class_entry *class_type); static void dom_object_namespace_node_free_storage(zend_object *object); static xmlNodePtr php_dom_create_fake_namespace_decl_node_ptr(xmlNodePtr nodep, xmlNsPtr original); +static zend_object *dom_nodelist_objects_new(zend_class_entry *class_type); typedef int (*dom_read_t)(dom_object *obj, zval *retval); typedef int (*dom_write_t)(dom_object *obj, zval *newval); @@ -577,6 +579,8 @@ void dom_objects_free_storage(zend_object *object); void dom_nnodemap_objects_free_storage(zend_object *object); static zval *dom_nodelist_read_dimension(zend_object *object, zval *offset, int type, zval *rv); static int dom_nodelist_has_dimension(zend_object *object, zval *member, int check_empty); +static zval *dom_nodemap_read_dimension(zend_object *object, zval *offset, int type, zval *rv); +static int dom_nodemap_has_dimension(zend_object *object, zval *member, int check_empty); static zend_object *dom_objects_store_clone_obj(zend_object *zobject); #ifdef LIBXML_XPATH_ENABLED void dom_xpath_objects_free_storage(zend_object *object); @@ -597,8 +601,12 @@ PHP_MINIT_FUNCTION(dom) memcpy(&dom_nnodemap_object_handlers, &dom_object_handlers, sizeof(zend_object_handlers)); dom_nnodemap_object_handlers.free_obj = dom_nnodemap_objects_free_storage; - dom_nnodemap_object_handlers.read_dimension = dom_nodelist_read_dimension; - dom_nnodemap_object_handlers.has_dimension = dom_nodelist_has_dimension; + dom_nnodemap_object_handlers.read_dimension = dom_nodemap_read_dimension; + dom_nnodemap_object_handlers.has_dimension = dom_nodemap_has_dimension; + + memcpy(&dom_nodelist_object_handlers, &dom_nnodemap_object_handlers, sizeof(zend_object_handlers)); + dom_nodelist_object_handlers.read_dimension = dom_nodelist_read_dimension; + dom_nodelist_object_handlers.has_dimension = dom_nodelist_has_dimension; memcpy(&dom_object_namespace_node_handlers, &dom_object_handlers, sizeof(zend_object_handlers)); dom_object_namespace_node_handlers.offset = XtOffsetOf(dom_object_namespace_node, dom.std); @@ -694,7 +702,7 @@ PHP_MINIT_FUNCTION(dom) zend_hash_add_ptr(&classes, dom_document_class_entry->name, &dom_document_prop_handlers); dom_nodelist_class_entry = register_class_DOMNodeList(zend_ce_aggregate, zend_ce_countable); - dom_nodelist_class_entry->create_object = dom_nnodemap_objects_new; + dom_nodelist_class_entry->create_object = dom_nodelist_objects_new; dom_nodelist_class_entry->get_iterator = php_dom_get_iterator; zend_hash_init(&dom_nodelist_prop_handlers, 0, NULL, dom_dtor_prop_handler, 1); @@ -1129,7 +1137,7 @@ void dom_nnodemap_objects_free_storage(zend_object *object) /* {{{ */ } /* }}} */ -zend_object *dom_nnodemap_objects_new(zend_class_entry *class_type) /* {{{ */ +static zend_object *dom_nodemap_or_nodelist_objects_new(zend_class_entry *class_type, const zend_object_handlers *handlers) { dom_object *intern; dom_nnodemap_object *objmap; @@ -1144,12 +1152,22 @@ zend_object *dom_nnodemap_objects_new(zend_class_entry *class_type) /* {{{ */ objmap->local = NULL; objmap->ns = NULL; - intern->std.handlers = &dom_nnodemap_object_handlers; + intern->std.handlers = handlers; return &intern->std; } + +zend_object *dom_nnodemap_objects_new(zend_class_entry *class_type) /* {{{ */ +{ + return dom_nodemap_or_nodelist_objects_new(class_type, &dom_nnodemap_object_handlers); +} /* }}} */ +zend_object *dom_nodelist_objects_new(zend_class_entry *class_type) +{ + return dom_nodemap_or_nodelist_objects_new(class_type, &dom_nodelist_object_handlers); +} + void php_dom_create_iterator(zval *return_value, int ce_type) /* {{{ */ { zend_class_entry *ce; @@ -1683,34 +1701,85 @@ xmlNodePtr php_dom_create_fake_namespace_decl(xmlNodePtr nodep, xmlNsPtr origina return attrp; } -static zval *dom_nodelist_read_dimension(zend_object *object, zval *offset, int type, zval *rv) /* {{{ */ +static bool dom_nodemap_or_nodelist_process_offset_as_named(zval *offset, zend_long *lval) { - zval offset_copy; + if (Z_TYPE_P(offset) == IS_STRING) { + /* See zval_get_long_func() */ + double dval; + zend_uchar is_numeric_string_type; + if (0 == (is_numeric_string_type = is_numeric_string(Z_STRVAL_P(offset), Z_STRLEN_P(offset), lval, &dval, true))) { + return true; + } else if (is_numeric_string_type == IS_DOUBLE) { + *lval = zend_dval_to_lval_cap(dval); + } + } else { + *lval = zval_get_long(offset); + } + return false; +} - if (!offset) { - zend_throw_error(NULL, "Cannot access node list without offset"); +static zval *dom_nodelist_read_dimension(zend_object *object, zval *offset, int type, zval *rv) /* {{{ */ +{ + if (UNEXPECTED(!offset)) { + zend_throw_error(NULL, "Cannot access DOMNodeList without offset"); return NULL; } - ZVAL_LONG(&offset_copy, zval_get_long(offset)); - - zend_call_method_with_1_params(object, object->ce, NULL, "item", rv, &offset_copy); + zend_long lval; + if (dom_nodemap_or_nodelist_process_offset_as_named(offset, &lval)) { + /* does not support named lookup */ + ZVAL_NULL(rv); + return rv; + } + php_dom_nodelist_get_item_into_zval(php_dom_obj_from_obj(object)->ptr, lval, rv); return rv; } /* }}} end dom_nodelist_read_dimension */ static int dom_nodelist_has_dimension(zend_object *object, zval *member, int check_empty) { - zend_long offset = zval_get_long(member); - zval rv; - - if (offset < 0) { + zend_long offset; + if (dom_nodemap_or_nodelist_process_offset_as_named(member, &offset)) { + /* does not support named lookup */ return 0; - } else { - zval *length = zend_read_property( - object->ce, object, "length", sizeof("length") - 1, 0, &rv); - return length && offset < Z_LVAL_P(length); } + + return offset >= 0 && offset < php_dom_get_nodelist_length(php_dom_obj_from_obj(object)); } /* }}} end dom_nodelist_has_dimension */ +static zval *dom_nodemap_read_dimension(zend_object *object, zval *offset, int type, zval *rv) /* {{{ */ +{ + if (UNEXPECTED(!offset)) { + zend_throw_error(NULL, "Cannot access DOMNamedNodeMap without offset"); + return NULL; + } + + zend_long lval; + if (dom_nodemap_or_nodelist_process_offset_as_named(offset, &lval)) { + /* exceptional case, switch to named lookup */ + php_dom_named_node_map_get_named_item_into_zval(php_dom_obj_from_obj(object)->ptr, Z_STRVAL_P(offset), rv); + return rv; + } + + /* see PHP_METHOD(DOMNamedNodeMap, item) */ + if (UNEXPECTED(lval < 0 || ZEND_LONG_INT_OVFL(lval))) { + zend_value_error("must be between 0 and %d", INT_MAX); + return NULL; + } + + php_dom_named_node_map_get_item_into_zval(php_dom_obj_from_obj(object)->ptr, lval, rv); + return rv; +} /* }}} end dom_nodemap_read_dimension */ + +static int dom_nodemap_has_dimension(zend_object *object, zval *member, int check_empty) +{ + zend_long offset; + if (dom_nodemap_or_nodelist_process_offset_as_named(member, &offset)) { + /* exceptional case, switch to named lookup */ + return php_dom_named_node_map_get_named_item(php_dom_obj_from_obj(object)->ptr, Z_STRVAL_P(member), false) != NULL; + } + + return offset >= 0 && offset < php_dom_get_namednodemap_length(php_dom_obj_from_obj(object)); +} /* }}} end dom_nodemap_has_dimension */ + #endif /* HAVE_DOM */ diff --git a/ext/dom/php_dom.h b/ext/dom/php_dom.h index 6ed382b6f84af..9a57996729dbf 100644 --- a/ext/dom/php_dom.h +++ b/ext/dom/php_dom.h @@ -147,6 +147,15 @@ void dom_parent_node_before(dom_object *context, zval *nodes, int nodesc); void dom_child_node_remove(dom_object *context); void dom_child_replace_with(dom_object *context, zval *nodes, int nodesc); +/* nodemap and nodelist APIs */ +xmlNodePtr php_dom_named_node_map_get_named_item(dom_nnodemap_object *objmap, const char *named, bool may_transform); +void php_dom_named_node_map_get_named_item_into_zval(dom_nnodemap_object *objmap, const char *named, zval *return_value); +xmlNodePtr php_dom_named_node_map_get_item(dom_nnodemap_object *objmap, zend_long index); +void php_dom_named_node_map_get_item_into_zval(dom_nnodemap_object *objmap, zend_long index, zval *return_value); +void php_dom_nodelist_get_item_into_zval(dom_nnodemap_object *objmap, zend_long index, zval *return_value); +int php_dom_get_namednodemap_length(dom_object *obj); +int php_dom_get_nodelist_length(dom_object *obj); + #define DOM_GET_OBJ(__ptr, __id, __prtype, __intern) { \ __intern = Z_DOMOBJ_P(__id); \ if (__intern->ptr == NULL || !(__ptr = (__prtype)((php_libxml_node_ptr *)__intern->ptr)->node)) { \ diff --git a/ext/dom/tests/bug67949.phpt b/ext/dom/tests/bug67949.phpt index 394c9888bd352..270beb02b494c 100644 --- a/ext/dom/tests/bug67949.phpt +++ b/ext/dom/tests/bug67949.phpt @@ -86,7 +86,7 @@ bool(true) string(4) "data" string(4) "test" testing read_dimension with null offset -Cannot access node list without offset +Cannot access DOMNodeList without offset testing attribute access string(4) "href" ==DONE== diff --git a/ext/dom/tests/bug80332_1.phpt b/ext/dom/tests/bug80332_1.phpt new file mode 100644 index 0000000000000..f0484399781e2 --- /dev/null +++ b/ext/dom/tests/bug80332_1.phpt @@ -0,0 +1,84 @@ +--TEST-- +Bug #80332 (Completely broken array access functionality with DOMNamedNodeMap) - DOMNamedNodeMap variation +--EXTENSIONS-- +dom +--FILE-- +loadHTML(''); + +$x = new DOMXPath($doc); +$span = $x->query('//span')[0]; + +print "Node name: {$span->nodeName}\n"; + +function test($span, $key) { + $key_formatted = match ($key) { + false => 'false', + true => 'true', + null => 'null', + default => is_string($key) ? "'$key'" : $key, + }; + echo "Attribute [{$key_formatted}] name: ", $span->attributes[$key]->nodeName ?? '/', "\n"; + echo "Attribute [{$key_formatted}] value: ", $span->attributes[$key]->nodeValue ?? '/', "\n"; + echo "Attribute [{$key_formatted}] isset: ", isset($span->attributes[$key]) ? "yes" : "no", "\n"; + echo "\n"; +} + +test($span, 0); +test($span, false); +test($span, true); +test($span, null); +test($span, 'attr2'); +// This one should fail because there is no 'hi' attribute +test($span, 'hi'); +test($span, '0'); +test($span, '0.5'); +test($span, '1'); +// This one should fail because it's out of bounds +test($span, '2147483647'); + +?> +--EXPECT-- +Node name: span +Attribute [0] name: attr1 +Attribute [0] value: value1 +Attribute [0] isset: yes + +Attribute [false] name: attr1 +Attribute [false] value: value1 +Attribute [false] isset: yes + +Attribute [true] name: attr2 +Attribute [true] value: value2 +Attribute [true] isset: yes + +Attribute [null] name: attr1 +Attribute [null] value: value1 +Attribute [null] isset: yes + +Attribute ['attr2'] name: attr2 +Attribute ['attr2'] value: value2 +Attribute ['attr2'] isset: yes + +Attribute ['hi'] name: / +Attribute ['hi'] value: / +Attribute ['hi'] isset: no + +Attribute ['0'] name: attr1 +Attribute ['0'] value: value1 +Attribute ['0'] isset: yes + +Attribute ['0.5'] name: attr1 +Attribute ['0.5'] value: value1 +Attribute ['0.5'] isset: yes + +Attribute ['1'] name: attr2 +Attribute ['1'] value: value2 +Attribute ['1'] isset: yes + +Attribute ['2147483647'] name: / +Attribute ['2147483647'] value: / +Attribute ['2147483647'] isset: no diff --git a/ext/dom/tests/bug80332_2.phpt b/ext/dom/tests/bug80332_2.phpt new file mode 100644 index 0000000000000..f1793e43b5931 --- /dev/null +++ b/ext/dom/tests/bug80332_2.phpt @@ -0,0 +1,47 @@ +--TEST-- +Bug #80332 (Completely broken array access functionality with DOMNamedNodeMap) - DOMNodeList variation +--EXTENSIONS-- +dom +--FILE-- +loadXML(''); + +$list = $doc->getElementsByTagName('strong'); + +function test($list, $key) { + $key_formatted = match ($key) { + false => 'false', + true => 'true', + null => 'null', + default => is_string($key) ? "'$key'" : $key, + }; + echo "list[$key_formatted] id attribute: ", $list[$key] ? $list[$key]->getAttribute('id') : '/', "\n"; +} + +test($list, 0); +test($list, false); +test($list, true); +test($list, null); +test($list, '0'); +test($list, '0.5'); +test($list, '1'); +// These two should fail because there's no named lookup possible here +test($list, 'attr2'); +test($list, 'hi'); +// This one should fail because it's out of bounds +test($list, '2147483647'); + +?> +--EXPECT-- +list[0] id attribute: 1 +list[false] id attribute: 1 +list[true] id attribute: 2 +list[null] id attribute: 1 +list['0'] id attribute: 1 +list['0.5'] id attribute: 1 +list['1'] id attribute: 2 +list['attr2'] id attribute: / +list['hi'] id attribute: / +list['2147483647'] id attribute: / From c174ebfce031a4f5439cd232f532365328052b5e Mon Sep 17 00:00:00 2001 From: nielsdos <7771979+nielsdos@users.noreply.github.com> Date: Mon, 19 Jun 2023 19:33:44 +0200 Subject: [PATCH 50/54] Revert "Fix GH-11404: DOMDocument::savexml and friends ommit xmlns="" declaration for null namespace, creating incorrect xml representation of the DOM" This reverts commit 7eb3e9cd173fbdd39eefa791aab610858e76399d. Although the fix follows the spec, it causes issues because a lot of old code assumes the incorrect behaviour PHP had since a long time. We cannot do this yet, especially not in a stable release. We revert this for the time being. See GH-11428. --- NEWS | 2 - ext/dom/document.c | 4 - ext/dom/element.c | 4 - ext/dom/node.c | 16 ++-- ext/dom/php_dom.c | 38 --------- ext/dom/tests/bug47530.phpt | 2 +- ext/dom/tests/gh11404.phpt | 160 ------------------------------------ 7 files changed, 10 insertions(+), 216 deletions(-) delete mode 100644 ext/dom/tests/gh11404.phpt diff --git a/NEWS b/NEWS index dd949084cce9c..06ee2b27e6f04 100644 --- a/NEWS +++ b/NEWS @@ -38,8 +38,6 @@ PHP NEWS . Fix "invalid state error" with cloned namespace declarations. (nielsdos) . Fixed bug #55294 and #47530 and #47847 (various namespace reconciliation issues). (nielsdos) - . Fixed bug GH-11404 (DOMDocument::saveXML and friends omit xmlns="" - declaration for null namespace). (nielsdos) . Fixed bug #80332 (Completely broken array access functionality with DOMNamedNodeMap). (nielsdos) diff --git a/ext/dom/document.c b/ext/dom/document.c index 199c39b5ff503..93091df83a04f 100644 --- a/ext/dom/document.c +++ b/ext/dom/document.c @@ -878,10 +878,6 @@ PHP_METHOD(DOMDocument, createElementNS) if (errorcode == 0) { if (xmlValidateName((xmlChar *) localname, 0) == 0) { - /* https://dom.spec.whatwg.org/#validate-and-extract: demands us to set an empty string uri to NULL */ - if (uri_len == 0) { - uri = NULL; - } nodep = xmlNewDocNode(docp, NULL, (xmlChar *) localname, (xmlChar *) value); if (nodep != NULL && uri != NULL) { nsptr = xmlSearchNsByHref(nodep->doc, nodep, (xmlChar *) uri); diff --git a/ext/dom/element.c b/ext/dom/element.c index 79786ed907808..44c576a07363f 100644 --- a/ext/dom/element.c +++ b/ext/dom/element.c @@ -56,10 +56,6 @@ PHP_METHOD(DOMElement, __construct) if (uri_len > 0) { errorcode = dom_check_qname(name, &localname, &prefix, uri_len, name_len); if (errorcode == 0) { - /* https://dom.spec.whatwg.org/#validate-and-extract: demands us to set an empty string uri to NULL */ - if (uri_len == 0) { - uri = NULL; - } nodep = xmlNewNode (NULL, (xmlChar *)localname); if (nodep != NULL && uri != NULL) { nsptr = dom_get_ns(nodep, uri, &errorcode, prefix); diff --git a/ext/dom/node.c b/ext/dom/node.c index dc8c96b85ff7f..bcf4ee487d38d 100644 --- a/ext/dom/node.c +++ b/ext/dom/node.c @@ -531,6 +531,7 @@ Since: DOM Level 2 int dom_node_namespace_uri_read(dom_object *obj, zval *retval) { xmlNode *nodep = dom_object_get_node(obj); + char *str = NULL; if (nodep == NULL) { php_dom_throw_error(INVALID_STATE_ERR, 1); @@ -542,19 +543,20 @@ int dom_node_namespace_uri_read(dom_object *obj, zval *retval) case XML_ATTRIBUTE_NODE: case XML_NAMESPACE_DECL: if (nodep->ns != NULL) { - char *str = (char *) nodep->ns->href; - /* https://dom.spec.whatwg.org/#concept-attribute: namespaceUri is "null or a non-empty string" */ - if (str != NULL && str[0] != '\0') { - ZVAL_STRING(retval, str); - return SUCCESS; - } + str = (char *) nodep->ns->href; } break; default: + str = NULL; break; } - ZVAL_NULL(retval); + if (str != NULL) { + ZVAL_STRING(retval, str); + } else { + ZVAL_NULL(retval); + } + return SUCCESS; } diff --git a/ext/dom/php_dom.c b/ext/dom/php_dom.c index be0534eb644e4..ba532045186e5 100644 --- a/ext/dom/php_dom.c +++ b/ext/dom/php_dom.c @@ -1496,22 +1496,6 @@ static void dom_libxml_reconcile_ensure_namespaces_are_declared(xmlNodePtr nodep xmlDOMWrapReconcileNamespaces(&dummy_ctxt, nodep, /* options */ 0); } -static bool dom_must_replace_namespace_by_empty_default(xmlDocPtr doc, xmlNodePtr nodep) -{ - xmlNsPtr default_ns = xmlSearchNs(doc, nodep->parent, NULL); - return default_ns != NULL && default_ns->href != NULL && default_ns->href[0] != '\0'; -} - -static void dom_replace_namespace_by_empty_default(xmlDocPtr doc, xmlNodePtr nodep) -{ - ZEND_ASSERT(nodep->ns == NULL); - /* The node uses the default empty namespace, but the current default namespace is non-empty. - * We can't unconditionally do this because otherwise libxml2 creates an xmlns="" declaration. - * Note: there's no point searching the oldNs list, because we haven't found it in the tree anyway. - * Ideally this would be pre-allocated but unfortunately libxml2 doesn't offer such a functionality. */ - xmlSetNs(nodep, xmlNewNs(nodep, (const xmlChar *) "", NULL)); -} - void dom_reconcile_ns(xmlDocPtr doc, xmlNodePtr nodep) /* {{{ */ { /* Although the node type will be checked by the libxml2 API, @@ -1519,10 +1503,6 @@ void dom_reconcile_ns(xmlDocPtr doc, xmlNodePtr nodep) /* {{{ */ if (nodep->type == XML_ELEMENT_NODE) { dom_reconcile_ns_internal(doc, nodep, nodep->parent); dom_libxml_reconcile_ensure_namespaces_are_declared(nodep); - /* Check nodep->ns first to avoid an expensive lookup. */ - if (nodep->ns == NULL && dom_must_replace_namespace_by_empty_default(doc, nodep)) { - dom_replace_namespace_by_empty_default(doc, nodep); - } } } /* }}} */ @@ -1546,30 +1526,12 @@ static void dom_reconcile_ns_list_internal(xmlDocPtr doc, xmlNodePtr nodep, xmlN void dom_reconcile_ns_list(xmlDocPtr doc, xmlNodePtr nodep, xmlNodePtr last) { - bool did_compute_must_replace_namespace_by_empty_default = false; - bool must_replace_namespace_by_empty_default = false; - dom_reconcile_ns_list_internal(doc, nodep, last, nodep->parent); - /* The loop is outside of the recursion in the above call because * dom_libxml_reconcile_ensure_namespaces_are_declared() performs its own recursion. */ while (true) { /* The internal libxml2 call will already check the node type, no need for us to do it here. */ dom_libxml_reconcile_ensure_namespaces_are_declared(nodep); - - /* We don't have to handle the children, because if their ns's are NULL they'll just take on the default - * which should've been reconciled before. */ - if (nodep->ns == NULL) { - /* This is an optimistic approach: we assume that most of the time we don't need the result of the computation. */ - if (!did_compute_must_replace_namespace_by_empty_default) { - did_compute_must_replace_namespace_by_empty_default = true; - must_replace_namespace_by_empty_default = dom_must_replace_namespace_by_empty_default(doc, nodep); - } - if (must_replace_namespace_by_empty_default) { - dom_replace_namespace_by_empty_default(doc, nodep); - } - } - if (nodep == last) { break; } diff --git a/ext/dom/tests/bug47530.phpt b/ext/dom/tests/bug47530.phpt index ab0b030c94be9..0fb990e0e7bff 100644 --- a/ext/dom/tests/bug47530.phpt +++ b/ext/dom/tests/bug47530.phpt @@ -121,7 +121,7 @@ test_appendChild_with_shadowing(); -- Test document fragment without import -- - + string(7) "foo:bar" string(19) "https://php.net/bar" -- Test document import -- diff --git a/ext/dom/tests/gh11404.phpt b/ext/dom/tests/gh11404.phpt deleted file mode 100644 index 6c868de508c6e..0000000000000 --- a/ext/dom/tests/gh11404.phpt +++ /dev/null @@ -1,160 +0,0 @@ ---TEST-- -GH-11404: DOMDocument::savexml and friends ommit xmlns="" declaration for null namespace, creating incorrect xml representation of the DOM ---EXTENSIONS-- -dom ---FILE-- -createElement('a'); - $nodeB = $dom->createElementNS(null, 'b'); - $nodeC = $dom->createElementNS('', 'c'); - $nodeD = $dom->createElement('d'); - $nodeD->setAttributeNS('some:ns', 'x:attrib', 'val'); - $nodeE = $dom->createElementNS('some:ns', 'e'); - // And these two respect the default ns. - $nodeE->setAttributeNS(null, 'attrib1', 'val'); - $nodeE->setAttributeNS('', 'attrib2', 'val'); - - $dom->documentElement->appendChild($nodeA); - $dom->documentElement->appendChild($nodeB); - $dom->documentElement->appendChild($nodeC); - $dom->documentElement->appendChild($nodeD); - $dom->documentElement->appendChild($nodeE); - - var_dump($nodeA->namespaceURI); - var_dump($nodeB->namespaceURI); - var_dump($nodeC->namespaceURI); - var_dump($nodeD->namespaceURI); - var_dump($nodeE->namespaceURI); - - echo $dom->saveXML(); - - // Create a subtree without using a fragment - $subtree = $dom->createElement('subtree'); - $subtree->appendChild($dom->createElementNS('some:ns', 'subtreechild1')); - $subtree->firstElementChild->appendChild($dom->createElement('subtreechild2')); - $dom->documentElement->appendChild($subtree); - - echo $dom->saveXML(); - - // Create a subtree with the use of a fragment - $subtree = $dom->createDocumentFragment(); - $subtree->appendChild($child3 = $dom->createElement('child3')); - $child3->appendChild($dom->createElement('child4')); - $subtree->appendChild($dom->createElement('child5')); - $dom->documentElement->appendChild($subtree); - - echo $dom->saveXML(); -} - -$dom1 = new DOMDocument; -$dom1->loadXML(''); -testAppendAndAttributes($dom1); - -echo "-- Test append and attributes: without default namespace variation --\n"; - -$dom1 = new DOMDocument; -$dom1->loadXML(''); -testAppendAndAttributes($dom1); - -echo "-- Test import --\n"; - -function testImport(?string $href, string $toBeImported) { - $dom1 = new DOMDocument; - $decl = $href === NULL ? '' : "xmlns=\"$href\""; - $dom1->loadXML(''); - - $dom2 = new DOMDocument; - $dom2->loadXML('' . $toBeImported); - - $dom1->documentElement->append( - $imported = $dom1->importNode($dom2->documentElement, true) - ); - - var_dump($imported->namespaceURI); - - echo $dom1->saveXML(); -} - -testImport(null, ''); -testImport('', ''); -testImport('some:ns', ''); -testImport('', '
'); -testImport('some:ns', '
'); - -echo "-- Namespace URI comparison --\n"; - -$dom1 = new DOMDocument; -$dom1->loadXML('
'); -var_dump($dom1->firstElementChild->namespaceURI); -var_dump($dom1->firstElementChild->firstElementChild->namespaceURI); - -$dom1 = new DOMDocument; -$dom1->appendChild($dom1->createElementNS('a:b', 'parent')); -$dom1->firstElementChild->appendChild($dom1->createElementNS('a:b', 'child1')); -$dom1->firstElementChild->appendChild($second = $dom1->createElement('child2')); -var_dump($dom1->firstElementChild->namespaceURI); -var_dump($dom1->firstElementChild->firstElementChild->namespaceURI); -var_dump($second->namespaceURI); -echo $dom1->saveXML(); - -$dom1 = new DOMDocument; -$dom1->loadXML(''); -var_dump($dom1->firstElementChild->namespaceURI); -$dom1->firstElementChild->appendChild($dom1->createElementNS('a:b', 'tag')); -var_dump($dom1->firstElementChild->firstElementChild->namespaceURI); -?> ---EXPECT-- --- Test append and attributes: with default namespace variation -- -NULL -NULL -NULL -NULL -string(7) "some:ns" - - - - - - --- Test append and attributes: without default namespace variation -- -NULL -NULL -NULL -NULL -string(7) "some:ns" - - - - - - --- Test import -- -NULL - - -NULL - - -NULL - - -NULL - -
-string(7) "some:ns" - -
--- Namespace URI comparison -- -string(3) "a:b" -string(3) "a:b" -string(3) "a:b" -string(3) "a:b" -NULL - - -string(3) "a:b" -string(3) "a:b" From 93becab506ac05a7bddce1ceaacb53d6657180ce Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 18 Jun 2023 13:33:15 +0200 Subject: [PATCH 51/54] Fix GH-11455: Segmentation fault with custom object date properties Closes GH-11473. --- NEWS | 4 ++++ ext/date/php_date.c | 4 +++- ext/date/tests/gh11455.phpt | 39 +++++++++++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 ext/date/tests/gh11455.phpt diff --git a/NEWS b/NEWS index e90584208939d..86b978760be07 100644 --- a/NEWS +++ b/NEWS @@ -13,6 +13,10 @@ PHP NEWS . Fixed bug GH-11433 (Unable to set CURLOPT_ACCEPT_ENCODING to NULL). (nielsdos) +- Date: + . Fixed bug GH-11455 (Segmentation fault with custom object date properties). + (nielsdos) + - DOM: . Fixed bugs GH-11288 and GH-11289 and GH-11290 and GH-9142 (DOMExceptions and segfaults with replaceWith). (nielsdos) diff --git a/ext/date/php_date.c b/ext/date/php_date.c index 2c622f66904a0..91420a94bd8ab 100644 --- a/ext/date/php_date.c +++ b/ext/date/php_date.c @@ -2305,7 +2305,9 @@ static void add_common_properties(HashTable *myht, zend_object *zobj) common = zend_std_get_properties(zobj); ZEND_HASH_MAP_FOREACH_STR_KEY_VAL_IND(common, name, prop) { - zend_hash_add(myht, name, prop); + if (zend_hash_add(myht, name, prop) != NULL) { + Z_TRY_ADDREF_P(prop); + } } ZEND_HASH_FOREACH_END(); } diff --git a/ext/date/tests/gh11455.phpt b/ext/date/tests/gh11455.phpt new file mode 100644 index 0000000000000..cb6784f0b5a71 --- /dev/null +++ b/ext/date/tests/gh11455.phpt @@ -0,0 +1,39 @@ +--TEST-- +Bug GH-11455 (PHP 8.2 Segmentation fault on nesbot/carbon) +--FILE-- +myProperty->field = str_repeat("hello", 3); +$serialized = serialize($datetime); +var_dump($datetime->myProperty); +$unserialized = unserialize($serialized); +var_dump($unserialized); +?> +--EXPECT-- +object(stdClass)#2 (1) { + ["field"]=> + string(15) "hellohellohello" +} +object(MyDateTimeImmutable)#3 (4) { + ["myProperty"]=> + object(stdClass)#4 (1) { + ["field"]=> + string(15) "hellohellohello" + } + ["date"]=> + string(26) "2022-12-22 11:26:00.000000" + ["timezone_type"]=> + int(2) + ["timezone"]=> + string(1) "Z" +} From 1a96d64828f7574d02cff7ffbec4a2746a521df8 Mon Sep 17 00:00:00 2001 From: Dmitry Stogov Date: Tue, 20 Jun 2023 11:59:36 +0300 Subject: [PATCH 52/54] Fixed incorrect VM stack overflow checks elimination --- ext/opcache/jit/zend_jit_internal.h | 2 ++ ext/opcache/jit/zend_jit_trace.c | 29 +++++++++++++++++++++++------ 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/ext/opcache/jit/zend_jit_internal.h b/ext/opcache/jit/zend_jit_internal.h index a1b8b98baa7c7..9de9de384e1ba 100644 --- a/ext/opcache/jit/zend_jit_internal.h +++ b/ext/opcache/jit/zend_jit_internal.h @@ -602,6 +602,8 @@ struct _zend_jit_trace_stack_frame { uint32_t call_level; uint32_t _info; int used_stack; + int old_checked_stack; + int old_peek_checked_stack; zend_jit_trace_stack stack[1]; }; diff --git a/ext/opcache/jit/zend_jit_trace.c b/ext/opcache/jit/zend_jit_trace.c index cae233c684eb3..88ce671b86188 100644 --- a/ext/opcache/jit/zend_jit_trace.c +++ b/ext/opcache/jit/zend_jit_trace.c @@ -6603,7 +6603,8 @@ static const void *zend_jit_trace(zend_jit_trace_rec *trace_buffer, uint32_t par op_array_ssa = &jit_extension->func_info.ssa; top = frame; if (frame->prev) { - checked_stack -= frame->used_stack; + checked_stack = frame->old_checked_stack; + peek_checked_stack = frame->old_peek_checked_stack; frame = frame->prev; stack = frame->stack; ZEND_ASSERT(&frame->func->op_array == op_array); @@ -6762,24 +6763,40 @@ static const void *zend_jit_trace(zend_jit_trace_rec *trace_buffer, uint32_t par } } } + call->old_checked_stack = checked_stack; + call->old_peek_checked_stack = peek_checked_stack; if (p->info & ZEND_JIT_TRACE_FAKE_INIT_CALL) { frame->call_level++; - call->used_stack = 0; + call->used_stack = checked_stack = peek_checked_stack = 0; } else { if (p->func) { call->used_stack = zend_vm_calc_used_stack(init_opline->extended_value, (zend_function*)p->func); } else { call->used_stack = (ZEND_CALL_FRAME_SLOT + init_opline->extended_value) * sizeof(zval); } - checked_stack += call->used_stack; - if (checked_stack > peek_checked_stack) { - peek_checked_stack = checked_stack; + switch (init_opline->opcode) { + case ZEND_INIT_FCALL: + case ZEND_INIT_FCALL_BY_NAME: + case ZEND_INIT_NS_FCALL_BY_NAME: + case ZEND_INIT_METHOD_CALL: + case ZEND_INIT_DYNAMIC_CALL: + //case ZEND_INIT_STATIC_METHOD_CALL: + //case ZEND_INIT_USER_CALL: + //case ZEND_NEW: + checked_stack += call->used_stack; + if (checked_stack > peek_checked_stack) { + peek_checked_stack = checked_stack; + } + break; + default: + checked_stack = peek_checked_stack = 0; } } } else if (p->op == ZEND_JIT_TRACE_DO_ICALL) { call = frame->call; if (call) { - checked_stack -= call->used_stack; + checked_stack = call->old_checked_stack; + peek_checked_stack = call->old_peek_checked_stack; top = call; frame->call = call->prev; } From 6c4b1e0417ffb987d8f6b1a07c6348d73fafba31 Mon Sep 17 00:00:00 2001 From: Patrick Allaert Date: Tue, 20 Jun 2023 16:07:05 +0200 Subject: [PATCH 53/54] PHP-8.1 is now for PHP 8.1.22-dev --- NEWS | 5 ++++- Zend/zend.h | 2 +- configure.ac | 2 +- main/php_version.h | 4 ++-- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/NEWS b/NEWS index 06ee2b27e6f04..d250187607204 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,9 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| -?? ??? ????, PHP 8.1.21 +?? ??? ????, PHP 8.1.22 + + +06 Jul 2023, PHP 8.1.21 - CLI: . Fixed bug GH-11246 (cli/get_set_process_title fails on MacOS). diff --git a/Zend/zend.h b/Zend/zend.h index e3b67150b7817..6ab71eedec906 100644 --- a/Zend/zend.h +++ b/Zend/zend.h @@ -20,7 +20,7 @@ #ifndef ZEND_H #define ZEND_H -#define ZEND_VERSION "4.1.21-dev" +#define ZEND_VERSION "4.1.21" #define ZEND_ENGINE_3 diff --git a/configure.ac b/configure.ac index baf8651f044ad..568c7222117a9 100644 --- a/configure.ac +++ b/configure.ac @@ -17,7 +17,7 @@ dnl Basic autoconf initialization, generation of config.nice. dnl ---------------------------------------------------------------------------- AC_PREREQ([2.68]) -AC_INIT([PHP],[8.1.21-dev],[https://github.com/php/php-src/issues],[php],[https://www.php.net]) +AC_INIT([PHP],[8.1.21],[https://github.com/php/php-src/issues],[php],[https://www.php.net]) AC_CONFIG_SRCDIR([main/php_version.h]) AC_CONFIG_AUX_DIR([build]) AC_PRESERVE_HELP_ORDER diff --git a/main/php_version.h b/main/php_version.h index 0cf267f2e80b5..97432f518c4da 100644 --- a/main/php_version.h +++ b/main/php_version.h @@ -3,6 +3,6 @@ #define PHP_MAJOR_VERSION 8 #define PHP_MINOR_VERSION 1 #define PHP_RELEASE_VERSION 21 -#define PHP_EXTRA_VERSION "-dev" -#define PHP_VERSION "8.1.21-dev" +#define PHP_EXTRA_VERSION "" +#define PHP_VERSION "8.1.21" #define PHP_VERSION_ID 80121 From 2e2416825d2270d256f7597904e14e386cc0d854 Mon Sep 17 00:00:00 2001 From: Sergey Panteleev Date: Tue, 4 Jul 2023 17:46:28 +0300 Subject: [PATCH 54/54] Update versions for PHP 8.2.8 --- NEWS | 2 +- Zend/zend.h | 2 +- configure.ac | 2 +- main/php_version.h | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/NEWS b/NEWS index 86b978760be07..d6dca61c18b0b 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,6 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| -?? ??? ????, PHP 8.2.8 +06 Jul 2023, PHP 8.2.8 - CLI: . Fixed bug GH-11246 (cli/get_set_process_title fails on MacOS). diff --git a/Zend/zend.h b/Zend/zend.h index a0f87c6c9dba8..5c6d42b42eb85 100644 --- a/Zend/zend.h +++ b/Zend/zend.h @@ -20,7 +20,7 @@ #ifndef ZEND_H #define ZEND_H -#define ZEND_VERSION "4.2.8-dev" +#define ZEND_VERSION "4.2.8" #define ZEND_ENGINE_3 diff --git a/configure.ac b/configure.ac index fdc3c64110dcc..b4bb044fae741 100644 --- a/configure.ac +++ b/configure.ac @@ -17,7 +17,7 @@ dnl Basic autoconf initialization, generation of config.nice. dnl ---------------------------------------------------------------------------- AC_PREREQ([2.68]) -AC_INIT([PHP],[8.2.8-dev],[https://github.com/php/php-src/issues],[php],[https://www.php.net]) +AC_INIT([PHP],[8.2.8],[https://github.com/php/php-src/issues],[php],[https://www.php.net]) AC_CONFIG_SRCDIR([main/php_version.h]) AC_CONFIG_AUX_DIR([build]) AC_PRESERVE_HELP_ORDER diff --git a/main/php_version.h b/main/php_version.h index 80517df950fb5..d79d13b232257 100644 --- a/main/php_version.h +++ b/main/php_version.h @@ -3,6 +3,6 @@ #define PHP_MAJOR_VERSION 8 #define PHP_MINOR_VERSION 2 #define PHP_RELEASE_VERSION 8 -#define PHP_EXTRA_VERSION "-dev" -#define PHP_VERSION "8.2.8-dev" +#define PHP_EXTRA_VERSION "" +#define PHP_VERSION "8.2.8" #define PHP_VERSION_ID 80208