[RFC] Improve callbacks in ext/dom and ext/xsl (#12627)

Author: nielsdos

NEWS

@@ -22,6 +22,7 @@ DOM:
   . Implement DOM HTML5 parsing and serialization RFC. (nielsdos)
   . Fix DOMElement->prefix with empty string creates bogus prefix. (nielsdos)
   . Handle OOM more consistently. (nielsdos)
+  . Implemented "Improve callbacks in ext/dom and ext/xsl" RFC. (nielsdos)
 
 FPM:
   . Implement GH-12385 (flush headers without body when calling flush()).
@@ -147,5 +148,6 @@ XML:
 XSL:
   . Implement request #64137 (XSLTProcessor::setParameter() should allow both
     quotes to be used). (nielsdos)
+  . Implemented "Improve callbacks in ext/dom and ext/xsl" RFC. (nielsdos)
 
 <<< NOTE: Insert NEWS from last stable release here prior to actual release! >>>

UPGRADING

@@ -121,6 +121,9 @@ PHP 8.4 UPGRADE NOTES
   . XSLTProcessor::setParameter() will now throw a ValueError when its arguments
     contain null bytes. This never actually worked correctly in the first place,
     which is why it throws an exception nowadays.
+  . Failure to call a PHP function callback during evaluation now throws
+    instead of emitting a warning.
+    RFC: https://wiki.php.net/rfc/improve_callbacks_dom_and_xsl
 
 ========================================
 2. New Features
@@ -145,6 +148,8 @@ PHP 8.4 UPGRADE NOTES
     These classes provide a cleaner API to handle HTML and XML documents.
     Furthermore, the DOM\HTMLDocument class implements spec-compliant HTML5
     parsing and serialization.
+  . It is now possible to pass any callable to registerPhpFunctions().
+    RFC: https://wiki.php.net/rfc/improve_callbacks_dom_and_xsl
 
 - FPM:
   . Flushing headers without a body will now succeed. See GH-12785.
@@ -195,6 +200,8 @@ PDO_SQLITE:
 - XSL:
   . It is now possible to use parameters that contain both single and double
     quotes.
+  . It is now possible to pass any callable to registerPhpFunctions().
+    RFC: https://wiki.php.net/rfc/improve_callbacks_dom_and_xsl
 
 ========================================
 3. Changes in SAPI modules
@@ -318,6 +325,8 @@ PDO_SQLITE:
 
 - DOM:
   . Added DOMNode::compareDocumentPosition().
+  . Added DOMXPath::registerPhpFunctionNS().
+    RFC: https://wiki.php.net/rfc/improve_callbacks_dom_and_xsl
 
 - MBString:
   . Added mb_trim, mb_ltrim and mb_rtrim functions.
@@ -334,6 +343,10 @@ PDO_SQLITE:
   . sodium_crypto_aead_aes256gcm_*() functions are now enabled on aarch64 CPUs
     with the ARM cryptographic extensions.
 
+- XSL:
+  . Added XSLTProcessor::registerPhpFunctionNS().
+    RFC: https://wiki.php.net/rfc/improve_callbacks_dom_and_xsl
+
 ========================================
 7. New Classes and Interfaces
 ========================================

UPGRADING.INTERNALS

@@ -50,6 +50,8 @@ PHP 8.4 INTERNALS UPGRADE NOTES
    - dom_read_t and dom_write_t now expect the function to return zend_result
      instead of int.
    - The macros DOM_NO_ARGS() and DOM_NOT_IMPLEMENTED() have been removed.
+   - New public APIs are available to handle callbacks from XPath, see
+     xpath_callbacks.h.
 
  b. ext/random
    - The macro RAND_RANGE_BADSCALING() has been removed. The implementation

ext/dom/config.m4

@@ -35,7 +35,7 @@ if test "$PHP_DOM" != "no"; then
                             nodelist.c text.c comment.c \
                             entityreference.c \
                             notation.c xpath.c dom_iterators.c \
-                            namednodemap.c \
+                            namednodemap.c xpath_callbacks.c \
                             $LEXBOR_SOURCES],
                             $ext_shared,,$PHP_LEXBOR_CFLAGS)
     PHP_ADD_BUILD_DIR($ext_builddir/$LEXBOR_DIR/ports/posix/lexbor/core)
@@ -49,7 +49,7 @@ if test "$PHP_DOM" != "no"; then
     PHP_ADD_BUILD_DIR($ext_builddir/$LEXBOR_DIR/ns)
     PHP_ADD_BUILD_DIR($ext_builddir/$LEXBOR_DIR/tag)
     PHP_SUBST(DOM_SHARED_LIBADD)
