ext/xml: Refactor extension to use FCC instead of zvals for handlers (#12340)

To get proper errors and sensible behaviour, as the current behaviour is somewhat insane and part of it should be axed ASAP.

The behaviour is mostly intact with some minor BC breaks which are mentioned in UPGRADING.

Co-authored-by: Niels Dossche <[email protected]>

Author: Girgias

UPGRADING

@@ -49,6 +49,17 @@ PHP 8.4 UPGRADE NOTES
     for invalid modes. Previously invalid modes would have been interpreted as
     PHP_ROUND_HALF_UP.
 
+- XML:
+  . The xml_set_*_handler() functions now declare and check for an effective
+    signature of callable|string|null for the $handler parameters.
+    Moreover, values of type string that correspond to method names,
+    of object set with xml_set_object() are now checked to see if the method
+    exists on the class of the previously passed object.
+    This means that xml_set_object() must now always be called prior to setting
+    method names as callables.
+    Passing an empty string to disable the handler is still allowed,
+    but not recommended.
+
 - XSL:
   . XSLTProcessor::setParameter() will now throw a ValueError when its arguments
     contain null bytes. This never actually worked correctly in the first place,

ext/xml/tests/bug30266.phpt

@@ -42,6 +42,7 @@ class XML_Parser
 $p1 = new Xml_Parser();
 try {
     $p1->parse('<tag1><tag2></tag2></tag1>');
+    echo "Exception swallowed\n";
 } catch (Exception $e) {
     echo "OK\n";
 }

ext/xml/tests/bug32001.phpt

@@ -100,8 +100,8 @@ HERE;
 
         $parser = xml_parser_create(NULL);
         xml_parser_set_option($parser, XML_OPTION_CASE_FOLDING, 0);
-        xml_set_element_handler($parser, "start_element", "end_element");
         xml_set_object($parser, $this);
+        xml_set_element_handler($parser, "start_element", "end_element");
 
         if ($this->chunk_size == 0) {
             $success = @xml_parse($parser, $data, true);

ext/xml/tests/bug72085.phpt

@@ -6,18 +6,20 @@ xml
 edgarsandi - <[email protected]>
 --FILE--
 <?php
-    function start_elem($parser, $xml) {
-        xml_parse($parser, $xml);
-    }
+function start_elem($parser, $xml) {
+    xml_parse($parser, $xml);
+}
+
+function dummy() {}
 
-    $xml = <<<HERE
-    <a xmlns="ahihi">
-        <bar foo="ahihi"/>
-    </a>
+$xml = <<<HERE
+<a xmlns="ahihi">
+    <bar foo="ahihi"/>
+</a>
 HERE;
 
     $parser = xml_parser_create_ns();
-    xml_set_element_handler($parser, 'start_elem', 'ahihi');
+    xml_set_element_handler($parser, 'start_elem', 'dummy');
     xml_parse($parser, $xml);
 ?>
 --EXPECTF--

ext/xml/tests/bug73135.phpt

@@ -0,0 +1,97 @@
+--TEST--
+Test xml_set_element_handler handlers as trampoline callback
+--EXTENSIONS--
+xml
+--FILE--
+<?php
+
+class CustomXmlParser
+{
+    public function startHandler($XmlParser, $tag, $attr)
+    {
+        echo 'Method start handler: ', $tag, PHP_EOL;
+    }
+
+    public function endHandler($XmlParser, $tag)
+    {
+        echo 'Method end handler: ', $tag, PHP_EOL;
+    }
+}
+
+$customParser = new CustomXmlParser;
+
+class TrampolineTest {
+    public function __call(string $name, array $arguments) {
+        echo 'Trampoline for ', $name, PHP_EOL;
+        echo 'Tag: ', $arguments[1], PHP_EOL;
+    }
+}
+
+$o = new TrampolineTest();
+$startCallback = [$o, 'start_handler'];
+$endCallback = [$o, 'end_handler'];
+
+$xml = <<<HERE
+<a>
+  <b/>
+  <c>Text</c>
+</a>
+HERE;
+
+echo "Both handlers are trampolines:\n";
+$parser = xml_parser_create();
+xml_set_element_handler($parser, $startCallback, $endCallback);
+xml_parse($parser, $xml, true);
+xml_parser_free($parser);
+
+echo "\nStart handler is trampoline, end handler method string:\n";
+$parser = xml_parser_create();
+xml_set_object($parser, $customParser);
+xml_set_element_handler($parser, $startCallback, 'endHandler');
+xml_parse($parser, $xml, true);
+xml_parser_free($parser);
+
+echo "\nEnd handler is trampoline, start handler method string:\n";
+$parser = xml_parser_create();
+xml_set_object($parser, $customParser);
+xml_set_element_handler($parser, 'startHandler', $endCallback);
+xml_parse($parser, $xml, true);
+xml_parser_free($parser);
+
+?>
+--EXPECT--
+Both handlers are trampolines:
+Trampoline for start_handler
+Tag: A
+Trampoline for start_handler
+Tag: B
+Trampoline for end_handler
+Tag: B
+Trampoline for start_handler
+Tag: C
+Trampoline for end_handler
+Tag: C
+Trampoline for end_handler
+Tag: A
+
+Start handler is trampoline, end handler method string:
+Trampoline for start_handler
+Tag: A
+Trampoline for start_handler
+Tag: B
+Method end handler: B
+Trampoline for start_handler
+Tag: C
+Method end handler: C
+Method end handler: A
+
+End handler is trampoline, start handler method string:
+Method start handler: A
+Method start handler: B
+Trampoline for end_handler
+Tag: B
+Method start handler: C
+Trampoline for end_handler
+Tag: C
+Trampoline for end_handler
+Tag: A

ext/xml/tests/set_element_handler_trampoline.phpt

@@ -0,0 +1,46 @@
+--TEST--
+Test xml_set_element_handler handlers as trampoline callback
+--EXTENSIONS--
+xml
+--FILE--
+<?php
+
+class TrampolineTest {
+    public function __call(string $name, array $arguments) {
+        echo 'Trampoline for ', $name, PHP_EOL;
+        echo 'Tag: ', $arguments[1], PHP_EOL;
+    }
+}
+
+$o = new TrampolineTest();
+$startCallback = [$o, 'start_handler'];
+$endCallback = [$o, 'end_handler'];
+
+$xml = <<<HERE
+<a>
+  <b/>
+  <c>Text</c>
+</a>
+HERE;
+
+$parser = xml_parser_create();
+echo "2nd arg is rubbish:\n";
+try {
+    xml_set_element_handler($parser, [], $endCallback);
+} catch (\Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+echo "3rd arg is rubbish:\n";
+try {
+    xml_set_element_handler($parser, $startCallback, []);
+} catch (\Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+xml_parser_free($parser);
+
+?>
+--EXPECT--
+2nd arg is rubbish:
+TypeError: xml_set_element_handler(): Argument #2 ($start_handler) must be of type callable|string|null
+3rd arg is rubbish:
+TypeError: xml_set_element_handler(): Argument #2 ($start_handler) must be of type callable|string|null

ext/xml/tests/set_element_handler_trampoline_errors.phpt

@@ -0,0 +1,61 @@
+--TEST--
+Error conditions when setting invalid handler callables
+--EXTENSIONS--
+xml
+--FILE--
+<?php
+declare(strict_types=1);
+
+/* Use xml_set_processing_instruction_handler() for generic implementation */
+echo 'Invalid $parser:', PHP_EOL;
+$obj = new stdClass();
+try {
+    xml_set_processing_instruction_handler($obj, null);
+} catch (\Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+/* Create valid parser */
+$parser = xml_parser_create();
+
+echo 'Invalid callable type true:', PHP_EOL;
+try {
+    xml_set_processing_instruction_handler($parser, true);
+} catch (\Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+echo 'Invalid callable type int:', PHP_EOL;
+try {
+    xml_set_processing_instruction_handler($parser, 10);
+} catch (\Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+echo 'String not callable and no object set:', PHP_EOL;
+try {
+    xml_set_processing_instruction_handler($parser, "nonexistent_method");
+} catch (\Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+echo 'String non existent method on set object:', PHP_EOL;
+xml_set_object($parser, $obj);
+try {
+    xml_set_processing_instruction_handler($parser, "nonexistent_method");
+} catch (\Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+?>
+--EXPECT--
+Invalid $parser:
+TypeError: xml_set_processing_instruction_handler(): Argument #1 ($parser) must be of type XMLParser, stdClass given
+Invalid callable type true:
+TypeError: xml_set_processing_instruction_handler(): Argument #2 ($handler) must be of type callable|string|null
+Invalid callable type int:
+TypeError: xml_set_processing_instruction_handler(): Argument #2 ($handler) must be of type callable|string|null
+String not callable and no object set:
+ValueError: xml_set_processing_instruction_handler(): Argument #2 ($handler) an object must be set via xml_set_object() to be able to lookup method
+String non existent method on set object:
+ValueError: xml_set_processing_instruction_handler(): Argument #2 ($handler) method stdClass::nonexistent_method() does not exist

ext/xml/tests/set_handler_errors.phpt

@@ -0,0 +1,33 @@
+--TEST--
+Test XMLParser generic handlers as trampoline callback
+--EXTENSIONS--
+xml
+--FILE--
+<?php
+class TrampolineTest {
+    public function __call(string $name, array $arguments) {
+        echo 'Trampoline for ', $name, PHP_EOL;
+        echo 'Target: ', $arguments[1], PHP_EOL;
+        echo 'Data: ', $arguments[2], PHP_EOL;
+    }
+}
+
+$o = new TrampolineTest();
+$callback = [$o, 'pi_handler'];
+
+$xml = <<<HERE
+<?xml version="1.0" encoding="ISO-8859-1"?>
+<?xml-stylesheet href="default.xsl" type="text/xml"?>
+HERE;
+
+/* Use xml_set_processing_instruction_handler() for generic implementation */
+$parser = xml_parser_create();
+xml_set_processing_instruction_handler($parser, $callback);
+xml_parse($parser, $xml, true);
+xml_parser_free($parser);
+
+?>
+--EXPECT--
+Trampoline for pi_handler
+Target: xml-stylesheet
+Data: href="default.xsl" type="text/xml"