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 1/4] 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 143418e2a30..e587b833339 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 aefa2aa933e..b00a1e66568 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 00000000000..0a59cea9e74
--- /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 2/4] 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 e587b833339..a0f4c4b10fe 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 97b82378780..ed0602394ce 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 afe539bf987..37f6fea9199 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 3/4] 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 a0f4c4b10fe..1b40b1bceb4 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 01a206c0985..1883767d2e4 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 00000000000..888d1ef9b80
--- /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 4/4] 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 1b40b1bceb4..42c54e587de 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 46c90a13e31..c99a2a5a662 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 00000000000..2d70b10fe9f
--- /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 00000000000..4d9cf24a618
--- /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
+"