-    PHP_INSTALL_HEADERS([ext/dom/xml_common.h])
+    PHP_INSTALL_HEADERS([ext/dom/xml_common.h ext/dom/xpath_callbacks.h])
     PHP_ADD_EXTENSION_DEP(dom, libxml)
   ])
 fi

ext/dom/config.w32

@@ -15,7 +15,7 @@ if (PHP_DOM == "yes") {
 			entity.c nodelist.c text.c comment.c \
 			entityreference.c \
 			notation.c xpath.c dom_iterators.c \
-			namednodemap.c", null, "-Iext/dom/lexbor");
+			namednodemap.c xpath_callbacks.c", null, "-Iext/dom/lexbor");
 
 		ADD_SOURCES("ext/dom/lexbor/lexbor/ports/windows_nt/lexbor/core", "memory.c", "dom");
 		ADD_SOURCES("ext/dom/lexbor/lexbor/core", "array_obj.c array.c avl.c bst.c diyfp.c conv.c dobject.c dtoa.c hash.c mem.c mraw.c print.c serialize.c shs.c str.c strtod.c", "dom");
@@ -41,7 +41,7 @@ if (PHP_DOM == "yes") {
 				WARNING("dom support can't be enabled, libxml is not found")
 			}
 		}
-		PHP_INSTALL_HEADERS("ext/dom", "xml_common.h");
+		PHP_INSTALL_HEADERS("ext/dom", "xml_common.h xpath_callbacks.h");
 	} else {
 		WARNING("dom support can't be enabled, libxml is not enabled")
 		PHP_DOM = "no"

ext/dom/php_dom.c

@@ -595,8 +595,10 @@ static int dom_nodelist_has_dimension(zend_object *object, zval *member, int che
 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);
+HashTable *dom_xpath_get_gc(zend_object *object, zval **table, int *n);
 #endif
 
 static void *dom_malloc(size_t size) {
@@ -888,6 +890,7 @@ PHP_MINIT_FUNCTION(dom)
 	memcpy(&dom_xpath_object_handlers, &dom_object_handlers, sizeof(zend_object_handlers));
 	dom_xpath_object_handlers.offset = XtOffsetOf(dom_xpath_object, dom) + XtOffsetOf(dom_object, std);
 	dom_xpath_object_handlers.free_obj = dom_xpath_objects_free_storage;
+	dom_xpath_object_handlers.get_gc = dom_xpath_get_gc;
 
 	dom_xpath_class_entry = register_class_DOMXPath();
 	dom_xpath_class_entry->create_object = dom_xpath_objects_new;
@@ -1000,32 +1003,6 @@ void node_list_unlink(xmlNodePtr node)
 }
 /* }}} end node_list_unlink */
 
-#ifdef LIBXML_XPATH_ENABLED
-/* {{{ dom_xpath_objects_free_storage */
-void dom_xpath_objects_free_storage(zend_object *object)
-{
-	dom_xpath_object *intern = php_xpath_obj_from_obj(object);
-
-	zend_object_std_dtor(&intern->dom.std);
-
-	if (intern->dom.ptr != NULL) {
-		xmlXPathFreeContext((xmlXPathContextPtr) intern->dom.ptr);
-		php_libxml_decrement_doc_ref((php_libxml_node_object *) &intern->dom);
-	}
-
-	if (intern->registered_phpfunctions) {
-		zend_hash_destroy(intern->registered_phpfunctions);
-		FREE_HASHTABLE(intern->registered_phpfunctions);
-	}
-
-	if (intern->node_list) {
-		zend_hash_destroy(intern->node_list);
-		FREE_HASHTABLE(intern->node_list);
-	}
-}
-/* }}} */
-#endif
-
 /* {{{ dom_objects_free_storage */
 void dom_objects_free_storage(zend_object *object)
 {
@@ -1132,12 +1109,13 @@ static void dom_object_namespace_node_free_storage(zend_object *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)
 {
 	dom_xpath_object *intern = zend_object_alloc(sizeof(dom_xpath_object), class_type);
 
-	intern->registered_phpfunctions = zend_new_array(0);
+	php_dom_xpath_callbacks_ctor(&intern->xpath_callbacks);
 	intern->register_node_ns = 1;
 
 	intern->dom.prop_handler = &dom_xpath_prop_handlers;
@@ -1148,6 +1126,7 @@ zend_object *dom_xpath_objects_new(zend_class_entry *class_type)
 	return &intern->dom.std;
 }
 /* }}} */
+
 #endif
 
 void dom_nnodemap_objects_free_storage(zend_object *object) /* {{{ */

ext/dom/php_dom.h

@@ -53,6 +53,7 @@ extern zend_module_entry dom_module_entry;
 
 #include "xml_common.h"
 #include "ext/libxml/php_libxml.h"
+#include "xpath_callbacks.h"
 #include "zend_exceptions.h"
 #include "dom_ce.h"
 /* DOM API_VERSION, please bump it up, if you change anything in the API
@@ -64,10 +65,8 @@ extern zend_module_entry dom_module_entry;
 #define DOM_NODESET XML_XINCLUDE_START
 
 typedef struct _dom_xpath_object {
-	int registerPhpFunctions;
+	php_dom_xpath_callbacks xpath_callbacks;
 	int register_node_ns;
-	HashTable *registered_phpfunctions;
-	HashTable *node_list;
 	dom_object dom;
 } dom_xpath_object;
 

ext/dom/php_dom.stub.php

@@ -932,6 +932,8 @@ public function registerNamespace(string $prefix, string $namespace): bool {}
 
         /** @tentative-return-type */
         public function registerPhpFunctions(string|array|null $restrict = null): void {}
+
+        public function registerPhpFunctionNS(string $namespaceURI, string $name, callable $callable): void {}
     }
 #endif
 

ext/dom/php_dom_arginfo.h

@@ -0,0 +1,88 @@
+--TEST--
+registerPHPFunctions() with callables - legit cases
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+
+class MyClass {
+    public static function dump(string $var) {
+        var_dump($var);
+    }
+}
+
+class MyDOMXPath extends DOMXPath {
+    public function registerCycle() {
+        $this->registerPhpFunctions(["cycle" => array($this, "dummy")]);
+    }
+
+    public function dummy(string $var) {
+        echo "dummy: $var\n";
+    }
+}
+
+$doc = new DOMDocument();
+$doc->loadHTML('<a href="https://php.net">hello</a>');
+
+echo "--- Legit cases: none ---\n";
+
+$xpath = new DOMXPath($doc);
+$xpath->registerNamespace("php", "http://php.net/xpath");
+try {
+    $xpath->evaluate("//a[php:function('var_dump', string(@href))]");
+} catch (Error $e) {
+    echo $e->getMessage(), "\n";
+}
+
+echo "--- Legit cases: all ---\n";
+
+$xpath->registerPHPFunctions(null);
+$xpath->evaluate("//a[php:function('var_dump', string(@href))]");
+$xpath->evaluate("//a[php:function('MyClass::dump', string(@href))]");
+
+echo "--- Legit cases: set ---\n";
+
+$xpath = new DOMXPath($doc);
+$xpath->registerNamespace("php", "http://php.net/xpath");
+$xpath->registerPhpFunctions([]);
+$xpath->registerPHPFunctions(["xyz" => MyClass::dump(...), "mydump" => function (string $x) {
+    var_dump($x);
+}]);
+$xpath->registerPhpFunctions(str_repeat("var_dump", mt_rand(1, 1) /* defeat SCCP */));
+$xpath->evaluate("//a[php:function('mydump', string(@href))]");
+$xpath->evaluate("//a[php:function('xyz', string(@href))]");
+$xpath->evaluate("//a[php:function('var_dump', string(@href))]");
+try {
+    $xpath->evaluate("//a[php:function('notinset', string(@href))]");
+} catch (Throwable $e) {
+    echo $e->getMessage(), "\n";
+}
+
+echo "--- Legit cases: set with cycle ---\n";
+
+$xpath = new MyDOMXPath($doc);
+$xpath->registerNamespace("php", "http://php.net/xpath");
+$xpath->registerCycle();
+$xpath->evaluate("//a[php:function('cycle', string(@href))]");
+
+echo "--- Legit cases: reset to null ---\n";
+
+$xpath->registerPhpFunctions(null);
+$xpath->evaluate("//a[php:function('var_dump', string(@href))]");
+
+?>
+--EXPECT--
+--- Legit cases: none ---
+No callbacks were registered
+--- Legit cases: all ---
+string(15) "https://php.net"
+string(15) "https://php.net"
+--- Legit cases: set ---
+string(15) "https://php.net"
+string(15) "https://php.net"
+string(15) "https://php.net"
+No callback handler "notinset" registered
+--- Legit cases: set with cycle ---
+dummy: https://php.net
+--- Legit cases: reset to null ---
+string(15) "https://php.net"