Skip to content

Commit fe1bfb7

Browse files
cmb69cmb69
committed
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.
1 parent b93e4aa commit fe1bfb7

File tree

6 files changed

+77
-21
lines changed

6 files changed

+77
-21
lines changed

‎NEWS‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ PHP NEWS
1717
-Intl:
1818
. Fixed bug #79212 (NumberFormatter::format() may detect wrong type). (cmb)
1919

20+
- Libxml:
21+
. Fixed bug #79191 (Error in SoapClient ctor disables DOMDocument::save()).
22+
(Nikita, cmb)
23+
2024
- MBString:
2125
. Fixed bug #79154 (mb_convert_encoding() can modify $from_encoding). (cmb)
2226

‎ext/libxml/libxml.c‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -385,9 +385,6 @@ static int php_libxml_streams_IO_read(void *context, char *buffer, int len)
385385

386386
static int php_libxml_streams_IO_write(void *context, const char *buffer, int len)
387387
{
388-
if (CG(unclean_shutdown)) {
389-
return -1;
390-
}
391388
return php_stream_write((php_stream*)context, buffer, len);
392389
}
393390

‎ext/libxml/tests/bug79191.phpt‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
--TEST--
2+
Bug #79191 (Error in SoapClient ctor disables DOMDocument::save())
3+
--SKIPIF--
4+
<?php
5+
if (!extension_loaded('soap')) die('skip soap extension not available');
6+
if (!extension_loaded('dom')) die('dom extension not available');
7+
?>
8+
--FILE--
9+
<?php
10+
try {
11+
new \SoapClient('does-not-exist.wsdl');
12+
} catch (Throwable $t) {
13+
}
14+
15+
$dom = new DOMDocument;
16+
$dom->loadxml('<?xml version="1.0" ?><root />');
17+
var_dump($dom->save(__DIR__ . '/bug79191.xml'));
18+
?>
19+
--CLEAN--
20+
<?php
21+
unlink(__DIR__ . '/bug79191.xml');
22+
?>
23+
--EXPECTF--
24+
int(%d)

‎ext/xmlwriter/php_xmlwriter.c‎

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -91,15 +91,13 @@ typedef int (*xmlwriter_read_int_t)(xmlTextWriterPtr writer);
9191
static void xmlwriter_free_resource_ptr(xmlwriter_object *intern)
9292
{
9393
if (intern) {
94-
if (EG(active)) {
95-
if (intern->ptr) {
96-
xmlFreeTextWriter(intern->ptr);
97-
intern->ptr = NULL;
98-
}
99-
if (intern->output) {
100-
xmlBufferFree(intern->output);
101-
intern->output = NULL;
102-
}
94+
if (intern->ptr) {
95+
xmlFreeTextWriter(intern->ptr);
96+
intern->ptr = NULL;
97+
}
98+
if (intern->output) {
99+
xmlBufferFree(intern->output);
100+
intern->output = NULL;
103101
}
104102
efree(intern);
105103
}
@@ -120,17 +118,25 @@ static void xmlwriter_free_resource_ptr(xmlwriter_object *intern)
120118

121119
static zend_object_handlers xmlwriter_object_handlers;
122120

123-
/* {{{ xmlwriter_object_free_storage */
124-
static void xmlwriter_object_free_storage(zend_object *object)
121+
/* {{{{ xmlwriter_object_dtor */
122+
static void xmlwriter_object_dtor(zend_object *object)
125123
{
126124
ze_xmlwriter_object *intern = php_xmlwriter_fetch_object(object);
127-
if (!intern) {
128-
return;
129-
}
125+
126+
/* freeing the resource here may leak, but otherwise we may use it after it has been freed */
130127
if (intern->xmlwriter_ptr) {
131128
xmlwriter_free_resource_ptr(intern->xmlwriter_ptr);
132129
}
133130
intern->xmlwriter_ptr = NULL;
131+
zend_objects_destroy_object(object);
132+
}
133+
/* }}} */
134+
135+
/* {{{ xmlwriter_object_free_storage */
136+
static void xmlwriter_object_free_storage(zend_object *object)
137+
{
138+
ze_xmlwriter_object *intern = php_xmlwriter_fetch_object(object);
139+
134140
zend_object_std_dtor(&intern->std);
135141
}
136142
/* }}} */
@@ -1844,6 +1850,7 @@ static PHP_MINIT_FUNCTION(xmlwriter)
18441850

18451851
memcpy(&xmlwriter_object_handlers, &std_object_handlers, sizeof(zend_object_handlers));
18461852
xmlwriter_object_handlers.offset = XtOffsetOf(ze_xmlwriter_object, std);
1853+
xmlwriter_object_handlers.dtor_obj = xmlwriter_object_dtor;
18471854
xmlwriter_object_handlers.free_obj = xmlwriter_object_free_storage;
18481855
xmlwriter_object_handlers.clone_obj = NULL;
18491856
INIT_CLASS_ENTRY(ce, "XMLWriter", xmlwriter_class_functions);

‎ext/xmlwriter/tests/bug71536.phpt‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
--TEST--
2+
Bug #71536 (Access Violation crashes php-cgi.exe)
3+
--SKIPIF--
4+
<?php
5+
if (!extension_loaded('xmlwriter')) die('skip xmlwriter extension not available');
6+
?>
7+
--FILE--
8+
<?php
9+
class Test {
10+
public static function init()
11+
{
12+
$xml = new \XMLWriter();
13+
$xml->openUri('php://memory');
14+
$xml->setIndent(false);
15+
$xml->startDocument('1.0', 'UTF-8');
16+
$xml->startElement('response');
17+
die('now'); // crashed with die()
18+
}
19+
}
20+
21+
Test::init();
22+
?>
23+
--EXPECT--
24+
now

‎ext/xmlwriter/tests/bug79029.phpt‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,13 @@ $x = array( new XMLWriter() );
1111
$x[0]->openUri("bug79029_1.txt");
1212
$x[0]->startComment();
1313

14-
$x = new XMLWriter();
15-
$x->openUri("bug79029_2.txt");
14+
$y = new XMLWriter();
15+
$y->openUri("bug79029_2.txt");
1616
fclose(@end(get_resources()));
1717

1818
file_put_contents("bug79029_3.txt", "a");
19-
$x = new XMLReader();
20-
$x->open("bug79029_3.txt");
19+
$z = new XMLReader();
20+
$z->open("bug79029_3.txt");
2121
fclose(@end(get_resources()));
2222
?>
2323
okey

0 commit comments

Comments
 (0)