summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorChristoph M. Becker <cmbecker69@gmx.de>2020-02-03 23:10:20 +0100
committerChristoph M. Becker <cmbecker69@gmx.de>2020-02-03 23:31:46 +0100
commitfe1bfb78d65d28dd151da417477a0cee51de8afb (patch)
treeef3cb80b0b846a838ec47cb3266b080223cabfe3
parentb93e4aa11c409ae34c9088ba976117438fc80b9c (diff)
downloadphp-git-fe1bfb78d65d28dd151da417477a0cee51de8afb.tar.gz
Fix #79191: Error in SoapClient ctor disables DOMDocument::save()
The culprit is the too restrictive fix for bug #71536, which prevents `php_libxml_streams_IO_write()` from properly executing when unclean shutdown is flagged. A *more* suitable solution is to move the `xmlwriter_free_resource_ptr()` call from the `free_obj` handler to an added `dtor_obj` handler, to avoid to write to a closed stream in case of late object freeing. This makes the `EG(active)` guard superfluous. We also fix bug79029.phpt which has to use different variables for the three parts to actually check the original shutdown issue. Thanks to bwoebi and daverandom for helping to investigate this issue.
-rw-r--r--NEWS4
-rw-r--r--ext/libxml/libxml.c3
-rw-r--r--ext/libxml/tests/bug79191.phpt24
-rw-r--r--ext/xmlwriter/php_xmlwriter.c35
-rw-r--r--ext/xmlwriter/tests/bug71536.phpt24
-rw-r--r--ext/xmlwriter/tests/bug79029.phpt8
6 files changed, 77 insertions, 21 deletions
diff --git a/NEWS b/NEWS
index 87d93a30e4..06c7e567d4 100644
--- a/NEWS
+++ b/NEWS
@@ -17,6 +17,10 @@ PHP NEWS
-Intl:
. Fixed bug #79212 (NumberFormatter::format() may detect wrong type). (cmb)
+- Libxml:
+ . Fixed bug #79191 (Error in SoapClient ctor disables DOMDocument::save()).
+ (Nikita, cmb)
+
- MBString:
. Fixed bug #79154 (mb_convert_encoding() can modify $from_encoding). (cmb)
diff --git a/ext/libxml/libxml.c b/ext/libxml/libxml.c
index 864e5a36fb..2be6a5b47a 100644
--- a/ext/libxml/libxml.c
+++ b/ext/libxml/libxml.c
@@ -385,9 +385,6 @@ static int php_libxml_streams_IO_read(void *context, char *buffer, int len)
static int php_libxml_streams_IO_write(void *context, const char *buffer, int len)
{
- if (CG(unclean_shutdown)) {
- return -1;
- }
return php_stream_write((php_stream*)context, buffer, len);
}
diff --git a/ext/libxml/tests/bug79191.phpt b/ext/libxml/tests/bug79191.phpt
new file mode 100644
index 0000000000..7d0dc83f23
--- /dev/null
+++ b/ext/libxml/tests/bug79191.phpt
@@ -0,0 +1,24 @@
+--TEST--
+Bug #79191 (Error in SoapClient ctor disables DOMDocument::save())
+--SKIPIF--
+<?php
+if (!extension_loaded('soap')) die('skip soap extension not available');
+if (!extension_loaded('dom')) die('dom extension not available');
+?>
+--FILE--
+<?php
+try {
+ new \SoapClient('does-not-exist.wsdl');
+} catch (Throwable $t) {
+}
+
+$dom = new DOMDocument;
+$dom->loadxml('<?xml version="1.0" ?><root />');
+var_dump($dom->save(__DIR__ . '/bug79191.xml'));
+?>
+--CLEAN--
+<?php
+unlink(__DIR__ . '/bug79191.xml');
+?>
+--EXPECTF--
+int(%d)
diff --git a/ext/xmlwriter/php_xmlwriter.c b/ext/xmlwriter/php_xmlwriter.c
index 24bb9dd182..aac3049b2f 100644
--- a/ext/xmlwriter/php_xmlwriter.c
+++ b/ext/xmlwriter/php_xmlwriter.c
@@ -91,15 +91,13 @@ typedef int (*xmlwriter_read_int_t)(xmlTextWriterPtr writer);
static void xmlwriter_free_resource_ptr(xmlwriter_object *intern)
{
if (intern) {
- if (EG(active)) {
- if (intern->ptr) {
- xmlFreeTextWriter(intern->ptr);
- intern->ptr = NULL;
- }
- if (intern->output) {
- xmlBufferFree(intern->output);
- intern->output = NULL;
- }
+ if (intern->ptr) {
+ xmlFreeTextWriter(intern->ptr);
+ intern->ptr = NULL;
+ }
+ if (intern->output) {
+ xmlBufferFree(intern->output);
+ intern->output = NULL;
}
efree(intern);
}
@@ -120,17 +118,25 @@ static void xmlwriter_free_resource_ptr(xmlwriter_object *intern)
static zend_object_handlers xmlwriter_object_handlers;
-/* {{{ xmlwriter_object_free_storage */
-static void xmlwriter_object_free_storage(zend_object *object)
+/* {{{{ xmlwriter_object_dtor */
+static void xmlwriter_object_dtor(zend_object *object)
{
ze_xmlwriter_object *intern = php_xmlwriter_fetch_object(object);
- if (!intern) {
- return;
- }
+
+ /* freeing the resource here may leak, but otherwise we may use it after it has been freed */
if (intern->xmlwriter_ptr) {
xmlwriter_free_resource_ptr(intern->xmlwriter_ptr);
}
intern->xmlwriter_ptr = NULL;
+ zend_objects_destroy_object(object);
+}
+/* }}} */
+
+/* {{{ xmlwriter_object_free_storage */
+static void xmlwriter_object_free_storage(zend_object *object)
+{
+ ze_xmlwriter_object *intern = php_xmlwriter_fetch_object(object);
+
zend_object_std_dtor(&intern->std);
}
/* }}} */
@@ -1844,6 +1850,7 @@ static PHP_MINIT_FUNCTION(xmlwriter)
memcpy(&xmlwriter_object_handlers, &std_object_handlers, sizeof(zend_object_handlers));
xmlwriter_object_handlers.offset = XtOffsetOf(ze_xmlwriter_object, std);
+ xmlwriter_object_handlers.dtor_obj = xmlwriter_object_dtor;
xmlwriter_object_handlers.free_obj = xmlwriter_object_free_storage;
xmlwriter_object_handlers.clone_obj = NULL;
INIT_CLASS_ENTRY(ce, "XMLWriter", xmlwriter_class_functions);
diff --git a/ext/xmlwriter/tests/bug71536.phpt b/ext/xmlwriter/tests/bug71536.phpt
new file mode 100644
index 0000000000..4ce2f2e404
--- /dev/null
+++ b/ext/xmlwriter/tests/bug71536.phpt
@@ -0,0 +1,24 @@
+--TEST--
+Bug #71536 (Access Violation crashes php-cgi.exe)
+--SKIPIF--
+<?php
+if (!extension_loaded('xmlwriter')) die('skip xmlwriter extension not available');
+?>
+--FILE--
+<?php
+class Test {
+ public static function init()
+ {
+ $xml = new \XMLWriter();
+ $xml->openUri('php://memory');
+ $xml->setIndent(false);
+ $xml->startDocument('1.0', 'UTF-8');
+ $xml->startElement('response');
+ die('now'); // crashed with die()
+ }
+}
+
+Test::init();
+?>
+--EXPECT--
+now
diff --git a/ext/xmlwriter/tests/bug79029.phpt b/ext/xmlwriter/tests/bug79029.phpt
index 2e76a4e409..b6b0c84b18 100644
--- a/ext/xmlwriter/tests/bug79029.phpt
+++ b/ext/xmlwriter/tests/bug79029.phpt
@@ -11,13 +11,13 @@ $x = array( new XMLWriter() );
$x[0]->openUri("bug79029_1.txt");
$x[0]->startComment();
-$x = new XMLWriter();
-$x->openUri("bug79029_2.txt");
+$y = new XMLWriter();
+$y->openUri("bug79029_2.txt");
fclose(@end(get_resources()));
file_put_contents("bug79029_3.txt", "a");
-$x = new XMLReader();
-$x->open("bug79029_3.txt");
+$z = new XMLReader();
+$z->open("bug79029_3.txt");
fclose(@end(get_resources()));
?>
okey