From fe1bfb78d65d28dd151da417477a0cee51de8afb Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Mon, 3 Feb 2020 23:10:20 +0100 Subject: [PATCH] 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. --- NEWS | 4 ++++ ext/libxml/libxml.c | 3 --- ext/libxml/tests/bug79191.phpt | 24 +++++++++++++++++++++ ext/xmlwriter/php_xmlwriter.c | 35 ++++++++++++++++++------------- ext/xmlwriter/tests/bug71536.phpt | 24 +++++++++++++++++++++ ext/xmlwriter/tests/bug79029.phpt | 8 +++---- 6 files changed, 77 insertions(+), 21 deletions(-) create mode 100644 ext/libxml/tests/bug79191.phpt create mode 100644 ext/xmlwriter/tests/bug71536.phpt diff --git a/NEWS b/NEWS index 87d93a30e4a..06c7e567d45 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 864e5a36fb7..2be6a5b47a0 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 00000000000..7d0dc83f232 --- /dev/null +++ b/ext/libxml/tests/bug79191.phpt @@ -0,0 +1,24 @@ +--TEST-- +Bug #79191 (Error in SoapClient ctor disables DOMDocument::save()) +--SKIPIF-- + +--FILE-- +loadxml(''); +var_dump($dom->save(__DIR__ . '/bug79191.xml')); +?> +--CLEAN-- + +--EXPECTF-- +int(%d) diff --git a/ext/xmlwriter/php_xmlwriter.c b/ext/xmlwriter/php_xmlwriter.c index 24bb9dd1829..aac3049b2f0 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 00000000000..4ce2f2e404a --- /dev/null +++ b/ext/xmlwriter/tests/bug71536.phpt @@ -0,0 +1,24 @@ +--TEST-- +Bug #71536 (Access Violation crashes php-cgi.exe) +--SKIPIF-- + +--FILE-- +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 2e76a4e4095..b6b0c84b182 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