[SkXMLParser] Initial text node support

src/xml/SkXMLParser.cpp

Index: src/xml/SkXMLParser.cpp
diff --git a/src/xml/SkXMLParser.cpp b/src/xml/SkXMLParser.cpp
index 77903cbb056b51daa69707d73aef0837e3662edb..01df87e6ea7210f4e55079bb54db059c39b0c4c0 100644
--- a/src/xml/SkXMLParser.cpp
+++ b/src/xml/SkXMLParser.cpp
@@ -8,6 +8,8 @@
 #include "expat.h"
 
 #include "SkStream.h"
+#include "SkString.h"
+#include "SkTypes.h"
 #include "SkXMLParser.h"
 
 static char const* const gErrorStrings[] = {
@@ -59,18 +61,58 @@ const XML_Memory_Handling_Suite sk_XML_alloc = {
     sk_free
 };
 
+struct ParsingContext {
+    ParsingContext(SkXMLParser* parser)
+        : fParser(parser)
+        , fXMLParser(XML_ParserCreate_MM(nullptr, &sk_XML_alloc, nullptr)) { }
+
+    void flushText() {
+        if (!fBufferedText.isEmpty()) {
+            fParser->text(fBufferedText.c_str(), SkTo<int>(fBufferedText.size()));
+            fBufferedText.reset();
+        }
+    }
+
+    SkXMLParser* fParser;
+    SkAutoTCallVProc<skstd::remove_pointer_t<XML_Parser>, XML_ParserFree> fXMLParser;

[robertphillips, 2016/07/18 15:29:18]

private: ?

[f(malita), 2016/07/18 17:36:32]

Done.

+    SkString fBufferedText;
+};
+
 void XMLCALL start_element_handler(void *data, const char* tag, const char** attributes) {
-    SkXMLParser* parser = static_cast<SkXMLParser*>(data);
+    ParsingContext* ctx = static_cast<ParsingContext*>(data);
+    ctx->flushText();
 
-    parser->startElement(tag);
+    ctx->fParser->startElement(tag);
 
     for (size_t i = 0; attributes[i]; i += 2) {
-        parser->addAttribute(attributes[i], attributes[i + 1]);
+        ctx->fParser->addAttribute(attributes[i], attributes[i + 1]);
     }
 }
 
 void XMLCALL end_element_handler(void* data, const char* tag) {
-    static_cast<SkXMLParser*>(data)->endElement(tag);
+    ParsingContext* ctx = static_cast<ParsingContext*>(data);
+    ctx->flushText();
+    ctx->fParser->endElement(tag);
+}
+
+void XMLCALL text_handler(void *data, const char* txt, int len) {

[robertphillips, 2016/07/18 15:29:18]

Should be have a macro like "HANDLER_PREAMBLE(ctx)" that does this hard cast and
hides the type?

[f(malita), 2016/07/18 17:36:32]

Done.

+    ParsingContext* ctx = static_cast<ParsingContext*>(data);
+    ctx->fBufferedText.append(txt, len);
+}
+
+void XMLCALL entity_decl_handler(void *data,
+                                 const XML_Char *entityName,
+                                 int is_parameter_entity,
+                                 const XML_Char *value,
+                                 int value_length,
+                                 const XML_Char *base,
+                                 const XML_Char *systemId,
+                                 const XML_Char *publicId,
+                                 const XML_Char *notationName) {
+    ParsingContext* ctx = static_cast<ParsingContext*>(data);
+
+    SkDebugf("'%s' entity declaration found, stopping processing", entityName);
+    XML_StopParser(ctx->fXMLParser, XML_FALSE);
 }
 
 } // anonymous namespace
@@ -85,20 +127,23 @@ SkXMLParser::~SkXMLParser()
 
 bool SkXMLParser::parse(SkStream& docStream)
 {
-    SkAutoTCallVProc<skstd::remove_pointer_t<XML_Parser>, XML_ParserFree>
-    parser(XML_ParserCreate_MM(nullptr, &sk_XML_alloc, nullptr));
-    if (!parser) {
+    ParsingContext ctx(this);
+    if (!ctx.fXMLParser) {
         SkDebugf("could not create XML parser\n");
         return false;
     }
 
-    XML_SetUserData(parser, this);
-    XML_SetElementHandler(parser, start_element_handler, end_element_handler);
+    XML_SetUserData(ctx.fXMLParser, &ctx);
+    XML_SetElementHandler(ctx.fXMLParser, start_element_handler, end_element_handler);
+    XML_SetCharacterDataHandler(ctx.fXMLParser, text_handler);
+
+    // Disable entity processing, to inhibit internal entity expansion. See expat CVE-2013-0340.
+    XML_SetEntityDeclHandler(ctx.fXMLParser, entity_decl_handler);
 
     static const int kBufferSize = 512 SkDEBUGCODE( - 507);
     bool done = false;
     do {
-        void* buffer = XML_GetBuffer(parser, kBufferSize);
+        void* buffer = XML_GetBuffer(ctx.fXMLParser, kBufferSize);
         if (!buffer) {
             SkDebugf("could not buffer enough to continue\n");
             return false;
@@ -106,11 +151,11 @@ bool SkXMLParser::parse(SkStream& docStream)
 
         size_t len = docStream.read(buffer, kBufferSize);
         done = docStream.isAtEnd();
-        XML_Status status = XML_ParseBuffer(parser, SkToS32(len), done);
+        XML_Status status = XML_ParseBuffer(ctx.fXMLParser, SkToS32(len), done);
         if (XML_STATUS_ERROR == status) {
-            XML_Error error = XML_GetErrorCode(parser);
-            int line = XML_GetCurrentLineNumber(parser);
-            int column = XML_GetCurrentColumnNumber(parser);
+            XML_Error error = XML_GetErrorCode(ctx.fXMLParser);
+            int line = XML_GetCurrentLineNumber(ctx.fXMLParser);
+            int column = XML_GetCurrentColumnNumber(ctx.fXMLParser);
             const XML_LChar* errorString = XML_ErrorString(error);
             SkDebugf("parse error @%d:%d: %d (%s).\n", line, column, error, errorString);
             return false;