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.
This commit is contained in:
Christoph M. Becker 2020-02-03 23:10:20 +01:00
parent b93e4aa11c
commit fe1bfb78d6
6 changed files with 77 additions and 21 deletions

4
NEWS
View file

@ -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)

View file

@ -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);
}

View file

@ -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)

View file

@ -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);

View file

@ -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

View file

@ -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