From 71f2e71b51c492a57ae4f39fdd419803006d6dd6 Mon Sep 17 00:00:00 2001
From: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Sat, 15 Mar 2025 01:38:27 +0100
Subject: [PATCH] Fix GH-12231: SimpleXML xpath should warn when returning
 other return types than node lists

---
 UPGRADING                        |  5 +++++
 ext/simplexml/simplexml.c        | 22 ++++++++++++++++++++++
 ext/simplexml/tests/008.phpt     |  7 ++++---
 ext/simplexml/tests/gh12231.phpt | 26 ++++++++++++++++++++++++++
 4 files changed, 57 insertions(+), 3 deletions(-)
 create mode 100644 ext/simplexml/tests/gh12231.phpt

diff --git a/UPGRADING b/UPGRADING
index 11cca55bd173..274f45d8f401 100644
--- a/UPGRADING
+++ b/UPGRADING
@@ -82,6 +82,11 @@ PHP 8.5 UPGRADE NOTES
   . A ValueError is now thrown when trying to set a cursor name that is too
     long on a PDOStatement resulting from the Firebird driver.
 
+- SimpleXML:
+  - Passing an XPath expression that returns something other than a node set
+    to SimpleXMLElement::xpath() will now emit a warning and return false,
+    instead of silently failing and returning an empty array.
+
 - SPL:
   . ArrayObject no longer accepts enums, as modifying the $name or $value
     properties can break engine assumptions.
diff --git a/ext/simplexml/simplexml.c b/ext/simplexml/simplexml.c
index 1de7ccc6e74e..877280fb378d 100644
--- a/ext/simplexml/simplexml.c
+++ b/ext/simplexml/simplexml.c
@@ -1215,6 +1215,21 @@ static int sxe_objects_compare(zval *object1, zval *object2) /* {{{ */
 }
 /* }}} */
 
+static const char *sxe_get_object_type_name(xmlXPathObjectType type)
+{
+	switch (type) {
+		case XPATH_BOOLEAN: return "bool";
+		case XPATH_NUMBER: return "number";
+		case XPATH_STRING: return "string";
+#ifdef LIBXML_XPTR_LOCS_ENABLED
+		case XPATH_POINT: return "point";
+		case XPATH_RANGE: return "range";
+		case XPATH_LOCATIONSET: return "location set";
+#endif
+		default: return "undefined";
+	}
+}
+
 /* {{{ Runs XPath query on the XML data */
 PHP_METHOD(SimpleXMLElement, xpath)
 {
@@ -1271,6 +1286,13 @@ PHP_METHOD(SimpleXMLElement, xpath)
 		RETURN_FALSE;
 	}
 
+	if (UNEXPECTED(retval->type != XPATH_NODESET)) {
+		php_error_docref(NULL, E_WARNING, "XPath expression must return a node set, %s returned",
+			sxe_get_object_type_name(retval->type));
+		xmlXPathFreeObject(retval);
+		RETURN_FALSE;
+	}
+
 	result = retval->nodesetval;
 
 	if (result != NULL) {
diff --git a/ext/simplexml/tests/008.phpt b/ext/simplexml/tests/008.phpt
index c946c36dafe6..dea6f98eacfc 100644
--- a/ext/simplexml/tests/008.phpt
+++ b/ext/simplexml/tests/008.phpt
@@ -39,8 +39,9 @@ array(1) {
     }
   }
 }
-array(0) {
-}
 
-Warning: SimpleXMLElement::xpath(): Invalid expression in %s on line %d%A
+Warning: SimpleXMLElement::xpath(): XPath expression must return a node set, number returned in %s on line %d
+bool(false)
+
+Warning: SimpleXMLElement::xpath(): Invalid expression in %s on line %d
 bool(false)
diff --git a/ext/simplexml/tests/gh12231.phpt b/ext/simplexml/tests/gh12231.phpt
new file mode 100644
index 000000000000..efacd92b76f9
--- /dev/null
+++ b/ext/simplexml/tests/gh12231.phpt
@@ -0,0 +1,26 @@
+--TEST--
+GH-12231 (SimpleXML xpath should warn when returning other return types than node lists)
+--EXTENSIONS--
+simplexml
+--FILE--
+<?php
+
+$xml = "<container><foo/><foo/></container>";
+$sxe = simplexml_load_string($xml);
+
+var_dump($sxe->xpath("count(//foo)"));
+var_dump($sxe->xpath("string(//foo)"));
+var_dump($sxe->xpath("boolean(//foo)"));
+var_dump(count($sxe->xpath("//foo")));
+
+?>
+--EXPECTF--
+Warning: SimpleXMLElement::xpath(): XPath expression must return a node set, number returned in %s on line %d
+bool(false)
+
+Warning: SimpleXMLElement::xpath(): XPath expression must return a node set, string returned in %s on line %d
+bool(false)
+
+Warning: SimpleXMLElement::xpath(): XPath expression must return a node set, bool returned in %s on line %d
+bool(false)
+int(2)