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 be988a822f179..fc4525650fc23 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 ede7b7acf0ca7..4ce1b24a508ff 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 0000000000000..19e4ca590b90d --- /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 af3431daa438c..916851def4e89 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 4ce1b24a508ff..ef1ca6c75989d 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 916851def4e89..fd8aebe03a524 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;