From 2bb6bb5bee97c5431457390ae749a3f9f2de8813 Mon Sep 17 00:00:00 2001 From: Sarat Addepalli Date: Mon, 18 May 2015 09:16:34 +0530 Subject: [PATCH 1/6] Add a test case for issue #332 XML Declarations can occur only at the beginning of an XML Document. Parse() should throw an error, for not well-formed XML Documents. --- xmltest.cpp | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/xmltest.cpp b/xmltest.cpp index b70fc51..597cebb 100644 --- a/xmltest.cpp +++ b/xmltest.cpp @@ -1459,6 +1459,25 @@ int main( int argc, const char ** argv ) doc.LoadFile( "resources/dream.xml" ); XMLTest( "Error should be cleared", false, doc.Error() ); } + + { + // Check that declarations are parsed only as the FirstChild + const char* xml0 = "" + " " + ""; + const char* xml1 = "" + " " + ""; + const char* xml2 = "" + ""; + XMLDocument doc; + doc.Parse(xml0); + XMLTest("Test that the code changes do not affect normal parsing", doc.Error(), false); + doc.Parse(xml1); + XMLTest("Test that the second declaration throws an error", doc.Error(), true); + doc.Parse(xml2); + XMLTest("Test that declaration after a child throws an error", doc.Error(), true); + } // ----------- Performance tracking -------------- { From a0f499dda1ad45306e7f19de45a851c1124837ee Mon Sep 17 00:00:00 2001 From: Sarat Addepalli Date: Mon, 18 May 2015 09:25:17 +0530 Subject: [PATCH 2/6] Fix ParseDeep() to close issue #332 If the node to be added is an XML Declaration, then check if the document has any children already. XML Declarations can only be the FirstChild() s of an XML Document. --- tinyxml2.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tinyxml2.cpp b/tinyxml2.cpp index 6e1985b..a6f1a9f 100755 --- a/tinyxml2.cpp +++ b/tinyxml2.cpp @@ -886,7 +886,18 @@ char* XMLNode::ParseDeep( char* p, StrPair* parentEnd ) } break; } - + + XMLElement* ele = node->ToDeclaration(); + if ( decl ) { + // A declaration can only be the first child of a document. + // Set error, if document already has children. + if ( ! _document->NoChildren() ) { + _document->SetError(XML_ERROR_PARSING_DECLARATION, decl->Value(), 0); + DeleteNode( decl ); + break; + } + } + XMLElement* ele = node->ToElement(); if ( ele ) { // We read the end tag. Return it to the parent. From 2f0d173f94aaa2e54d2c0f8060373b4307d46f0d Mon Sep 17 00:00:00 2001 From: Sarat Addepalli Date: Tue, 19 May 2015 09:02:16 +0530 Subject: [PATCH 3/6] Fix whitespaces in tinyxml2.cpp --- tinyxml2.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tinyxml2.cpp b/tinyxml2.cpp index a6f1a9f..377ad23 100755 --- a/tinyxml2.cpp +++ b/tinyxml2.cpp @@ -886,18 +886,18 @@ char* XMLNode::ParseDeep( char* p, StrPair* parentEnd ) } break; } - + XMLElement* ele = node->ToDeclaration(); if ( decl ) { - // A declaration can only be the first child of a document. - // Set error, if document already has children. - if ( ! _document->NoChildren() ) { - _document->SetError(XML_ERROR_PARSING_DECLARATION, decl->Value(), 0); - DeleteNode( decl ); - break; - } + // A declaration can only be the first child of a document. + // Set error, if document already has children. + if ( !_document->NoChildren() ) { + _document->SetError( XML_ERROR_PARSING_DECLARATION, decl->Value(), 0); + DeleteNode( decl ); + break; + } } - + XMLElement* ele = node->ToElement(); if ( ele ) { // We read the end tag. Return it to the parent. From 8e85afa406b3519f8751a6bbfd31ce0d8a21c168 Mon Sep 17 00:00:00 2001 From: Sarat Addepalli Date: Tue, 19 May 2015 09:07:03 +0530 Subject: [PATCH 4/6] Fix whitespaces in xmltest.cpp --- xmltest.cpp | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/xmltest.cpp b/xmltest.cpp index 597cebb..7a6d866 100644 --- a/xmltest.cpp +++ b/xmltest.cpp @@ -1459,24 +1459,24 @@ int main( int argc, const char ** argv ) doc.LoadFile( "resources/dream.xml" ); XMLTest( "Error should be cleared", false, doc.Error() ); } - + { // Check that declarations are parsed only as the FirstChild - const char* xml0 = "" - " " - ""; - const char* xml1 = "" - " " - ""; - const char* xml2 = "" - ""; - XMLDocument doc; - doc.Parse(xml0); - XMLTest("Test that the code changes do not affect normal parsing", doc.Error(), false); - doc.Parse(xml1); - XMLTest("Test that the second declaration throws an error", doc.Error(), true); - doc.Parse(xml2); - XMLTest("Test that declaration after a child throws an error", doc.Error(), true); + const char* xml0 = "" + " " + ""; + const char* xml1 = "" + " " + ""; + const char* xml2 = "" + ""; + XMLDocument doc; + doc.Parse(xml0); + XMLTest("Test that the code changes do not affect normal parsing", doc.Error(), false); + doc.Parse(xml1); + XMLTest("Test that the second declaration throws an error", doc.Error(), true); + doc.Parse(xml2); + XMLTest("Test that declaration after a child throws an error", doc.Error(), true); } // ----------- Performance tracking -------------- From 3df007ef9d4c2ced820a186eaff6655525e10728 Mon Sep 17 00:00:00 2001 From: Sarat Addepalli Date: Wed, 20 May 2015 10:43:51 +0530 Subject: [PATCH 5/6] Fix and use correct pointers and types. Should have been `XMLDeclaration* decl = ...` instead of `XMLElement* ele = ...` --- tinyxml2.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tinyxml2.cpp b/tinyxml2.cpp index 377ad23..2f6a5b9 100755 --- a/tinyxml2.cpp +++ b/tinyxml2.cpp @@ -887,7 +887,7 @@ char* XMLNode::ParseDeep( char* p, StrPair* parentEnd ) break; } - XMLElement* ele = node->ToDeclaration(); + XMLDeclaration* decl = node->ToDeclaration(); if ( decl ) { // A declaration can only be the first child of a document. // Set error, if document already has children. From 7a93b33160dedbce228b3a35851e520f6ecd84aa Mon Sep 17 00:00:00 2001 From: Lee Thomason Date: Fri, 22 May 2015 11:00:32 -0700 Subject: [PATCH 6/6] tighten up the error checks --- tinyxml2.cpp | 2 +- xmltest.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tinyxml2.cpp b/tinyxml2.cpp index 81296e6..cefd22d 100755 --- a/tinyxml2.cpp +++ b/tinyxml2.cpp @@ -1857,7 +1857,7 @@ XMLError XMLDocument::LoadFile( FILE* fp ) return _errorID; } - if ( filelength >= (size_t)-1 ) { + if ( (size_t)filelength >= (size_t)-1 ) { // Cannot handle files which won't fit in buffer together with null terminator SetError( XML_ERROR_FILE_READ_ERROR, 0, 0 ); return _errorID; diff --git a/xmltest.cpp b/xmltest.cpp index 7a6d866..f3fa7b2 100644 --- a/xmltest.cpp +++ b/xmltest.cpp @@ -1474,9 +1474,9 @@ int main( int argc, const char ** argv ) doc.Parse(xml0); XMLTest("Test that the code changes do not affect normal parsing", doc.Error(), false); doc.Parse(xml1); - XMLTest("Test that the second declaration throws an error", doc.Error(), true); + XMLTest("Test that the second declaration throws an error", doc.ErrorID(), XML_ERROR_PARSING_DECLARATION); doc.Parse(xml2); - XMLTest("Test that declaration after a child throws an error", doc.Error(), true); + XMLTest("Test that declaration after a child throws an error", doc.ErrorID(), XML_ERROR_PARSING_DECLARATION); } // ----------- Performance tracking --------------