ext/xml: Use object instead of resource

nikic · nikic · commit e35a3cb28cd6 · 2019-04-08T16:23:13.000+02:00
Use an XmlParser object instead of a resource. This is an internal
representation change, not a conversion to OO APIs. XmlParser objects
cannot be explicitly constructed, they are created through the usual
xml_parser_* APIs.

This change allows us to provide a proper get_gc() implementation,
thus resolving bugs #72793 and #76874.

xml_parser_free() is a no-op now and need not be called anymore.
diff --git a/NEWS b/NEWS
@@ -31,4 +31,7 @@ PHP                                                                        NEWS
 - sodium:
   . Fixed bug #77646 (sign_detached() strings not terminated). (Frank)
 
+- XML:
+  . Fixed bug #76874 (xml_parser_free() should never leak memory). (Nikita)
+
 <<< NOTE: Insert NEWS from last stable release here prior to actual release! >>>
diff --git a/UPGRADING b/UPGRADING
@@ -189,6 +189,13 @@ PHP 8.0 UPGRADE NOTES
   . The quotemeta() function will now return an empty string if an empty string
     was passed. Previously false was returned.
 
+- XML:
+  . xml_parser_create(_ns) will now return an XmlParser object rather than a
+    resource. Return value checks using is_resource() should be replaced with
+    checks for `false`. The xml_parser_free() function no longer has an effect,
+    instead the XmlParser instance is automatically destroyed if it is no longer
+    referenced.
+
 - Zlib:
   . gzgetss() has been removed.
 
diff --git a/ext/xml/tests/bug72793.phpt b/ext/xml/tests/bug72793.phpt
@@ -0,0 +1,33 @@
+--TEST--
+Bug #72793: xml_parser_free leaks mem when execute xml_set_object
+--FILE--
+<?php
+
+class xml  {
+    var $parser;
+
+    function __construct()
+    {
+        $this->parser = xml_parser_create();
+        xml_set_object($this->parser, $this);
+    }
+
+    function parse($data)
+    {
+        xml_parse($this->parser, $data);
+    }
+
+    function free(){
+        xml_parser_free($this->parser);
+    }
+}
+
+$xml_test = '<?xml version="1.0" encoding="utf-8"?><test></test>';
+$xml_parser = new xml();
+$xml_parser->parse($xml_test);
+$xml_parser->free();
+
+?>
+===DONE===
+--EXPECT--
+===DONE===
diff --git a/ext/xml/tests/bug76874.phpt b/ext/xml/tests/bug76874.phpt
@@ -0,0 +1,29 @@
+--TEST--
+Bug #76874: xml_parser_free() should never leak memory
+--FILE--
+<?php
+
+class c
+{
+	private $xml;
+	private $test;
+
+	public function test()
+	{
+		$this->xml = xml_parser_create();
+		xml_set_character_data_handler($this->xml, array(&$this, 'handle_cdata'));
+		xml_parser_free($this->xml);
+	}
+
+	public function handle_cdata(&$parser, $data)
+	{
+	}
+}
+
+$object = new c();
+$object->test();
+
+?>
+===DONE===
+--EXPECT--
+===DONE===
diff --git a/ext/xml/xml.c b/ext/xml/xml.c
