From 0612e65d90afbbf54e7d59733de391bded22abbc Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Thu, 12 Aug 2021 16:31:20 +0200 Subject: [PATCH 1/2] Fix #81351: xml_parse may fail, but has no error code The fix for bug #73151[1] cured the symptoms, but not the root cause, namely xmlParse() must not be called recursively. Since that bugfix also messed up the error handling, we basically revert it (but also simplify the return), and then prevent calling the parser recursively. [1] --- ext/xml/compat.c | 10 +--------- ext/xml/tests/bug73135.phpt | 4 ++++ ext/xml/tests/bug81351.phpt | 28 ++++++++++++++++++++++++++++ ext/xml/xml.c | 8 ++++++++ 4 files changed, 41 insertions(+), 9 deletions(-) create mode 100644 ext/xml/tests/bug81351.phpt diff --git a/ext/xml/compat.c b/ext/xml/compat.c index be988a822f17..fc4525650fc2 100644 --- a/ext/xml/compat.c +++ b/ext/xml/compat.c @@ -565,16 +565,8 @@ XML_Parse(XML_Parser parser, const XML_Char *data, int data_len, int is_final) { int error; - if (parser->parser->lastError.level >= XML_ERR_WARNING) { - return 0; - } - error = xmlParseChunk(parser->parser, (char *) data, data_len, is_final); - if (error) { - return 0; - } else { - return 1; - } + return !error && parser->parser->lastError.level <= XML_ERR_WARNING; } PHP_XML_API int diff --git a/ext/xml/tests/bug73135.phpt b/ext/xml/tests/bug73135.phpt index ede7b7acf0ca..4ce1b24a508f 100644 --- a/ext/xml/tests/bug73135.phpt +++ b/ext/xml/tests/bug73135.phpt @@ -21,6 +21,10 @@ HERE; xml_parse($parser, $xml); ?> --EXPECTF-- +Warning: xml_parse(): Parser must not be called recursively. in %s%ebug73135.php on line %d + +Warning: xml_parse(): Parser must not be called recursively. in %s%ebug73135.php on line %d + Warning: xml_parse(): Unable to call handler ahihi() in %s%ebug73135.php on line %d Warning: xml_parse(): Unable to call handler ahihi() in %s%ebug73135.php on line %d diff --git a/ext/xml/tests/bug81351.phpt b/ext/xml/tests/bug81351.phpt new file mode 100644 index 000000000000..19e4ca590b90 --- /dev/null +++ b/ext/xml/tests/bug81351.phpt @@ -0,0 +1,28 @@ +--TEST-- +Bug #81351 (xml_parse may fail, but has no error code) +--SKIPIF-- + +--FILE-- + + + + +XML; + +$parser = xml_parser_create_ns('UTF-8'); +$success = xml_parse($parser, $xml, false); +$code = xml_get_error_code($parser); +$error = xml_error_string($code); +echo "xml_parse returned $success, xml_get_error_code = $code, xml_error_string = $error\r\n"; +$success = xml_parse($parser, 'Y>', true); +$code = xml_get_error_code($parser); +$error = xml_error_string($code); +echo "xml_parse returned $success, xml_get_error_code = $code, xml_error_string = $error\r\n"; +?> +--EXPECT-- +xml_parse returned 1, xml_get_error_code = 0, xml_error_string = No error +xml_parse returned 0, xml_get_error_code = 5, xml_error_string = Invalid document end diff --git a/ext/xml/xml.c b/ext/xml/xml.c index af3431daa438..916851def4e8 100644 --- a/ext/xml/xml.c +++ b/ext/xml/xml.c @@ -1416,6 +1416,10 @@ PHP_FUNCTION(xml_parse) RETURN_FALSE; } + if (parser->isparsing) { + php_error_docref(NULL, E_WARNING, "Parser must not be called recursively."); + RETURN_FALSE; + } parser->isparsing = 1; ret = XML_Parse(parser->parser, (XML_Char*)data, data_len, isFinal); parser->isparsing = 0; @@ -1467,6 +1471,10 @@ PHP_FUNCTION(xml_parse_into_struct) XML_SetElementHandler(parser->parser, _xml_startElementHandler, _xml_endElementHandler); XML_SetCharacterDataHandler(parser->parser, _xml_characterDataHandler); + if (parser->isparsing) { + php_error_docref(NULL, E_WARNING, "Parser must not be called recursively."); + RETURN_FALSE; + } parser->isparsing = 1; ret = XML_Parse(parser->parser, (XML_Char*)data, data_len, 1); parser->isparsing = 0; From de5b6def53992c5221c8741d8420f635297a5a99 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Fri, 13 Aug 2021 16:09:11 +0200 Subject: [PATCH 2/2] Remove trailing period from error message Co-authored-by: Nikita Popov --- ext/xml/tests/bug73135.phpt | 4 ++-- ext/xml/xml.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ext/xml/tests/bug73135.phpt b/ext/xml/tests/bug73135.phpt index 4ce1b24a508f..ef1ca6c75989 100644 --- a/ext/xml/tests/bug73135.phpt +++ b/ext/xml/tests/bug73135.phpt @@ -21,9 +21,9 @@ HERE; xml_parse($parser, $xml); ?> --EXPECTF-- -Warning: xml_parse(): Parser must not be called recursively. in %s%ebug73135.php on line %d +Warning: xml_parse(): Parser must not be called recursively in %s%ebug73135.php on line %d -Warning: xml_parse(): Parser must not be called recursively. in %s%ebug73135.php on line %d +Warning: xml_parse(): Parser must not be called recursively in %s%ebug73135.php on line %d Warning: xml_parse(): Unable to call handler ahihi() in %s%ebug73135.php on line %d diff --git a/ext/xml/xml.c b/ext/xml/xml.c index 916851def4e8..fd8aebe03a52 100644 --- a/ext/xml/xml.c +++ b/ext/xml/xml.c @@ -1417,7 +1417,7 @@ PHP_FUNCTION(xml_parse) } if (parser->isparsing) { - php_error_docref(NULL, E_WARNING, "Parser must not be called recursively."); + php_error_docref(NULL, E_WARNING, "Parser must not be called recursively"); RETURN_FALSE; } parser->isparsing = 1; @@ -1472,7 +1472,7 @@ PHP_FUNCTION(xml_parse_into_struct) XML_SetCharacterDataHandler(parser->parser, _xml_characterDataHandler); if (parser->isparsing) { - php_error_docref(NULL, E_WARNING, "Parser must not be called recursively."); + php_error_docref(NULL, E_WARNING, "Parser must not be called recursively"); RETURN_FALSE; } parser->isparsing = 1;