Skip to content

Commit 30f26b5

Browse files
committedSep 21, 2023
Fix memory leak when calling xml_parse_into_struct() twice
Closes GH-12254.
1 parent b1d9a8d commit 30f26b5

File tree

3 files changed

+49
-10
lines changed

3 files changed

+49
-10
lines changed
 

‎NEWS

+1
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ PHP NEWS
3636

3737
- XML:
3838
. Fix return type of stub of xml_parse_into_struct(). (nielsdos)
39+
. Fix memory leak when calling xml_parse_into_struct() twice. (nielsdos)
3940

4041
28 Sep 2023, PHP 8.1.24
4142

‎ext/xml/tests/gh12254.phpt

+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
--TEST--
2+
GH-12254: xml_parse_into_struct() memory leak when called twice
3+
--EXTENSIONS--
4+
xml
5+
--FILE--
6+
<?php
7+
8+
$parser = xml_parser_create();
9+
xml_set_element_handler($parser, function ($parser, $name, $attrs) {
10+
echo "open\n";
11+
var_dump($name, $attrs);
12+
var_dump(xml_parse_into_struct($parser, "<container/>", $values, $tags));
13+
}, function ($parser, $name) {
14+
echo "close\n";
15+
var_dump($name);
16+
});
17+
xml_parse_into_struct($parser, "<container/>", $values, $tags);
18+
// Yes, this doesn't do anything but it at least shouldn't leak...
19+
xml_parse_into_struct($parser, "<container/>", $values, $tags);
20+
21+
?>
22+
--EXPECTF--
23+
open
24+
string(9) "CONTAINER"
25+
array(0) {
26+
}
27+
28+
Warning: xml_parse_into_struct(): Parser must not be called recursively in %s on line %d
29+
bool(false)
30+
close
31+
string(9) "CONTAINER"

‎ext/xml/xml.c

+17-10
Original file line numberDiff line numberDiff line change
@@ -353,19 +353,24 @@ static zend_object *xml_parser_create_object(zend_class_entry *class_type) {
353353
return &intern->std;
354354
}
355355

356-
static void xml_parser_free_obj(zend_object *object)
356+
static void xml_parser_free_ltags(xml_parser *parser)
357357
{
358-
xml_parser *parser = xml_parser_from_obj(object);
359-
360-
if (parser->parser) {
361-
XML_ParserFree(parser->parser);
362-
}
363358
if (parser->ltags) {
364359
int inx;
365360
for (inx = 0; ((inx < parser->level) && (inx < XML_MAXLEVEL)); inx++)
366361
efree(parser->ltags[ inx ]);
367362
efree(parser->ltags);
368363
}
364+
}
365+
366+
static void xml_parser_free_obj(zend_object *object)
367+
{
368+
xml_parser *parser = xml_parser_from_obj(object);
369+
370+
if (parser->parser) {
371+
XML_ParserFree(parser->parser);
372+
}
373+
xml_parser_free_ltags(parser);
369374
if (!Z_ISUNDEF(parser->startElementHandler)) {
370375
zval_ptr_dtor(&parser->startElementHandler);
371376
}
@@ -1282,6 +1287,11 @@ PHP_FUNCTION(xml_parse_into_struct)
12821287

12831288
parser = Z_XMLPARSER_P(pind);
12841289

1290+
if (parser->isparsing) {
1291+
php_error_docref(NULL, E_WARNING, "Parser must not be called recursively");
1292+
RETURN_FALSE;
1293+
}
1294+
12851295
if (info) {
12861296
info = zend_try_array_init(info);
12871297
if (!info) {
@@ -1301,15 +1311,12 @@ PHP_FUNCTION(xml_parse_into_struct)
13011311
}
13021312

13031313
parser->level = 0;
1314+
xml_parser_free_ltags(parser);
13041315
parser->ltags = safe_emalloc(XML_MAXLEVEL, sizeof(char *), 0);
13051316

13061317
XML_SetElementHandler(parser->parser, _xml_startElementHandler, _xml_endElementHandler);
13071318
XML_SetCharacterDataHandler(parser->parser, _xml_characterDataHandler);
13081319

1309-
if (parser->isparsing) {
1310-
php_error_docref(NULL, E_WARNING, "Parser must not be called recursively");
1311-
RETURN_FALSE;
1312-
}
13131320
parser->isparsing = 1;
13141321
ret = XML_Parse(parser->parser, (XML_Char*)data, data_len, 1);
13151322
parser->isparsing = 0;

0 commit comments

Comments
 (0)