Merge pull request #654 from libexpat/issue-613-fix-processing-of-nested-entities

Fix processing of nested entities (fixes #613)

3 files changed, 62 insertions, 9 deletions

diff --git a/expat/Changes b/expat/Changes
index 9d0dff87..fa4bab99 100644
--- a/expat/Changes
+++ b/expat/Changes
@@ -5,6 +5,8 @@ NOTE: We are looking for help with a few things:
 Release x.x.x xxx xxxxxxxxxxxx xx xxxx
         Bug fixes:
        #612 #645  Fix curruption from undefined entities
+       #613 #654  Fix case when parsing was suspended while processing nested
+                    entities
   #616 #652 #653  Stop leaking opening tag bindings after a closing tag
                     mismatch error where a parser is reset through
                     XML_ParserReset and then reused to parse
diff --git a/expat/lib/xmlparse.c b/expat/lib/xmlparse.c
index e415068b..aacd6e7f 100644
--- a/expat/lib/xmlparse.c
+++ b/expat/lib/xmlparse.c
@@ -5798,19 +5798,27 @@ internalEntityProcessor(XML_Parser parser, const char *s, const char *end,
 
   if (result != XML_ERROR_NONE)
     return result;
-  else if (textEnd != next
-           && parser->m_parsingStatus.parsing == XML_SUSPENDED) {
+
+  if (textEnd != next && parser->m_parsingStatus.parsing == XML_SUSPENDED) {
     entity->processed = (int)(next - (const char *)entity->textPtr);
     return result;
-  } else {
+  }
+
 #ifdef XML_DTD
-    entityTrackingOnClose(parser, entity, __LINE__);
+  entityTrackingOnClose(parser, entity, __LINE__);
 #endif
-    entity->open = XML_FALSE;
-    parser->m_openInternalEntities = openEntity->next;
-    /* put openEntity back in list of free instances */
-    openEntity->next = parser->m_freeInternalEntities;
-    parser->m_freeInternalEntities = openEntity;
+  entity->open = XML_FALSE;
+  parser->m_openInternalEntities = openEntity->next;
+  /* put openEntity back in list of free instances */
+  openEntity->next = parser->m_freeInternalEntities;
+  parser->m_freeInternalEntities = openEntity;
+
+  // If there are more open entities we want to stop right here and have the
+  // upcoming call to XML_ResumeParser continue with entity content, or it would
+  // be ignored altogether.
+  if (parser->m_openInternalEntities != NULL
+      && parser->m_parsingStatus.parsing == XML_SUSPENDED) {
+    return XML_ERROR_NONE;
   }
 
 #ifdef XML_DTD
diff --git a/expat/tests/runtests.c b/expat/tests/runtests.c
index 7477fa24..245fe9bd 100644
--- a/expat/tests/runtests.c
+++ b/expat/tests/runtests.c
@@ -6788,6 +6788,48 @@ START_TEST(test_pool_integrity_with_unfinished_attr) {
 }
 END_TEST
 
+typedef struct {
+  XML_Parser parser;
+  CharData *storage;
+} ParserPlusStorage;
+
+static void XMLCALL
+accumulate_and_suspend_comment_handler(void *userData, const XML_Char *data) {
+  ParserPlusStorage *const parserPlusStorage = (ParserPlusStorage *)userData;
+  accumulate_comment(parserPlusStorage->storage, data);
+  XML_StopParser(parserPlusStorage->parser, XML_TRUE);
+}
+
+START_TEST(test_nested_entity_suspend) {
+  const char *const text = "<!DOCTYPE a [\n"
+                           "  <!ENTITY e1 '<!--e1-->'>\n"
+                           "  <!ENTITY e2 '<!--e2 head-->&e1;<!--e2 tail-->'>\n"
+                           "  <!ENTITY e3 '<!--e3 head-->&e2;<!--e3 tail-->'>\n"
+                           "]>\n"
+                           "<a><!--start-->&e3;<!--end--></a>";
+  const XML_Char *const expected = XCS("start") XCS("e3 head") XCS("e2 head")
+      XCS("e1") XCS("e2 tail") XCS("e3 tail") XCS("end");
+  CharData storage;
+  XML_Parser parser = XML_ParserCreate(NULL);
+  ParserPlusStorage parserPlusStorage = {parser, &storage};
+
+  CharData_Init(&storage);
+  XML_SetParamEntityParsing(parser, XML_PARAM_ENTITY_PARSING_ALWAYS);
+  XML_SetCommentHandler(parser, accumulate_and_suspend_comment_handler);
+  XML_SetUserData(parser, &parserPlusStorage);
+
+  enum XML_Status status = XML_Parse(parser, text, (int)strlen(text), XML_TRUE);
+  while (status == XML_STATUS_SUSPENDED) {
+    status = XML_ResumeParser(parser);
+  }
+  if (status != XML_STATUS_OK)
+    xml_failure(parser);
+
+  CharData_CheckXMLChars(&storage, expected);
+  XML_ParserFree(parser);
+}
+END_TEST
+
 /*
  * Namespaces tests.
  */
@@ -12247,6 +12289,7 @@ make_suite(void) {
   tcase_add_test(tc_basic, test_empty_element_abort);
   tcase_add_test__ifdef_xml_dtd(tc_basic,
                                 test_pool_integrity_with_unfinished_attr);
+  tcase_add_test(tc_basic, test_nested_entity_suspend);
 
   suite_add_tcase(s, tc_namespace);
   tcase_add_checked_fixture(tc_namespace, namespace_setup, namespace_teardown);