diff options
38 files changed, 700 insertions, 618 deletions
@@ -885,6 +885,8 @@ jumbo_static_library("fxcrt") { "core/fxcrt/widestring.h", "core/fxcrt/xml/cfx_xmlchardata.cpp", "core/fxcrt/xml/cfx_xmlchardata.h", + "core/fxcrt/xml/cfx_xmldocument.cpp", + "core/fxcrt/xml/cfx_xmldocument.h", "core/fxcrt/xml/cfx_xmlelement.cpp", "core/fxcrt/xml/cfx_xmlelement.h", "core/fxcrt/xml/cfx_xmlinstruction.cpp", @@ -2890,6 +2892,7 @@ test("pdfium_unittests") { "core/fxcrt/weak_ptr_unittest.cpp", "core/fxcrt/widestring_unittest.cpp", "core/fxcrt/xml/cfx_xmlchardata_unittest.cpp", + "core/fxcrt/xml/cfx_xmldocument_unittest.cpp", "core/fxcrt/xml/cfx_xmlelement_unittest.cpp", "core/fxcrt/xml/cfx_xmlinstruction_unittest.cpp", "core/fxcrt/xml/cfx_xmlnode_unittest.cpp", diff --git a/core/fpdfdoc/cpdf_metadata.cpp b/core/fpdfdoc/cpdf_metadata.cpp index 161fc93edd..323de4ffcf 100644 --- a/core/fpdfdoc/cpdf_metadata.cpp +++ b/core/fpdfdoc/cpdf_metadata.cpp @@ -10,6 +10,7 @@ #include "core/fpdfapi/parser/cpdf_stream_acc.h" #include "core/fxcrt/cfx_memorystream.h" #include "core/fxcrt/fx_codepage.h" +#include "core/fxcrt/xml/cfx_xmldocument.h" #include "core/fxcrt/xml/cfx_xmlelement.h" #include "core/fxcrt/xml/cfx_xmlparser.h" @@ -68,14 +69,14 @@ std::vector<UnsupportedFeature> CPDF_Metadata::CheckForSharedForm() const { auto pAcc = pdfium::MakeRetain<CPDF_StreamAcc>(stream_.Get()); pAcc->LoadAllDataFiltered(); - auto root = pdfium::MakeUnique<CFX_XMLElement>(L"root"); auto stream = pdfium::MakeRetain<CFX_MemoryStream>(pAcc->GetData(), pAcc->GetSize(), false); - CFX_XMLParser parser(root.get(), stream); - if (!parser.Parse()) + CFX_XMLParser parser(stream); + std::unique_ptr<CFX_XMLDocument> doc = parser.Parse(); + if (!doc) return {}; std::vector<UnsupportedFeature> unsupported; - CheckForSharedFormInternal(root.get(), &unsupported); + CheckForSharedFormInternal(doc->GetRoot(), &unsupported); return unsupported; } diff --git a/core/fxcrt/xml/cfx_xmlchardata.cpp b/core/fxcrt/xml/cfx_xmlchardata.cpp index e62a43ac53..307d319537 100644 --- a/core/fxcrt/xml/cfx_xmlchardata.cpp +++ b/core/fxcrt/xml/cfx_xmlchardata.cpp @@ -6,6 +6,7 @@ #include "core/fxcrt/xml/cfx_xmlchardata.h" +#include "core/fxcrt/xml/cfx_xmldocument.h" #include "third_party/base/ptr_util.h" CFX_XMLCharData::CFX_XMLCharData(const WideString& wsCData) @@ -17,8 +18,8 @@ FX_XMLNODETYPE CFX_XMLCharData::GetType() const { return FX_XMLNODE_CharData; } -std::unique_ptr<CFX_XMLNode> CFX_XMLCharData::Clone() { - return pdfium::MakeUnique<CFX_XMLCharData>(GetText()); +CFX_XMLNode* CFX_XMLCharData::Clone(CFX_XMLDocument* doc) { + return doc->CreateNode<CFX_XMLCharData>(GetText()); } void CFX_XMLCharData::Save( diff --git a/core/fxcrt/xml/cfx_xmlchardata.h b/core/fxcrt/xml/cfx_xmlchardata.h index 0cdc348f26..013414bc82 100644 --- a/core/fxcrt/xml/cfx_xmlchardata.h +++ b/core/fxcrt/xml/cfx_xmlchardata.h @@ -12,6 +12,8 @@ #include "core/fxcrt/fx_string.h" #include "core/fxcrt/xml/cfx_xmltext.h" +class CFX_XMLDocument; + class CFX_XMLCharData : public CFX_XMLText { public: explicit CFX_XMLCharData(const WideString& wsCData); @@ -19,7 +21,7 @@ class CFX_XMLCharData : public CFX_XMLText { // CFX_XMLNode FX_XMLNODETYPE GetType() const override; - std::unique_ptr<CFX_XMLNode> Clone() override; + CFX_XMLNode* Clone(CFX_XMLDocument* doc) override; void Save(const RetainPtr<IFX_SeekableWriteStream>& pXMLStream) override; }; diff --git a/core/fxcrt/xml/cfx_xmlchardata_unittest.cpp b/core/fxcrt/xml/cfx_xmlchardata_unittest.cpp index 0fa48e520f..4f0c49eaac 100644 --- a/core/fxcrt/xml/cfx_xmlchardata_unittest.cpp +++ b/core/fxcrt/xml/cfx_xmlchardata_unittest.cpp @@ -3,6 +3,7 @@ // found in the LICENSE file. #include "core/fxcrt/xml/cfx_xmlchardata.h" +#include "core/fxcrt/xml/cfx_xmldocument.h" #include "testing/gtest/include/gtest/gtest.h" #include "testing/string_write_stream.h" #include "testing/test_support.h" @@ -18,12 +19,14 @@ TEST(CFX_XMLCharDataTest, GetText) { } TEST(CFX_XMLCharDataTest, Clone) { + CFX_XMLDocument doc; + CFX_XMLCharData data(L"My Data"); - auto clone = data.Clone(); + CFX_XMLNode* clone = data.Clone(&doc); EXPECT_TRUE(clone != nullptr); - EXPECT_NE(&data, clone.get()); + EXPECT_NE(&data, clone); ASSERT_EQ(FX_XMLNODE_CharData, clone->GetType()); - EXPECT_EQ(L"My Data", static_cast<CFX_XMLCharData*>(clone.get())->GetText()); + EXPECT_EQ(L"My Data", static_cast<CFX_XMLCharData*>(clone)->GetText()); } TEST(CFX_XMLCharDataTest, Save) { diff --git a/core/fxcrt/xml/cfx_xmldocument.cpp b/core/fxcrt/xml/cfx_xmldocument.cpp new file mode 100644 index 0000000000..f6e55551ed --- /dev/null +++ b/core/fxcrt/xml/cfx_xmldocument.cpp @@ -0,0 +1,14 @@ +// Copyright 2018 PDFium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "core/fxcrt/xml/cfx_xmldocument.h" +#include "core/fxcrt/fx_system.h" +#include "core/fxcrt/xml/cfx_xmlinstruction.h" +#include "third_party/base/ptr_util.h" + +CFX_XMLDocument::CFX_XMLDocument() { + root_ = CreateNode<CFX_XMLElement>(L"root"); +} + +CFX_XMLDocument::~CFX_XMLDocument() = default; diff --git a/core/fxcrt/xml/cfx_xmldocument.h b/core/fxcrt/xml/cfx_xmldocument.h new file mode 100644 index 0000000000..a568f8390f --- /dev/null +++ b/core/fxcrt/xml/cfx_xmldocument.h @@ -0,0 +1,37 @@ +// Copyright 2018 PDFium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CORE_FXCRT_XML_CFX_XMLDOCUMENT_H_ +#define CORE_FXCRT_XML_CFX_XMLDOCUMENT_H_ + +#include <memory> +#include <utility> +#include <vector> + +#include "core/fxcrt/unowned_ptr.h" +#include "core/fxcrt/xml/cfx_xmlelement.h" +#include "third_party/base/ptr_util.h" + +class CFX_XMLInstruction; +class CFX_XMLNode; + +class CFX_XMLDocument { + public: + CFX_XMLDocument(); + ~CFX_XMLDocument(); + + CFX_XMLElement* GetRoot() const { return root_.Get(); } + + template <typename T, typename... Args> + T* CreateNode(Args&&... args) { + nodes_.push_back(pdfium::MakeUnique<T>(std::forward<Args>(args)...)); + return static_cast<T*>(nodes_.back().get()); + } + + private: + std::vector<std::unique_ptr<CFX_XMLNode>> nodes_; + UnownedPtr<CFX_XMLElement> root_; +}; + +#endif // CORE_FXCRT_XML_CFX_XMLDOCUMENT_H_ diff --git a/core/fxcrt/xml/cfx_xmldocument_unittest.cpp b/core/fxcrt/xml/cfx_xmldocument_unittest.cpp new file mode 100644 index 0000000000..486b040db5 --- /dev/null +++ b/core/fxcrt/xml/cfx_xmldocument_unittest.cpp @@ -0,0 +1,22 @@ +// Copyright 2018 PDFium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "core/fxcrt/xml/cfx_xmldocument.h" +#include "core/fxcrt/xml/cfx_xmlelement.h" +#include "core/fxcrt/xml/cfx_xmlinstruction.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "testing/test_support.h" + +TEST(CFX_XMLDocumentTest, Root) { + CFX_XMLDocument doc; + EXPECT_TRUE(doc.GetRoot() != nullptr); +} + +TEST(CFX_XMLDocumentTest, CreateNode) { + CFX_XMLDocument doc; + auto* node = doc.CreateNode<CFX_XMLElement>(L"elem"); + + ASSERT_EQ(FX_XMLNODE_Element, node->GetType()); + EXPECT_EQ(L"elem", node->GetName()); +} diff --git a/core/fxcrt/xml/cfx_xmlelement.cpp b/core/fxcrt/xml/cfx_xmlelement.cpp index 3befb242c9..74351b8c58 100644 --- a/core/fxcrt/xml/cfx_xmlelement.cpp +++ b/core/fxcrt/xml/cfx_xmlelement.cpp @@ -11,6 +11,7 @@ #include "core/fxcrt/cfx_widetextbuf.h" #include "core/fxcrt/fx_extension.h" #include "core/fxcrt/xml/cfx_xmlchardata.h" +#include "core/fxcrt/xml/cfx_xmldocument.h" #include "core/fxcrt/xml/cfx_xmltext.h" #include "third_party/base/ptr_util.h" #include "third_party/base/stl_util.h" @@ -26,20 +27,18 @@ FX_XMLNODETYPE CFX_XMLElement::GetType() const { return FX_XMLNODE_Element; } -std::unique_ptr<CFX_XMLNode> CFX_XMLElement::Clone() { - auto pClone = pdfium::MakeUnique<CFX_XMLElement>(name_); - pClone->attrs_ = attrs_; +CFX_XMLNode* CFX_XMLElement::Clone(CFX_XMLDocument* doc) { + auto* node = doc->CreateNode<CFX_XMLElement>(name_); + node->attrs_ = attrs_; - // TODO(dsinclair): This clone is wrong. It doesn't clone child nodes just - // copies text nodes? - WideString wsText; + // TODO(dsinclair): This clone is wrong. It doesn't clone all child nodes just + // text nodes? for (CFX_XMLNode* pChild = GetFirstChild(); pChild; pChild = pChild->GetNextSibling()) { if (pChild->GetType() == FX_XMLNODE_Text) - wsText += static_cast<CFX_XMLText*>(pChild)->GetText(); + node->AppendChild(pChild->Clone(doc)); } - pClone->SetTextData(wsText); - return std::move(pClone); + return node; } WideString CFX_XMLElement::GetLocalTagName() const { @@ -60,7 +59,6 @@ WideString CFX_XMLElement::GetNamespaceURI() const { attr += L":"; attr += wsPrefix; } - const CFX_XMLNode* pNode = this; while (pNode) { if (pNode->GetType() != FX_XMLNODE_Element) @@ -89,13 +87,6 @@ WideString CFX_XMLElement::GetTextData() const { return buffer.MakeString(); } -void CFX_XMLElement::SetTextData(const WideString& wsText) { - // TODO(dsinclair): Shouldn't this remove the children if you set blank text? - if (wsText.IsEmpty()) - return; - AppendChild(pdfium::MakeUnique<CFX_XMLText>(wsText)); -} - void CFX_XMLElement::Save( const RetainPtr<IFX_SeekableWriteStream>& pXMLStream) { ByteString bsNameEncoded = name_.UTF8Encode(); diff --git a/core/fxcrt/xml/cfx_xmlelement.h b/core/fxcrt/xml/cfx_xmlelement.h index 678ba6d996..ca5fd5e797 100644 --- a/core/fxcrt/xml/cfx_xmlelement.h +++ b/core/fxcrt/xml/cfx_xmlelement.h @@ -14,6 +14,8 @@ #include "core/fxcrt/fx_string.h" #include "core/fxcrt/xml/cfx_xmlnode.h" +class CFX_XMLDocument; + class CFX_XMLElement : public CFX_XMLNode { public: explicit CFX_XMLElement(const WideString& wsTag); @@ -21,7 +23,7 @@ class CFX_XMLElement : public CFX_XMLNode { // CFX_XMLNode FX_XMLNODETYPE GetType() const override; - std::unique_ptr<CFX_XMLNode> Clone() override; + CFX_XMLNode* Clone(CFX_XMLDocument* doc) override; void Save(const RetainPtr<IFX_SeekableWriteStream>& pXMLStream) override; WideString GetName() const { return name_; } @@ -44,7 +46,6 @@ class CFX_XMLElement : public CFX_XMLNode { WideString GetNamespaceURI() const; WideString GetTextData() const; - void SetTextData(const WideString& wsText); private: WideString AttributeToString(const WideString& name, const WideString& value); diff --git a/core/fxcrt/xml/cfx_xmlelement_unittest.cpp b/core/fxcrt/xml/cfx_xmlelement_unittest.cpp index 79e067f511..1e53ef3dd7 100644 --- a/core/fxcrt/xml/cfx_xmlelement_unittest.cpp +++ b/core/fxcrt/xml/cfx_xmlelement_unittest.cpp @@ -2,15 +2,13 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include <utility> - -#include "core/fxcrt/xml/cfx_xmlchardata.h" #include "core/fxcrt/xml/cfx_xmlelement.h" +#include "core/fxcrt/xml/cfx_xmlchardata.h" +#include "core/fxcrt/xml/cfx_xmldocument.h" #include "core/fxcrt/xml/cfx_xmltext.h" #include "testing/gtest/include/gtest/gtest.h" #include "testing/string_write_stream.h" #include "testing/test_support.h" -#include "third_party/base/ptr_util.h" TEST(CFX_XMLElementTest, GetType) { CFX_XMLElement node(L"node"); @@ -74,19 +72,24 @@ TEST(CFX_XMLElementTest, Attributes) { } TEST(CFX_XMLElementTest, Clone) { + CFX_XMLDocument doc; + CFX_XMLElement node(L"test:node"); node.SetAttribute(L"first", L"one"); node.SetAttribute(L"second", L"two"); node.SetAttribute(L"xmlns:test", L"https://example.org/test"); - node.AppendChild(pdfium::MakeUnique<CFX_XMLText>(L"Text Child")); - node.AppendChild(pdfium::MakeUnique<CFX_XMLElement>(L"Node child")); + CFX_XMLText text_child1(L"Text Child"); + node.AppendChild(&text_child1); - auto clone = node.Clone(); + CFX_XMLElement node_child1(L"Node child"); + node.AppendChild(&node_child1); + + CFX_XMLNode* clone = node.Clone(&doc); EXPECT_TRUE(clone != nullptr); ASSERT_EQ(FX_XMLNODE_Element, clone->GetType()); - CFX_XMLElement* inst = static_cast<CFX_XMLElement*>(clone.get()); + CFX_XMLElement* inst = static_cast<CFX_XMLElement*>(clone); EXPECT_EQ(L"test:node", inst->GetName()); EXPECT_EQ(L"node", inst->GetLocalTagName()); @@ -129,14 +132,17 @@ TEST(CFX_XMLElementTest, SaveWithChildren) { auto stream = pdfium::MakeRetain<StringWriteStream>(); CFX_XMLElement node(L"node"); - node.AppendChild(pdfium::MakeUnique<CFX_XMLText>(L"Text Child 1")); + CFX_XMLText text_child1(L"Text Child 1"); + node.AppendChild(&text_child1); + + CFX_XMLElement node_child1(L"node-child"); + node.AppendChild(&node_child1); - auto child = pdfium::MakeUnique<CFX_XMLElement>(L"node-child"); - CFX_XMLElement* node_child1 = child.get(); - node.AppendChild(std::move(child)); + CFX_XMLText text_child2(L"Text Child 2"); + node_child1.AppendChild(&text_child2); - node_child1->AppendChild(pdfium::MakeUnique<CFX_XMLText>(L"Text Child 2")); - node.AppendChild(pdfium::MakeUnique<CFX_XMLCharData>(L"Char Data")); + CFX_XMLCharData char_data1(L"Char Data"); + node.AppendChild(&char_data1); node.Save(stream); EXPECT_EQ( @@ -160,18 +166,18 @@ TEST(CFX_XMLElementTest, SaveWithNamespace) { TEST(CFX_XMLElementTest, GetFirstChildNamed) { CFX_XMLElement node(L"node"); - auto child = pdfium::MakeUnique<CFX_XMLElement>(L"node-child"); - CFX_XMLElement* node_child1 = child.get(); - node.AppendChild(std::move(child)); + CFX_XMLElement node_child1(L"node-child"); + node.AppendChild(&node_child1); auto* found = node.GetFirstChildNamed(L"node-child"); EXPECT_TRUE(found != nullptr); - EXPECT_EQ(node_child1, found); + EXPECT_EQ(&node_child1, found); } TEST(CFX_XMLElementTest, GetFirstChildNamedMissing) { CFX_XMLElement node(L"node"); - node.AppendChild(pdfium::MakeUnique<CFX_XMLElement>(L"node-child")); + CFX_XMLElement node_child1(L"node-child"); + node.AppendChild(&node_child1); auto* found = node.GetFirstChildNamed(L"node-sibling"); EXPECT_TRUE(found == nullptr); @@ -179,23 +185,26 @@ TEST(CFX_XMLElementTest, GetFirstChildNamedMissing) { TEST(CFX_XMLElementTest, GetNthChildNamed) { CFX_XMLElement node(L"node"); - node.AppendChild(pdfium::MakeUnique<CFX_XMLElement>(L"node-child")); - node.AppendChild(pdfium::MakeUnique<CFX_XMLElement>(L"node-child")); - - auto child = pdfium::MakeUnique<CFX_XMLElement>(L"node-child"); - CFX_XMLElement* node_child3 = child.get(); - node.AppendChild(std::move(child)); + CFX_XMLElement node_child1(L"node-child"); + CFX_XMLElement node_child2(L"node-child"); + CFX_XMLElement node_child3(L"node-child"); + node.AppendChild(&node_child1); + node.AppendChild(&node_child2); + node.AppendChild(&node_child3); auto* found = node.GetNthChildNamed(L"node-child", 2); EXPECT_TRUE(found != nullptr); - EXPECT_EQ(node_child3, found); + EXPECT_EQ(&node_child3, found); } TEST(CFX_XMLElementTest, GetNthChildNamedMissingChild) { CFX_XMLElement node(L"node"); - node.AppendChild(pdfium::MakeUnique<CFX_XMLElement>(L"node-child")); - node.AppendChild(pdfium::MakeUnique<CFX_XMLElement>(L"node-child")); - node.AppendChild(pdfium::MakeUnique<CFX_XMLElement>(L"node-child")); + CFX_XMLElement node_child1(L"node-child"); + CFX_XMLElement node_child2(L"node-child"); + CFX_XMLElement node_child3(L"node-child"); + node.AppendChild(&node_child1); + node.AppendChild(&node_child2); + node.AppendChild(&node_child3); auto* found = node.GetNthChildNamed(L"node-child", 5); EXPECT_TRUE(found == nullptr); @@ -203,15 +212,18 @@ TEST(CFX_XMLElementTest, GetNthChildNamedMissingChild) { TEST(CFX_XMLElementTest, GetTextData) { CFX_XMLElement node(L"node"); - node.AppendChild(pdfium::MakeUnique<CFX_XMLText>(L"Text Child 1")); - auto child = pdfium::MakeUnique<CFX_XMLElement>(L"Node child"); - CFX_XMLElement* node_child1 = child.get(); - node.AppendChild(std::move(child)); + CFX_XMLText text_child1(L"Text Child 1"); + node.AppendChild(&text_child1); + + CFX_XMLElement node_child1(L"Node child"); + node.AppendChild(&node_child1); - node_child1->AppendChild(pdfium::MakeUnique<CFX_XMLText>(L"Text Child 2")); + CFX_XMLText text_child2(L"Text Child 2"); + node_child1.AppendChild(&text_child2); - node.AppendChild(pdfium::MakeUnique<CFX_XMLCharData>(L"Char Data")); + CFX_XMLCharData char_data1(L"Char Data"); + node.AppendChild(&char_data1); EXPECT_EQ(L"Text Child 1Char Data", node.GetTextData()); } diff --git a/core/fxcrt/xml/cfx_xmlinstruction.cpp b/core/fxcrt/xml/cfx_xmlinstruction.cpp index b7a87b588a..32841fb68e 100644 --- a/core/fxcrt/xml/cfx_xmlinstruction.cpp +++ b/core/fxcrt/xml/cfx_xmlinstruction.cpp @@ -10,6 +10,7 @@ #include "core/fxcrt/fx_codepage.h" #include "core/fxcrt/fx_extension.h" +#include "core/fxcrt/xml/cfx_xmldocument.h" #include "third_party/base/ptr_util.h" #include "third_party/base/stl_util.h" @@ -22,10 +23,10 @@ FX_XMLNODETYPE CFX_XMLInstruction::GetType() const { return FX_XMLNODE_Instruction; } -std::unique_ptr<CFX_XMLNode> CFX_XMLInstruction::Clone() { - auto pClone = pdfium::MakeUnique<CFX_XMLInstruction>(name_); - pClone->m_TargetData = m_TargetData; - return std::move(pClone); +CFX_XMLNode* CFX_XMLInstruction::Clone(CFX_XMLDocument* doc) { + auto* node = doc->CreateNode<CFX_XMLInstruction>(name_); + node->m_TargetData = m_TargetData; + return node; } void CFX_XMLInstruction::AppendData(const WideString& wsData) { diff --git a/core/fxcrt/xml/cfx_xmlinstruction.h b/core/fxcrt/xml/cfx_xmlinstruction.h index 3be9d48573..153ef2b625 100644 --- a/core/fxcrt/xml/cfx_xmlinstruction.h +++ b/core/fxcrt/xml/cfx_xmlinstruction.h @@ -13,6 +13,8 @@ #include "core/fxcrt/fx_string.h" #include "core/fxcrt/xml/cfx_xmlnode.h" +class CFX_XMLDocument; + class CFX_XMLInstruction : public CFX_XMLNode { public: explicit CFX_XMLInstruction(const WideString& wsTarget); @@ -20,7 +22,7 @@ class CFX_XMLInstruction : public CFX_XMLNode { // CFX_XMLNode FX_XMLNODETYPE GetType() const override; - std::unique_ptr<CFX_XMLNode> Clone() override; + CFX_XMLNode* Clone(CFX_XMLDocument* doc) override; void Save(const RetainPtr<IFX_SeekableWriteStream>& pXMLStream) override; bool IsOriginalXFAVersion() const; diff --git a/core/fxcrt/xml/cfx_xmlinstruction_unittest.cpp b/core/fxcrt/xml/cfx_xmlinstruction_unittest.cpp index c60d401303..0c43c59ccd 100644 --- a/core/fxcrt/xml/cfx_xmlinstruction_unittest.cpp +++ b/core/fxcrt/xml/cfx_xmlinstruction_unittest.cpp @@ -4,6 +4,7 @@ #include "core/fxcrt/xml/cfx_xmlinstruction.h" #include "core/fxcrt/cfx_memorystream.h" +#include "core/fxcrt/xml/cfx_xmldocument.h" #include "core/fxcrt/xml/cfx_xmlelement.h" #include "core/fxcrt/xml/cfx_xmlparser.h" #include "testing/gtest/include/gtest/gtest.h" @@ -41,15 +42,17 @@ TEST(CFX_XMLInstructionTest, TargetData) { } TEST(CFX_XMLInstructionTest, Clone) { + CFX_XMLDocument doc; + CFX_XMLInstruction node(L"acrobat"); node.AppendData(L"firstString"); node.AppendData(L"secondString"); - auto clone = node.Clone(); + CFX_XMLNode* clone = node.Clone(&doc); EXPECT_TRUE(clone != nullptr); ASSERT_EQ(FX_XMLNODE_Instruction, clone->GetType()); - CFX_XMLInstruction* inst = static_cast<CFX_XMLInstruction*>(clone.get()); + CFX_XMLInstruction* inst = static_cast<CFX_XMLInstruction*>(clone); EXPECT_TRUE(inst->IsAcrobat()); @@ -87,14 +90,16 @@ TEST(CFX_XMLInstructionTest, ParseAndReSave) { reinterpret_cast<uint8_t*>(const_cast<char*>(input)), strlen(input), false); - CFX_XMLElement root(L"root"); - CFX_XMLParser parser(&root, in_stream); - ASSERT_TRUE(parser.Parse()); - ASSERT_TRUE(root.GetFirstChild() != nullptr); - ASSERT_EQ(FX_XMLNODE_Instruction, root.GetFirstChild()->GetType()); + CFX_XMLParser parser(in_stream); + std::unique_ptr<CFX_XMLDocument> doc = parser.Parse(); + ASSERT_TRUE(doc != nullptr); + + CFX_XMLElement* root = doc->GetRoot(); + ASSERT_TRUE(root->GetFirstChild() != nullptr); + ASSERT_EQ(FX_XMLNODE_Instruction, root->GetFirstChild()->GetType()); CFX_XMLInstruction* node = - static_cast<CFX_XMLInstruction*>(root.GetFirstChild()); + static_cast<CFX_XMLInstruction*>(root->GetFirstChild()); ASSERT_TRUE(node != nullptr); EXPECT_TRUE(node->IsAcrobat()); @@ -109,3 +114,49 @@ TEST(CFX_XMLInstructionTest, ParseAndReSave) { "<?acrobat http://www.xfa.org/schema/xfa-template/3.3/ Display:1 ?>\n", out_stream->ToString()); } + +TEST(CFX_XMLInstructionTest, ParseAndReSaveInnerInstruction) { + const char* input = + "<node>\n" + "<?acrobat http://www.xfa.org/schema/xfa-template/3.3/ Display:1 ?>\n" + "</node>"; + + auto in_stream = pdfium::MakeRetain<CFX_MemoryStream>( + reinterpret_cast<uint8_t*>(const_cast<char*>(input)), strlen(input), + false); + + CFX_XMLParser parser(in_stream); + std::unique_ptr<CFX_XMLDocument> doc = parser.Parse(); + ASSERT_TRUE(doc != nullptr); + + CFX_XMLElement* root = doc->GetRoot(); + ASSERT_TRUE(root->GetFirstChild() != nullptr); + ASSERT_TRUE(root->GetFirstChild()->GetType() == FX_XMLNODE_Element); + + CFX_XMLElement* node = static_cast<CFX_XMLElement*>(root->GetFirstChild()); + EXPECT_EQ(L"node", node->GetName()); + + CFX_XMLInstruction* instruction = nullptr; + for (auto* elem = node->GetFirstChild(); elem; + elem = elem->GetNextSibling()) { + if (elem->GetType() == FX_XMLNODE_Instruction) { + instruction = static_cast<CFX_XMLInstruction*>(elem); + break; + } + } + ASSERT_TRUE(instruction != nullptr); + EXPECT_TRUE(instruction->IsAcrobat()); + + auto& data = instruction->GetTargetData(); + ASSERT_EQ(2U, data.size()); + EXPECT_EQ(L"http://www.xfa.org/schema/xfa-template/3.3/", data[0]); + EXPECT_EQ(L"Display:1", data[1]); + + auto out_stream = pdfium::MakeRetain<StringWriteStream>(); + node->Save(out_stream); + EXPECT_EQ( + "<node>\n\n" + "<?acrobat http://www.xfa.org/schema/xfa-template/3.3/ Display:1 ?>\n\n" + "</node>\n", + out_stream->ToString()); +} diff --git a/core/fxcrt/xml/cfx_xmlnode.cpp b/core/fxcrt/xml/cfx_xmlnode.cpp index 088cbf367c..c8fea23bda 100644 --- a/core/fxcrt/xml/cfx_xmlnode.cpp +++ b/core/fxcrt/xml/cfx_xmlnode.cpp @@ -17,38 +17,26 @@ CFX_XMLNode::CFX_XMLNode() = default; -CFX_XMLNode::~CFX_XMLNode() { - DeleteChildren(); -} +CFX_XMLNode::~CFX_XMLNode() = default; void CFX_XMLNode::DeleteChildren() { - CFX_XMLNode* child = last_child_.Get(); - // Clear last child early as it will have been deleted already. - last_child_ = nullptr; - while (child) { - child = child->prev_sibling_.Get(); - if (child) { - if (child->next_sibling_) { - child->next_sibling_->prev_sibling_ = nullptr; - child->next_sibling_->parent_ = nullptr; - } - - child->next_sibling_ = nullptr; - } - } - if (first_child_) { - first_child_->next_sibling_ = nullptr; + while (first_child_) { first_child_->parent_ = nullptr; + first_child_->prev_sibling_ = nullptr; + + CFX_XMLNode* child = first_child_; + first_child_ = child->next_sibling_; + + child->next_sibling_ = nullptr; } - first_child_ = nullptr; + last_child_ = nullptr; } -void CFX_XMLNode::AppendChild(std::unique_ptr<CFX_XMLNode> pNode) { - InsertChildNode(std::move(pNode), -1); +void CFX_XMLNode::AppendChild(CFX_XMLNode* pNode) { + InsertChildNode(pNode, -1); } -void CFX_XMLNode::InsertChildNode(std::unique_ptr<CFX_XMLNode> pNode, - int32_t index) { +void CFX_XMLNode::InsertChildNode(CFX_XMLNode* pNode, int32_t index) { ASSERT(!pNode->parent_); pNode->parent_ = this; @@ -56,36 +44,36 @@ void CFX_XMLNode::InsertChildNode(std::unique_ptr<CFX_XMLNode> pNode, if (!first_child_) { ASSERT(!last_child_); - first_child_ = std::move(pNode); - last_child_ = first_child_.get(); + first_child_ = pNode; + last_child_ = first_child_; first_child_->prev_sibling_ = nullptr; first_child_->next_sibling_ = nullptr; return; } if (index == 0) { - first_child_->prev_sibling_ = pNode.get(); - pNode->next_sibling_ = std::move(first_child_); + first_child_->prev_sibling_ = pNode; + pNode->next_sibling_ = first_child_; pNode->prev_sibling_ = nullptr; - first_child_ = std::move(pNode); + first_child_ = pNode; return; } int32_t iCount = 0; - CFX_XMLNode* pFind = first_child_.get(); + CFX_XMLNode* pFind = first_child_; // Note, negative indexes, and indexes after the end of the list will result // in appending to the list. while (++iCount != index && pFind->next_sibling_) - pFind = pFind->next_sibling_.get(); + pFind = pFind->next_sibling_; pNode->prev_sibling_ = pFind; if (pFind->next_sibling_) - pFind->next_sibling_->prev_sibling_ = pNode.get(); - pNode->next_sibling_ = std::move(pFind->next_sibling_); + pFind->next_sibling_->prev_sibling_ = pNode; + pNode->next_sibling_ = pFind->next_sibling_; - pFind->next_sibling_ = std::move(pNode); - if (pFind == last_child_.Get()) - last_child_ = pFind->next_sibling_.get(); + pFind->next_sibling_ = pNode; + if (pFind == last_child_) + last_child_ = pFind->next_sibling_; } void CFX_XMLNode::RemoveChildNode(CFX_XMLNode* pNode) { @@ -95,35 +83,25 @@ void CFX_XMLNode::RemoveChildNode(CFX_XMLNode* pNode) { if (pNode->GetParent() != this) return; - if (first_child_.get() == pNode) { - first_child_.release(); - first_child_ = std::move(pNode->next_sibling_); - if (first_child_) { - first_child_->prev_sibling_ = nullptr; - - if (first_child_->next_sibling_) - first_child_->next_sibling_->prev_sibling_ = first_child_.get(); - } - } else { - CFX_XMLNode* prev = pNode->prev_sibling_.Get(); - prev->next_sibling_.release(); // Release pNode - prev->next_sibling_ = std::move(pNode->next_sibling_); - - if (prev->next_sibling_) - prev->next_sibling_->prev_sibling_ = prev; - } + if (first_child_ == pNode) + first_child_ = pNode->next_sibling_; + if (last_child_ == pNode) + last_child_ = pNode->prev_sibling_; - if (last_child_.Get() == pNode) - last_child_ = pNode->prev_sibling_.Get(); + if (pNode->prev_sibling_) + pNode->prev_sibling_->next_sibling_ = pNode->next_sibling_; + if (pNode->next_sibling_) + pNode->next_sibling_->prev_sibling_ = pNode->prev_sibling_; pNode->parent_ = nullptr; pNode->prev_sibling_ = nullptr; + pNode->next_sibling_ = nullptr; } CFX_XMLNode* CFX_XMLNode::GetRoot() { CFX_XMLNode* pParent = this; while (pParent->parent_) - pParent = pParent->parent_.Get(); + pParent = pParent->parent_; return pParent; } diff --git a/core/fxcrt/xml/cfx_xmlnode.h b/core/fxcrt/xml/cfx_xmlnode.h index 4c98a3b51a..95dc0acce9 100644 --- a/core/fxcrt/xml/cfx_xmlnode.h +++ b/core/fxcrt/xml/cfx_xmlnode.h @@ -19,39 +19,41 @@ enum FX_XMLNODETYPE { FX_XMLNODE_CharData, }; +class CFX_XMLDocument; + class CFX_XMLNode { public: CFX_XMLNode(); virtual ~CFX_XMLNode(); virtual FX_XMLNODETYPE GetType() const = 0; - virtual std::unique_ptr<CFX_XMLNode> Clone() = 0; + virtual CFX_XMLNode* Clone(CFX_XMLDocument* doc) = 0; virtual void Save(const RetainPtr<IFX_SeekableWriteStream>& pXMLStream) = 0; CFX_XMLNode* GetRoot(); - CFX_XMLNode* GetParent() const { return parent_.Get(); } - CFX_XMLNode* GetFirstChild() const { return first_child_.get(); } - CFX_XMLNode* GetNextSibling() const { return next_sibling_.get(); } + CFX_XMLNode* GetParent() const { return parent_; } + CFX_XMLNode* GetFirstChild() const { return first_child_; } + CFX_XMLNode* GetNextSibling() const { return next_sibling_; } - void AppendChild(std::unique_ptr<CFX_XMLNode> pNode); - void InsertChildNode(std::unique_ptr<CFX_XMLNode> pNode, int32_t index); + void AppendChild(CFX_XMLNode* pNode); + void InsertChildNode(CFX_XMLNode* pNode, int32_t index); void RemoveChildNode(CFX_XMLNode* pNode); void DeleteChildren(); - CFX_XMLNode* GetLastChildForTesting() const { return last_child_.Get(); } - CFX_XMLNode* GetPrevSiblingForTesting() const { return prev_sibling_.Get(); } + CFX_XMLNode* GetLastChildForTesting() const { return last_child_; } + CFX_XMLNode* GetPrevSiblingForTesting() const { return prev_sibling_; } protected: WideString EncodeEntities(const WideString& value); private: - // A node owns its first child and it owns its next sibling. The rest - // are unowned pointers. - UnownedPtr<CFX_XMLNode> parent_; - UnownedPtr<CFX_XMLNode> last_child_; - UnownedPtr<CFX_XMLNode> prev_sibling_; - std::unique_ptr<CFX_XMLNode> first_child_; - std::unique_ptr<CFX_XMLNode> next_sibling_; + // The nodes are owned by the XML document. We do not know what order the + // nodes will be destroyed in so they can not be UnownedPtrs. + CFX_XMLNode* parent_ = nullptr; + CFX_XMLNode* first_child_ = nullptr; + CFX_XMLNode* last_child_ = nullptr; + CFX_XMLNode* next_sibling_ = nullptr; + CFX_XMLNode* prev_sibling_ = nullptr; }; #endif // CORE_FXCRT_XML_CFX_XMLNODE_H_ diff --git a/core/fxcrt/xml/cfx_xmlnode_unittest.cpp b/core/fxcrt/xml/cfx_xmlnode_unittest.cpp index 1c69069942..ec56603a58 100644 --- a/core/fxcrt/xml/cfx_xmlnode_unittest.cpp +++ b/core/fxcrt/xml/cfx_xmlnode_unittest.cpp @@ -2,274 +2,227 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include <utility> - #include "core/fxcrt/xml/cfx_xmlelement.h" #include "testing/gtest/include/gtest/gtest.h" #include "testing/test_support.h" -#include "third_party/base/ptr_util.h" TEST(CFX_XMLNodeTest, GetParent) { - auto node1 = pdfium::MakeUnique<CFX_XMLElement>(L"node"); - auto child2 = pdfium::MakeUnique<CFX_XMLElement>(L"node2"); - auto child3 = pdfium::MakeUnique<CFX_XMLElement>(L"node3"); - - CFX_XMLElement* node2 = child2.get(); - CFX_XMLElement* node3 = child3.get(); + CFX_XMLElement node1(L"node"); + CFX_XMLElement node2(L"node2"); + CFX_XMLElement node3(L"node3"); - node1->AppendChild(std::move(child2)); - node2->AppendChild(std::move(child3)); + node1.AppendChild(&node2); + node2.AppendChild(&node3); - EXPECT_EQ(nullptr, node1->GetParent()); - EXPECT_EQ(node1.get(), node2->GetParent()); - EXPECT_EQ(node2, node3->GetParent()); + EXPECT_EQ(nullptr, node1.GetParent()); + EXPECT_EQ(&node1, node2.GetParent()); + EXPECT_EQ(&node2, node3.GetParent()); } TEST(CFX_XMLNodeTest, GetRoot) { - auto node1 = pdfium::MakeUnique<CFX_XMLElement>(L"node"); - auto child2 = pdfium::MakeUnique<CFX_XMLElement>(L"node2"); - auto child3 = pdfium::MakeUnique<CFX_XMLElement>(L"node3"); + CFX_XMLElement node1(L"node"); + CFX_XMLElement node2(L"node2"); + CFX_XMLElement node3(L"node3"); - CFX_XMLElement* node2 = child2.get(); - CFX_XMLElement* node3 = child3.get(); + node1.AppendChild(&node2); + node2.AppendChild(&node3); - node1->AppendChild(std::move(child2)); - node2->AppendChild(std::move(child3)); - - EXPECT_EQ(node1.get(), node1->GetRoot()); - EXPECT_EQ(node1.get(), node2->GetRoot()); - EXPECT_EQ(node1.get(), node3->GetRoot()); + EXPECT_EQ(&node1, node1.GetRoot()); + EXPECT_EQ(&node1, node2.GetRoot()); + EXPECT_EQ(&node1, node3.GetRoot()); } TEST(CFX_XMLNodeTest, GetChildren) { - auto node1 = pdfium::MakeUnique<CFX_XMLElement>(L"node"); - auto child2 = pdfium::MakeUnique<CFX_XMLElement>(L"node2"); - auto child3 = pdfium::MakeUnique<CFX_XMLElement>(L"node3"); - auto child4 = pdfium::MakeUnique<CFX_XMLElement>(L"node4"); - - CFX_XMLElement* node2 = child2.get(); - CFX_XMLElement* node3 = child3.get(); - CFX_XMLElement* node4 = child4.get(); + CFX_XMLElement node1(L"node"); + CFX_XMLElement node2(L"node2"); + CFX_XMLElement node3(L"node3"); + CFX_XMLElement node4(L"node4"); - node1->AppendChild(std::move(child2)); - node1->AppendChild(std::move(child4)); - node2->AppendChild(std::move(child3)); + node1.AppendChild(&node2); + node1.AppendChild(&node4); + node2.AppendChild(&node3); - EXPECT_EQ(node2, node1->GetFirstChild()); + EXPECT_EQ(&node2, node1.GetFirstChild()); - EXPECT_EQ(node4, node2->GetNextSibling()); - EXPECT_EQ(node3, node2->GetFirstChild()); + EXPECT_EQ(&node4, node2.GetNextSibling()); + EXPECT_EQ(&node3, node2.GetFirstChild()); - EXPECT_TRUE(node3->GetNextSibling() == nullptr); - EXPECT_TRUE(node3->GetFirstChild() == nullptr); + EXPECT_TRUE(node3.GetNextSibling() == nullptr); + EXPECT_TRUE(node3.GetFirstChild() == nullptr); - EXPECT_TRUE(node4->GetNextSibling() == nullptr); - EXPECT_TRUE(node4->GetFirstChild() == nullptr); + EXPECT_TRUE(node4.GetNextSibling() == nullptr); + EXPECT_TRUE(node4.GetFirstChild() == nullptr); } TEST(CFX_XMLNodeTest, DeleteChildren) { - auto node1 = pdfium::MakeUnique<CFX_XMLElement>(L"node"); - auto child2 = pdfium::MakeUnique<CFX_XMLElement>(L"node2"); - auto child3 = pdfium::MakeUnique<CFX_XMLElement>(L"node3"); - auto child4 = pdfium::MakeUnique<CFX_XMLElement>(L"node4"); - - CFX_XMLElement* node2 = child2.get(); - // CFX_XMLElement* node3 = child3.get(); - // CFX_XMLElement* node4 = child4.get(); - - node1->AppendChild(std::move(child2)); - node1->AppendChild(std::move(child4)); - node2->AppendChild(std::move(child3)); - - node1->DeleteChildren(); - EXPECT_TRUE(node1->GetFirstChild() == nullptr); - EXPECT_TRUE(node1->GetLastChildForTesting() == nullptr); + CFX_XMLElement node1(L"node"); + CFX_XMLElement node2(L"node2"); + CFX_XMLElement node3(L"node3"); + CFX_XMLElement node4(L"node4"); - // TODO(dsinclair): This isn't true currently but will be true when - // we own the nodes in an XML document. (Currently nodes are unique_ptrs - // so the objects have been deleted by this point.) + node1.AppendChild(&node2); + node1.AppendChild(&node4); + node2.AppendChild(&node3); - // EXPECT_TRUE(node2->GetParent() == nullptr); - // EXPECT_TRUE(node4->GetParent() == nullptr); + node1.DeleteChildren(); + EXPECT_TRUE(node1.GetFirstChild() == nullptr); + EXPECT_TRUE(node2.GetParent() == nullptr); + EXPECT_TRUE(node4.GetParent() == nullptr); - // // node2 and node4 should no longer be siblings. - // EXPECT_TRUE(node2->GetNextSibling() == nullptr); - // EXPECT_TRUE(node4->GetPrevSiblingForTesting() == nullptr); + // node2 and node4 should no longer be siblings. + EXPECT_TRUE(node2.GetNextSibling() == nullptr); + EXPECT_TRUE(node4.GetPrevSiblingForTesting() == nullptr); // Deleting children doesn't change deleted substructure - // EXPECT_EQ(node3, node2->GetFirstChild()); - // EXPECT_TRUE(node3->GetParent() == node2); + EXPECT_EQ(&node3, node2.GetFirstChild()); + EXPECT_TRUE(node3.GetParent() == &node2); } TEST(CFX_XMLNodeTest, AddingChildren) { - auto node1 = pdfium::MakeUnique<CFX_XMLElement>(L"node"); - auto child2 = pdfium::MakeUnique<CFX_XMLElement>(L"node2"); - auto child3 = pdfium::MakeUnique<CFX_XMLElement>(L"node3"); - auto child4 = pdfium::MakeUnique<CFX_XMLElement>(L"node4"); - auto child5 = pdfium::MakeUnique<CFX_XMLElement>(L"node5"); + CFX_XMLElement node1(L"node"); + CFX_XMLElement node2(L"node2"); + CFX_XMLElement node3(L"node3"); - CFX_XMLElement* node2 = child2.get(); - CFX_XMLElement* node3 = child3.get(); - CFX_XMLElement* node4 = child4.get(); - CFX_XMLElement* node5 = child5.get(); + node1.AppendChild(&node2); + node1.AppendChild(&node3); - node1->AppendChild(std::move(child2)); - node1->AppendChild(std::move(child3)); + EXPECT_EQ(&node1, node2.GetParent()); + EXPECT_EQ(&node1, node3.GetParent()); - EXPECT_EQ(node1.get(), node2->GetParent()); - EXPECT_EQ(node1.get(), node3->GetParent()); - - EXPECT_EQ(node2, node1->GetFirstChild()); - EXPECT_EQ(node3, node2->GetNextSibling()); - EXPECT_TRUE(node3->GetNextSibling() == nullptr); + EXPECT_EQ(&node2, node1.GetFirstChild()); + EXPECT_EQ(&node3, node2.GetNextSibling()); + EXPECT_TRUE(node3.GetNextSibling() == nullptr); // Insert to negative appends. - node1->InsertChildNode(std::move(child4), -1); - EXPECT_EQ(node1.get(), node4->GetParent()); - EXPECT_EQ(node4, node3->GetNextSibling()); - EXPECT_TRUE(node4->GetNextSibling() == nullptr); - - node1->InsertChildNode(std::move(child5), 1); - EXPECT_EQ(node1.get(), node5->GetParent()); - EXPECT_EQ(node2, node1->GetFirstChild()); - EXPECT_EQ(node5, node2->GetNextSibling()); - EXPECT_EQ(node3, node5->GetNextSibling()); - EXPECT_EQ(node4, node3->GetNextSibling()); - EXPECT_TRUE(node4->GetNextSibling() == nullptr); + CFX_XMLElement node4(L"node4"); + node1.InsertChildNode(&node4, -1); + EXPECT_EQ(&node1, node4.GetParent()); + EXPECT_EQ(&node4, node3.GetNextSibling()); + EXPECT_TRUE(node4.GetNextSibling() == nullptr); + + CFX_XMLElement node5(L"node5"); + node1.InsertChildNode(&node5, 1); + EXPECT_EQ(&node1, node5.GetParent()); + EXPECT_EQ(&node2, node1.GetFirstChild()); + EXPECT_EQ(&node5, node2.GetNextSibling()); + EXPECT_EQ(&node3, node5.GetNextSibling()); + EXPECT_EQ(&node4, node3.GetNextSibling()); + EXPECT_TRUE(node4.GetNextSibling() == nullptr); +} + +#ifndef NDEBUG +TEST(CFX_XMLNodeTest, DuplicateInsertOfNode) { + CFX_XMLElement node1(L"node"); + CFX_XMLElement node2(L"node2"); + + node1.AppendChild(&node2); + EXPECT_DEATH(node1.AppendChild(&node2), ""); } +#endif // NDEBUG TEST(CFX_XMLNodeTest, RemovingMiddleChild) { - auto node1 = pdfium::MakeUnique<CFX_XMLElement>(L"node"); - auto child2 = pdfium::MakeUnique<CFX_XMLElement>(L"node2"); - auto child3 = pdfium::MakeUnique<CFX_XMLElement>(L"node3"); - auto child4 = pdfium::MakeUnique<CFX_XMLElement>(L"node4"); - - CFX_XMLElement* node2 = child2.get(); - CFX_XMLElement* node3 = child3.get(); - CFX_XMLElement* node4 = child4.get(); - - node1->AppendChild(std::move(child2)); - node1->AppendChild(std::move(child3)); - node1->AppendChild(std::move(child4)); - - EXPECT_EQ(node2, node1->GetFirstChild()); - EXPECT_EQ(node3, node2->GetNextSibling()); - EXPECT_EQ(node4, node3->GetNextSibling()); - EXPECT_TRUE(node4->GetNextSibling() == nullptr); - - node1->RemoveChildNode(node3); - // Node is released by parent, so need to take ownership - child3 = pdfium::WrapUnique(node3); - - EXPECT_TRUE(node3->GetParent() == nullptr); - EXPECT_TRUE(node3->GetNextSibling() == nullptr); - EXPECT_TRUE(node3->GetPrevSiblingForTesting() == nullptr); - - EXPECT_EQ(node2, node1->GetFirstChild()); - EXPECT_EQ(node4, node2->GetNextSibling()); - EXPECT_EQ(node2, node4->GetPrevSiblingForTesting()); - EXPECT_TRUE(node4->GetNextSibling() == nullptr); + CFX_XMLElement node1(L"node1"); + CFX_XMLElement node2(L"node2"); + CFX_XMLElement node3(L"node3"); + CFX_XMLElement node4(L"node4"); + + node1.AppendChild(&node2); + node1.AppendChild(&node3); + node1.AppendChild(&node4); + + EXPECT_EQ(&node2, node1.GetFirstChild()); + EXPECT_EQ(&node3, node2.GetNextSibling()); + EXPECT_EQ(&node4, node3.GetNextSibling()); + EXPECT_TRUE(node4.GetNextSibling() == nullptr); + + node1.RemoveChildNode(&node3); + EXPECT_TRUE(node3.GetParent() == nullptr); + EXPECT_TRUE(node3.GetNextSibling() == nullptr); + EXPECT_TRUE(node3.GetPrevSiblingForTesting() == nullptr); + + EXPECT_EQ(&node2, node1.GetFirstChild()); + EXPECT_EQ(&node4, node2.GetNextSibling()); + EXPECT_EQ(&node2, node4.GetPrevSiblingForTesting()); + EXPECT_TRUE(node4.GetNextSibling() == nullptr); } TEST(CFX_XMLNodeTest, RemovingFirstChild) { - auto node1 = pdfium::MakeUnique<CFX_XMLElement>(L"node"); - auto child2 = pdfium::MakeUnique<CFX_XMLElement>(L"node2"); - auto child3 = pdfium::MakeUnique<CFX_XMLElement>(L"node3"); - auto child4 = pdfium::MakeUnique<CFX_XMLElement>(L"node4"); - - CFX_XMLElement* node2 = child2.get(); - CFX_XMLElement* node3 = child3.get(); - CFX_XMLElement* node4 = child4.get(); - - node1->AppendChild(std::move(child2)); - node1->AppendChild(std::move(child3)); - node1->AppendChild(std::move(child4)); - - EXPECT_EQ(node2, node1->GetFirstChild()); - EXPECT_EQ(node3, node2->GetNextSibling()); - EXPECT_EQ(node4, node3->GetNextSibling()); - EXPECT_TRUE(node4->GetNextSibling() == nullptr); - - node1->RemoveChildNode(node2); - // Node is released by parent, so need to take ownership - child2 = pdfium::WrapUnique(node2); - - EXPECT_TRUE(node2->GetParent() == nullptr); - EXPECT_TRUE(node2->GetNextSibling() == nullptr); - EXPECT_TRUE(node2->GetPrevSiblingForTesting() == nullptr); - - EXPECT_EQ(node3, node1->GetFirstChild()); - EXPECT_TRUE(node3->GetPrevSiblingForTesting() == nullptr); - EXPECT_EQ(node4, node3->GetNextSibling()); - EXPECT_TRUE(node4->GetNextSibling() == nullptr); + CFX_XMLElement node1(L"node1"); + CFX_XMLElement node2(L"node2"); + CFX_XMLElement node3(L"node3"); + CFX_XMLElement node4(L"node4"); + + node1.AppendChild(&node2); + node1.AppendChild(&node3); + node1.AppendChild(&node4); + + EXPECT_EQ(&node2, node1.GetFirstChild()); + EXPECT_EQ(&node3, node2.GetNextSibling()); + EXPECT_EQ(&node4, node3.GetNextSibling()); + EXPECT_TRUE(node4.GetNextSibling() == nullptr); + + node1.RemoveChildNode(&node2); + EXPECT_TRUE(node2.GetParent() == nullptr); + EXPECT_TRUE(node2.GetNextSibling() == nullptr); + EXPECT_TRUE(node2.GetPrevSiblingForTesting() == nullptr); + + EXPECT_EQ(&node3, node1.GetFirstChild()); + EXPECT_TRUE(node3.GetPrevSiblingForTesting() == nullptr); + EXPECT_EQ(&node4, node3.GetNextSibling()); + EXPECT_TRUE(node4.GetNextSibling() == nullptr); } TEST(CFX_XMLNodeTest, RemovingLastChild) { - auto node1 = pdfium::MakeUnique<CFX_XMLElement>(L"node"); - auto child2 = pdfium::MakeUnique<CFX_XMLElement>(L"node2"); - auto child3 = pdfium::MakeUnique<CFX_XMLElement>(L"node3"); - auto child4 = pdfium::MakeUnique<CFX_XMLElement>(L"node4"); - - CFX_XMLElement* node2 = child2.get(); - CFX_XMLElement* node3 = child3.get(); - CFX_XMLElement* node4 = child4.get(); - - node1->AppendChild(std::move(child2)); - node1->AppendChild(std::move(child3)); - node1->AppendChild(std::move(child4)); - - EXPECT_EQ(node2, node1->GetFirstChild()); - EXPECT_EQ(node3, node2->GetNextSibling()); - EXPECT_EQ(node4, node3->GetNextSibling()); - EXPECT_TRUE(node4->GetNextSibling() == nullptr); - - node1->RemoveChildNode(node4); - // Node is released by parent, so need to take ownership - child4 = pdfium::WrapUnique(node4); - - EXPECT_TRUE(node4->GetParent() == nullptr); - EXPECT_TRUE(node4->GetNextSibling() == nullptr); - EXPECT_TRUE(node4->GetPrevSiblingForTesting() == nullptr); - - EXPECT_EQ(node2, node1->GetFirstChild()); - EXPECT_EQ(node3, node2->GetNextSibling()); - EXPECT_TRUE(node3->GetNextSibling() == nullptr); + CFX_XMLElement node1(L"node1"); + CFX_XMLElement node2(L"node2"); + CFX_XMLElement node3(L"node3"); + CFX_XMLElement node4(L"node4"); + + node1.AppendChild(&node2); + node1.AppendChild(&node3); + node1.AppendChild(&node4); + + EXPECT_EQ(&node2, node1.GetFirstChild()); + EXPECT_EQ(&node3, node2.GetNextSibling()); + EXPECT_EQ(&node4, node3.GetNextSibling()); + EXPECT_TRUE(node4.GetNextSibling() == nullptr); + + node1.RemoveChildNode(&node4); + EXPECT_TRUE(node4.GetParent() == nullptr); + EXPECT_TRUE(node4.GetNextSibling() == nullptr); + EXPECT_TRUE(node4.GetPrevSiblingForTesting() == nullptr); + + EXPECT_EQ(&node2, node1.GetFirstChild()); + EXPECT_EQ(&node3, node2.GetNextSibling()); + EXPECT_TRUE(node3.GetNextSibling() == nullptr); } TEST(CFX_XMLNodeTest, RemovingOnlyChild) { - auto node1 = pdfium::MakeUnique<CFX_XMLElement>(L"node"); - auto child2 = pdfium::MakeUnique<CFX_XMLElement>(L"node2"); + CFX_XMLElement node1(L"node1"); + CFX_XMLElement node2(L"node2"); - CFX_XMLElement* node2 = child2.get(); + node1.AppendChild(&node2); - node1->AppendChild(std::move(child2)); + EXPECT_EQ(&node2, node1.GetFirstChild()); + EXPECT_TRUE(node2.GetNextSibling() == nullptr); - EXPECT_EQ(node2, node1->GetFirstChild()); - EXPECT_TRUE(node2->GetNextSibling() == nullptr); + node1.RemoveChildNode(&node2); + EXPECT_TRUE(node2.GetParent() == nullptr); - node1->RemoveChildNode(node2); - // Node is released by parent, so need to take ownership - child2 = pdfium::WrapUnique(node2); - - EXPECT_TRUE(node2->GetParent() == nullptr); - - EXPECT_TRUE(node1->GetFirstChild() == nullptr); - EXPECT_TRUE(node2->GetNextSibling() == nullptr); - EXPECT_TRUE(node2->GetPrevSiblingForTesting() == nullptr); + EXPECT_TRUE(node1.GetFirstChild() == nullptr); + EXPECT_TRUE(node2.GetNextSibling() == nullptr); + EXPECT_TRUE(node2.GetPrevSiblingForTesting() == nullptr); } TEST(CFX_XMLNodeTest, RemoveMissingChild) { - auto node1 = pdfium::MakeUnique<CFX_XMLElement>(L"node"); - auto child2 = pdfium::MakeUnique<CFX_XMLElement>(L"node2"); - auto child3 = pdfium::MakeUnique<CFX_XMLElement>(L"node3"); - - CFX_XMLElement* node2 = child2.get(); - CFX_XMLElement* node3 = child3.get(); + CFX_XMLElement node1(L"node1"); + CFX_XMLElement node2(L"node2"); + CFX_XMLElement node3(L"node3"); - node1->AppendChild(std::move(child2)); - node1->RemoveChildNode(node3); + node1.AppendChild(&node2); + node1.RemoveChildNode(&node3); - EXPECT_TRUE(node3->GetParent() == nullptr); - EXPECT_EQ(node2, node1->GetFirstChild()); - EXPECT_TRUE(node2->GetNextSibling() == nullptr); + EXPECT_TRUE(node3.GetParent() == nullptr); + EXPECT_EQ(&node2, node1.GetFirstChild()); + EXPECT_TRUE(node2.GetNextSibling() == nullptr); } diff --git a/core/fxcrt/xml/cfx_xmlparser.cpp b/core/fxcrt/xml/cfx_xmlparser.cpp index c7a81afc16..b7dbd19eab 100644 --- a/core/fxcrt/xml/cfx_xmlparser.cpp +++ b/core/fxcrt/xml/cfx_xmlparser.cpp @@ -16,6 +16,7 @@ #include "core/fxcrt/fx_extension.h" #include "core/fxcrt/fx_safe_types.h" #include "core/fxcrt/xml/cfx_xmlchardata.h" +#include "core/fxcrt/xml/cfx_xmldocument.h" #include "core/fxcrt/xml/cfx_xmlelement.h" #include "core/fxcrt/xml/cfx_xmlinstruction.h" #include "core/fxcrt/xml/cfx_xmlnode.h" @@ -58,10 +59,7 @@ bool CFX_XMLParser::IsXMLNameChar(wchar_t ch, bool bFirstChar) { (!bFirstChar || it->bStartChar); } -CFX_XMLParser::CFX_XMLParser(CFX_XMLNode* pParent, - const RetainPtr<IFX_SeekableReadStream>& pStream) - : m_pParent(pParent) { - ASSERT(m_pParent); +CFX_XMLParser::CFX_XMLParser(const RetainPtr<IFX_SeekableReadStream>& pStream) { ASSERT(pStream); auto proxy = pdfium::MakeRetain<CFX_SeekableStreamProxy>(pStream); @@ -89,56 +87,51 @@ CFX_XMLParser::CFX_XMLParser(CFX_XMLNode* pParent, CFX_XMLParser::~CFX_XMLParser() = default; -bool CFX_XMLParser::Parse() { +std::unique_ptr<CFX_XMLDocument> CFX_XMLParser::Parse() { + auto doc = pdfium::MakeUnique<CFX_XMLDocument>(); + current_node_ = doc->GetRoot(); + int32_t iCount = 0; while (true) { FX_XmlSyntaxResult result = DoSyntaxParse(); if (result == FX_XmlSyntaxResult::Error) - return false; + return nullptr; if (result == FX_XmlSyntaxResult::EndOfString) break; switch (result) { case FX_XmlSyntaxResult::InstructionClose: - if (m_pChild && m_pChild->GetType() != FX_XMLNODE_Instruction) - return false; - - m_pChild = m_pParent; + if (current_node_ && current_node_->GetType() == FX_XMLNODE_Instruction) + current_node_ = current_node_->GetParent(); break; case FX_XmlSyntaxResult::ElementClose: { - if (m_pChild->GetType() != FX_XMLNODE_Element) - return false; + if (current_node_->GetType() != FX_XMLNODE_Element) + return nullptr; WideString element_name = GetTextData(); if (element_name.GetLength() > 0 && - element_name != static_cast<CFX_XMLElement*>(m_pChild)->GetName()) { - return false; + element_name != + static_cast<CFX_XMLElement*>(current_node_)->GetName()) { + return nullptr; } - if (!m_pChild || !m_pChild->GetParent()) - return false; - - m_pParent = m_pChild->GetParent(); - m_pChild = m_pParent; + current_node_ = current_node_->GetParent(); iCount++; break; } case FX_XmlSyntaxResult::TargetName: { WideString target_name = GetTextData(); if (target_name == L"originalXFAVersion" || target_name == L"acrobat") { - auto child = pdfium::MakeUnique<CFX_XMLInstruction>(target_name); - m_pChild = child.get(); - m_pParent->AppendChild(std::move(child)); - } else { - m_pChild = nullptr; + auto* node = doc->CreateNode<CFX_XMLInstruction>(target_name); + current_node_->AppendChild(node); + current_node_ = node; } break; } case FX_XmlSyntaxResult::TagName: { - auto child = pdfium::MakeUnique<CFX_XMLElement>(GetTextData()); - m_pChild = child.get(); - m_pParent->AppendChild(std::move(child)); - m_pParent = m_pChild; + auto* child = doc->CreateNode<CFX_XMLElement>(GetTextData()); + current_node_->AppendChild(child); + current_node_ = child; break; } case FX_XmlSyntaxResult::AttriName: { @@ -146,35 +139,29 @@ bool CFX_XMLParser::Parse() { break; } case FX_XmlSyntaxResult::AttriValue: - if (m_pChild && m_pChild->GetType() == FX_XMLNODE_Element) { - static_cast<CFX_XMLElement*>(m_pChild)->SetAttribute( - current_attribute_name_, GetTextData()); + if (current_node_ && current_node_->GetType() == FX_XMLNODE_Element) { + static_cast<CFX_XMLElement*>(current_node_) + ->SetAttribute(current_attribute_name_, GetTextData()); } current_attribute_name_.clear(); break; case FX_XmlSyntaxResult::Text: { - auto child = pdfium::MakeUnique<CFX_XMLText>(GetTextData()); - m_pChild = child.get(); - m_pParent->AppendChild(std::move(child)); - m_pChild = m_pParent; + current_node_->AppendChild(doc->CreateNode<CFX_XMLText>(GetTextData())); break; } case FX_XmlSyntaxResult::CData: { - auto child = pdfium::MakeUnique<CFX_XMLCharData>(GetTextData()); - m_pChild = child.get(); - m_pParent->AppendChild(std::move(child)); - m_pChild = m_pParent; + current_node_->AppendChild( + doc->CreateNode<CFX_XMLCharData>(GetTextData())); break; } case FX_XmlSyntaxResult::TargetData: { WideString target_data = GetTextData(); - if (m_pChild) { - if (m_pChild->GetType() != FX_XMLNODE_Instruction) - return false; - - auto* instruction = static_cast<CFX_XMLInstruction*>(m_pChild); - if (!target_data.IsEmpty()) - instruction->AppendData(target_data); + if (current_node_ && + current_node_->GetType() == FX_XMLNODE_Instruction) { + if (target_data.IsEmpty()) + break; + static_cast<CFX_XMLInstruction*>(current_node_) + ->AppendData(target_data); } break; } @@ -185,7 +172,7 @@ bool CFX_XMLParser::Parse() { break; } } - return !m_pParent || m_pParent->GetParent() ? false : GetStatus(); + return doc; } FX_XmlSyntaxResult CFX_XMLParser::DoSyntaxParse() { diff --git a/core/fxcrt/xml/cfx_xmlparser.h b/core/fxcrt/xml/cfx_xmlparser.h index e2a2d5b30f..6bf1946402 100644 --- a/core/fxcrt/xml/cfx_xmlparser.h +++ b/core/fxcrt/xml/cfx_xmlparser.h @@ -15,6 +15,7 @@ #include "core/fxcrt/retain_ptr.h" #include "core/fxcrt/xml/cfx_xmlnode.h" +class CFX_XMLDocument; class CFX_XMLElement; class CFX_XMLNode; class IFX_SeekableReadStream; @@ -41,11 +42,10 @@ class CFX_XMLParser { public: static bool IsXMLNameChar(wchar_t ch, bool bFirstChar); - CFX_XMLParser(CFX_XMLNode* pParent, - const RetainPtr<IFX_SeekableReadStream>& pStream); + explicit CFX_XMLParser(const RetainPtr<IFX_SeekableReadStream>& pStream); virtual ~CFX_XMLParser(); - bool Parse(); + std::unique_ptr<CFX_XMLDocument> Parse(); protected: FX_XmlSyntaxResult DoSyntaxParse(); @@ -74,8 +74,7 @@ class CFX_XMLParser { void ParseTextChar(wchar_t ch); bool GetStatus() const; - CFX_XMLNode* m_pParent; - CFX_XMLNode* m_pChild = nullptr; + CFX_XMLNode* current_node_ = nullptr; WideString current_attribute_name_; RetainPtr<IFX_SeekableReadStream> m_pStream; FX_FILESIZE m_Start = 0; // Start position in m_Buffer diff --git a/core/fxcrt/xml/cfx_xmlparser_unittest.cpp b/core/fxcrt/xml/cfx_xmlparser_unittest.cpp index 73d6685dad..7ca242211d 100644 --- a/core/fxcrt/xml/cfx_xmlparser_unittest.cpp +++ b/core/fxcrt/xml/cfx_xmlparser_unittest.cpp @@ -8,7 +8,9 @@ #include "core/fxcrt/cfx_memorystream.h" #include "core/fxcrt/fx_codepage.h" +#include "core/fxcrt/xml/cfx_xmldocument.h" #include "core/fxcrt/xml/cfx_xmlelement.h" +#include "core/fxcrt/xml/cfx_xmlinstruction.h" #include "testing/gtest/include/gtest/gtest.h" #include "testing/test_support.h" #include "third_party/base/ptr_util.h" @@ -17,9 +19,8 @@ namespace { class CFX_XMLTestParser : public CFX_XMLParser { public: - CFX_XMLTestParser(CFX_XMLNode* pParent, - const RetainPtr<IFX_SeekableStream>& pStream) - : CFX_XMLParser(pParent, pStream) {} + explicit CFX_XMLTestParser(const RetainPtr<IFX_SeekableStream>& pStream) + : CFX_XMLParser(pStream) {} ~CFX_XMLTestParser() override = default; @@ -52,9 +53,7 @@ TEST(CFX_XMLParserTest, CData) { L" "; auto stream = MakeProxy(input); - auto root = pdfium::MakeUnique<CFX_XMLElement>(L"ROOT"); - - CFX_XMLTestParser parser(root.get(), stream); + CFX_XMLTestParser parser(stream); ASSERT_EQ(FX_XmlSyntaxResult::ElementOpen, parser.DoSyntaxParse()); ASSERT_EQ(FX_XmlSyntaxResult::TagName, parser.DoSyntaxParse()); ASSERT_EQ(L"script", parser.GetTextData()); @@ -98,9 +97,7 @@ TEST(CFX_XMLParserTest, CDataWithInnerScript) { L" "; auto stream = MakeProxy(input); - auto root = pdfium::MakeUnique<CFX_XMLElement>(L"ROOT"); - - CFX_XMLTestParser parser(root.get(), stream); + CFX_XMLTestParser parser(stream); ASSERT_EQ(FX_XmlSyntaxResult::ElementOpen, parser.DoSyntaxParse()); ASSERT_EQ(FX_XmlSyntaxResult::TagName, parser.DoSyntaxParse()); ASSERT_EQ(L"script", parser.GetTextData()); @@ -133,9 +130,7 @@ TEST(CFX_XMLParserTest, ArrowBangArrow) { "</script>"; auto stream = MakeProxy(input); - auto root = pdfium::MakeUnique<CFX_XMLElement>(L"ROOT"); - - CFX_XMLTestParser parser(root.get(), stream); + CFX_XMLTestParser parser(stream); ASSERT_EQ(FX_XmlSyntaxResult::ElementOpen, parser.DoSyntaxParse()); ASSERT_EQ(FX_XmlSyntaxResult::TagName, parser.DoSyntaxParse()); @@ -166,9 +161,7 @@ TEST(CFX_XMLParserTest, ArrowBangBracketArrow) { "</script>"; auto stream = MakeProxy(input); - auto root = pdfium::MakeUnique<CFX_XMLElement>(L"ROOT"); - - CFX_XMLTestParser parser(root.get(), stream); + CFX_XMLTestParser parser(stream); ASSERT_EQ(FX_XmlSyntaxResult::ElementOpen, parser.DoSyntaxParse()); ASSERT_EQ(FX_XmlSyntaxResult::TagName, parser.DoSyntaxParse()); ASSERT_EQ(L"script", parser.GetTextData()); @@ -194,9 +187,7 @@ TEST(CFX_XMLParserTest, IncompleteCData) { "</script>"; auto stream = MakeProxy(input); - auto root = pdfium::MakeUnique<CFX_XMLElement>(L"ROOT"); - - CFX_XMLTestParser parser(root.get(), stream); + CFX_XMLTestParser parser(stream); ASSERT_EQ(FX_XmlSyntaxResult::ElementOpen, parser.DoSyntaxParse()); ASSERT_EQ(FX_XmlSyntaxResult::TagName, parser.DoSyntaxParse()); ASSERT_EQ(L"script", parser.GetTextData()); @@ -222,9 +213,7 @@ TEST(CFX_XMLParserTest, UnClosedCData) { "</script>"; auto stream = MakeProxy(input); - auto root = pdfium::MakeUnique<CFX_XMLElement>(L"ROOT"); - - CFX_XMLTestParser parser(root.get(), stream); + CFX_XMLTestParser parser(stream); ASSERT_EQ(FX_XmlSyntaxResult::ElementOpen, parser.DoSyntaxParse()); ASSERT_EQ(FX_XmlSyntaxResult::TagName, parser.DoSyntaxParse()); ASSERT_EQ(L"script", parser.GetTextData()); @@ -250,9 +239,7 @@ TEST(CFX_XMLParserTest, EmptyCData) { "</script>"; auto stream = MakeProxy(input); - auto root = pdfium::MakeUnique<CFX_XMLElement>(L"ROOT"); - - CFX_XMLTestParser parser(root.get(), stream); + CFX_XMLTestParser parser(stream); ASSERT_EQ(FX_XmlSyntaxResult::ElementOpen, parser.DoSyntaxParse()); ASSERT_EQ(FX_XmlSyntaxResult::TagName, parser.DoSyntaxParse()); ASSERT_EQ(L"script", parser.GetTextData()); @@ -285,9 +272,7 @@ TEST(CFX_XMLParserTest, Comment) { "</script>"; auto stream = MakeProxy(input); - auto root = pdfium::MakeUnique<CFX_XMLElement>(L"ROOT"); - - CFX_XMLTestParser parser(root.get(), stream); + CFX_XMLTestParser parser(stream); ASSERT_EQ(FX_XmlSyntaxResult::ElementOpen, parser.DoSyntaxParse()); ASSERT_EQ(FX_XmlSyntaxResult::TagName, parser.DoSyntaxParse()); ASSERT_EQ(L"script", parser.GetTextData()); @@ -317,9 +302,7 @@ TEST(CFX_XMLParserTest, IncorrectCommentStart) { "</script>"; auto stream = MakeProxy(input); - auto root = pdfium::MakeUnique<CFX_XMLElement>(L"ROOT"); - - CFX_XMLTestParser parser(root.get(), stream); + CFX_XMLTestParser parser(stream); ASSERT_EQ(FX_XmlSyntaxResult::ElementOpen, parser.DoSyntaxParse()); ASSERT_EQ(FX_XmlSyntaxResult::TagName, parser.DoSyntaxParse()); ASSERT_EQ(L"script", parser.GetTextData()); @@ -349,9 +332,7 @@ TEST(CFX_XMLParserTest, CommentEmpty) { "</script>"; auto stream = MakeProxy(input); - auto root = pdfium::MakeUnique<CFX_XMLElement>(L"ROOT"); - - CFX_XMLTestParser parser(root.get(), stream); + CFX_XMLTestParser parser(stream); ASSERT_EQ(FX_XmlSyntaxResult::ElementOpen, parser.DoSyntaxParse()); ASSERT_EQ(FX_XmlSyntaxResult::TagName, parser.DoSyntaxParse()); ASSERT_EQ(L"script", parser.GetTextData()); @@ -381,9 +362,7 @@ TEST(CFX_XMLParserTest, CommentThreeDash) { "</script>"; auto stream = MakeProxy(input); - auto root = pdfium::MakeUnique<CFX_XMLElement>(L"ROOT"); - - CFX_XMLTestParser parser(root.get(), stream); + CFX_XMLTestParser parser(stream); ASSERT_EQ(FX_XmlSyntaxResult::ElementOpen, parser.DoSyntaxParse()); ASSERT_EQ(FX_XmlSyntaxResult::TagName, parser.DoSyntaxParse()); ASSERT_EQ(L"script", parser.GetTextData()); @@ -407,9 +386,7 @@ TEST(CFX_XMLParserTest, CommentTwoDash) { "</script>"; auto stream = MakeProxy(input); - auto root = pdfium::MakeUnique<CFX_XMLElement>(L"ROOT"); - - CFX_XMLTestParser parser(root.get(), stream); + CFX_XMLTestParser parser(stream); ASSERT_EQ(FX_XmlSyntaxResult::ElementOpen, parser.DoSyntaxParse()); ASSERT_EQ(FX_XmlSyntaxResult::TagName, parser.DoSyntaxParse()); ASSERT_EQ(L"script", parser.GetTextData()); @@ -444,9 +421,7 @@ TEST(CFX_XMLParserTest, Entities) { "</script>"; auto stream = MakeProxy(input); - auto root = pdfium::MakeUnique<CFX_XMLElement>(L"ROOT"); - - CFX_XMLTestParser parser(root.get(), stream); + CFX_XMLTestParser parser(stream); ASSERT_EQ(FX_XmlSyntaxResult::ElementOpen, parser.DoSyntaxParse()); ASSERT_EQ(FX_XmlSyntaxResult::TagName, parser.DoSyntaxParse()); ASSERT_EQ(L"script", parser.GetTextData()); @@ -474,9 +449,7 @@ TEST(CFX_XMLParserTest, EntityOverflowHex) { "</script>"; auto stream = MakeProxy(input); - auto root = pdfium::MakeUnique<CFX_XMLElement>(L"ROOT"); - - CFX_XMLTestParser parser(root.get(), stream); + CFX_XMLTestParser parser(stream); ASSERT_EQ(FX_XmlSyntaxResult::ElementOpen, parser.DoSyntaxParse()); ASSERT_EQ(FX_XmlSyntaxResult::TagName, parser.DoSyntaxParse()); ASSERT_EQ(L"script", parser.GetTextData()); @@ -504,9 +477,7 @@ TEST(CFX_XMLParserTest, EntityOverflowDecimal) { "</script>"; auto stream = MakeProxy(input); - auto root = pdfium::MakeUnique<CFX_XMLElement>(L"ROOT"); - - CFX_XMLTestParser parser(root.get(), stream); + CFX_XMLTestParser parser(stream); ASSERT_EQ(FX_XmlSyntaxResult::ElementOpen, parser.DoSyntaxParse()); ASSERT_EQ(FX_XmlSyntaxResult::TagName, parser.DoSyntaxParse()); ASSERT_EQ(L"script", parser.GetTextData()); @@ -547,9 +518,7 @@ TEST(CFX_XMLParserTest, BadElementClose) { const char* input = "</endtag>"; auto stream = MakeProxy(input); - auto root = pdfium::MakeUnique<CFX_XMLElement>(L"ROOT"); - - CFX_XMLTestParser parser(root.get(), stream); + CFX_XMLTestParser parser(stream); ASSERT_EQ(FX_XmlSyntaxResult::Error, parser.DoSyntaxParse()); } @@ -557,9 +526,7 @@ TEST(CFX_XMLParserTest, DoubleElementClose) { const char* input = "<p></p></p>"; auto stream = MakeProxy(input); - auto root = pdfium::MakeUnique<CFX_XMLElement>(L"ROOT"); - - CFX_XMLTestParser parser(root.get(), stream); + CFX_XMLTestParser parser(stream); ASSERT_EQ(FX_XmlSyntaxResult::ElementOpen, parser.DoSyntaxParse()); ASSERT_EQ(FX_XmlSyntaxResult::TagName, parser.DoSyntaxParse()); ASSERT_EQ(L"p", parser.GetTextData()); @@ -570,6 +537,26 @@ TEST(CFX_XMLParserTest, DoubleElementClose) { ASSERT_EQ(FX_XmlSyntaxResult::Error, parser.DoSyntaxParse()); } +TEST(CFX_XMLParserTest, ParseInstruction) { + const char* input = + "<?originalXFAVersion http://www.xfa.org/schema/xfa-template/3.3/ ?>" + "<form></form>"; + + auto stream = MakeProxy(input); + CFX_XMLTestParser parser(stream); + + auto doc = parser.Parse(); + ASSERT_TRUE(doc.get() != nullptr); + + CFX_XMLElement* root = doc->GetRoot(); + ASSERT_TRUE(root->GetFirstChild() != nullptr); + ASSERT_EQ(FX_XMLNODE_Instruction, root->GetFirstChild()->GetType()); + + CFX_XMLInstruction* instruction = + static_cast<CFX_XMLInstruction*>(root->GetFirstChild()); + EXPECT_TRUE(instruction->IsOriginalXFAVersion()); +} + TEST(CFX_XMLParserTest, BadEntity) { const char* input = "<script>" @@ -577,9 +564,7 @@ TEST(CFX_XMLParserTest, BadEntity) { "</script>"; auto stream = MakeProxy(input); - auto root = pdfium::MakeUnique<CFX_XMLElement>(L"ROOT"); - - CFX_XMLTestParser parser(root.get(), stream); + CFX_XMLTestParser parser(stream); ASSERT_EQ(FX_XmlSyntaxResult::ElementOpen, parser.DoSyntaxParse()); ASSERT_EQ(FX_XmlSyntaxResult::TagName, parser.DoSyntaxParse()); ASSERT_EQ(L"script", parser.GetTextData()); diff --git a/core/fxcrt/xml/cfx_xmltext.cpp b/core/fxcrt/xml/cfx_xmltext.cpp index 2c324be1c9..73f0477482 100644 --- a/core/fxcrt/xml/cfx_xmltext.cpp +++ b/core/fxcrt/xml/cfx_xmltext.cpp @@ -6,6 +6,7 @@ #include "core/fxcrt/xml/cfx_xmltext.h" +#include "core/fxcrt/xml/cfx_xmldocument.h" #include "third_party/base/ptr_util.h" CFX_XMLText::CFX_XMLText(const WideString& wsText) @@ -17,8 +18,8 @@ FX_XMLNODETYPE CFX_XMLText::GetType() const { return FX_XMLNODE_Text; } -std::unique_ptr<CFX_XMLNode> CFX_XMLText::Clone() { - return pdfium::MakeUnique<CFX_XMLText>(m_wsText); +CFX_XMLNode* CFX_XMLText::Clone(CFX_XMLDocument* doc) { + return doc->CreateNode<CFX_XMLText>(m_wsText); } void CFX_XMLText::Save(const RetainPtr<IFX_SeekableWriteStream>& pXMLStream) { diff --git a/core/fxcrt/xml/cfx_xmltext.h b/core/fxcrt/xml/cfx_xmltext.h index cc9c71d01c..c5c168059d 100644 --- a/core/fxcrt/xml/cfx_xmltext.h +++ b/core/fxcrt/xml/cfx_xmltext.h @@ -12,6 +12,8 @@ #include "core/fxcrt/fx_string.h" #include "core/fxcrt/xml/cfx_xmlnode.h" +class CFX_XMLDocument; + class CFX_XMLText : public CFX_XMLNode { public: explicit CFX_XMLText(const WideString& wsText); @@ -19,7 +21,7 @@ class CFX_XMLText : public CFX_XMLNode { // CFX_XMLNode FX_XMLNODETYPE GetType() const override; - std::unique_ptr<CFX_XMLNode> Clone() override; + CFX_XMLNode* Clone(CFX_XMLDocument* doc) override; void Save(const RetainPtr<IFX_SeekableWriteStream>& pXMLStream) override; WideString GetText() const { return m_wsText; } diff --git a/core/fxcrt/xml/cfx_xmltext_unittest.cpp b/core/fxcrt/xml/cfx_xmltext_unittest.cpp index ab977e9546..6e53f66612 100644 --- a/core/fxcrt/xml/cfx_xmltext_unittest.cpp +++ b/core/fxcrt/xml/cfx_xmltext_unittest.cpp @@ -3,6 +3,7 @@ // found in the LICENSE file. #include "core/fxcrt/xml/cfx_xmltext.h" +#include "core/fxcrt/xml/cfx_xmldocument.h" #include "testing/gtest/include/gtest/gtest.h" #include "testing/string_write_stream.h" #include "testing/test_support.h" @@ -18,11 +19,13 @@ TEST(CFX_XMLTextTest, GetText) { } TEST(CFX_XMLTextTest, Clone) { + CFX_XMLDocument doc; + CFX_XMLText data(L"My Data"); - auto clone = data.Clone(); + CFX_XMLNode* clone = data.Clone(&doc); EXPECT_TRUE(clone != nullptr); ASSERT_EQ(FX_XMLNODE_Text, clone->GetType()); - EXPECT_EQ(L"My Data", static_cast<CFX_XMLText*>(clone.get())->GetText()); + EXPECT_EQ(L"My Data", static_cast<CFX_XMLText*>(clone)->GetText()); } TEST(CFX_XMLTextTest, Save) { diff --git a/fxjs/xfa/cjx_node.cpp b/fxjs/xfa/cjx_node.cpp index a8a88c9627..a07aa4cb2c 100644 --- a/fxjs/xfa/cjx_node.cpp +++ b/fxjs/xfa/cjx_node.cpp @@ -16,6 +16,7 @@ #include "fxjs/js_resources.h" #include "third_party/base/ptr_util.h" #include "xfa/fxfa/cxfa_eventparam.h" +#include "xfa/fxfa/cxfa_ffdoc.h" #include "xfa/fxfa/cxfa_ffnotify.h" #include "xfa/fxfa/parser/cxfa_document.h" #include "xfa/fxfa/parser/cxfa_document_parser.h" @@ -236,14 +237,26 @@ CJS_Return CJX_Node::loadXML(CFX_V8* runtime, WideString(wsContentType), false, false); } - std::unique_ptr<CFX_XMLNode> pFakeXMLRoot(pFakeRoot->GetXMLMappingNode()); + CFX_XMLNode* pFakeXMLRoot = pFakeRoot->GetXMLMappingNode(); if (!pFakeXMLRoot) { CFX_XMLNode* pThisXMLRoot = GetXFANode()->GetXMLMappingNode(); - pFakeXMLRoot = pThisXMLRoot ? pThisXMLRoot->Clone() : nullptr; - } - if (!pFakeXMLRoot) { - pFakeXMLRoot = pdfium::MakeUnique<CFX_XMLElement>( - WideString(GetXFANode()->GetClassName())); + CFX_XMLNode* clone; + if (pThisXMLRoot) { + clone = pThisXMLRoot->Clone(GetXFANode() + ->GetDocument() + ->GetNotify() + ->GetHDOC() + ->GetXMLDocument()); + } else { + clone = GetXFANode() + ->GetDocument() + ->GetNotify() + ->GetHDOC() + ->GetXMLDocument() + ->CreateNode<CFX_XMLElement>( + WideString(GetXFANode()->GetClassName())); + } + pFakeXMLRoot = clone; } if (bIgnoreRoot) { @@ -251,7 +264,7 @@ CJS_Return CJX_Node::loadXML(CFX_V8* runtime, while (pXMLChild) { CFX_XMLNode* pXMLSibling = pXMLChild->GetNextSibling(); pXMLNode->RemoveChildNode(pXMLChild); - pFakeXMLRoot->AppendChild(pdfium::WrapUnique<CFX_XMLNode>(pXMLChild)); + pFakeXMLRoot->AppendChild(pXMLChild); pXMLChild = pXMLSibling; } } else { @@ -259,10 +272,10 @@ CJS_Return CJX_Node::loadXML(CFX_V8* runtime, if (pXMLParent) pXMLParent->RemoveChildNode(pXMLNode); - pFakeXMLRoot->AppendChild(pdfium::WrapUnique<CFX_XMLNode>(pXMLNode)); + pFakeXMLRoot->AppendChild(pXMLNode); } - pParser->ConstructXFANode(pFakeRoot, pFakeXMLRoot.get()); + pParser->ConstructXFANode(pFakeRoot, pFakeXMLRoot); pFakeRoot = pParser->GetRootNode(); if (!pFakeRoot) return CJS_Return(true); @@ -289,10 +302,10 @@ CJS_Return CJX_Node::loadXML(CFX_V8* runtime, if (GetXFANode()->GetPacketType() == XFA_PacketType::Form && GetXFANode()->GetElementType() == XFA_Element::ExData) { CFX_XMLNode* pTempXMLNode = GetXFANode()->GetXMLMappingNode(); - GetXFANode()->SetXMLMappingNode(std::move(pFakeXMLRoot)); + GetXFANode()->SetXMLMappingNode(pFakeXMLRoot); if (pTempXMLNode && !pTempXMLNode->GetParent()) - pFakeXMLRoot.reset(pTempXMLNode); + pFakeXMLRoot = pTempXMLNode; else pFakeXMLRoot = nullptr; } diff --git a/fxjs/xfa/cjx_packet.cpp b/fxjs/xfa/cjx_packet.cpp index bd39ec0538..c3d8dbbeba 100644 --- a/fxjs/xfa/cjx_packet.cpp +++ b/fxjs/xfa/cjx_packet.cpp @@ -6,10 +6,15 @@ #include "fxjs/xfa/cjx_packet.h" +#include <utility> #include <vector> +#include "core/fxcrt/xml/cfx_xmldocument.h" +#include "core/fxcrt/xml/cfx_xmltext.h" #include "fxjs/cfxjse_value.h" #include "fxjs/js_resources.h" +#include "xfa/fxfa/cxfa_ffdoc.h" +#include "xfa/fxfa/cxfa_ffnotify.h" #include "xfa/fxfa/parser/cxfa_packet.h" const CJX_MethodSpec CJX_Packet::MethodSpecs[] = { @@ -75,8 +80,13 @@ void CJX_Packet::content(CFXJSE_Value* pValue, CFX_XMLNode* pXMLNode = GetXFANode()->GetXMLMappingNode(); if (bSetting) { if (pXMLNode && pXMLNode->GetType() == FX_XMLNODE_Element) { - CFX_XMLElement* pXMLElement = static_cast<CFX_XMLElement*>(pXMLNode); - pXMLElement->SetTextData(pValue->ToWideString()); + auto* text = GetXFANode() + ->GetDocument() + ->GetNotify() + ->GetHDOC() + ->GetXMLDocument() + ->CreateNode<CFX_XMLText>(pValue->ToWideString()); + pXMLNode->AppendChild(text); } return; } diff --git a/testing/libfuzzer/pdf_xml_fuzzer.cc b/testing/libfuzzer/pdf_xml_fuzzer.cc index 97b9d3c7cf..a3c399f78d 100644 --- a/testing/libfuzzer/pdf_xml_fuzzer.cc +++ b/testing/libfuzzer/pdf_xml_fuzzer.cc @@ -9,6 +9,7 @@ #include "core/fxcrt/cfx_memorystream.h" #include "core/fxcrt/fx_safe_types.h" #include "core/fxcrt/fx_system.h" +#include "core/fxcrt/xml/cfx_xmldocument.h" #include "core/fxcrt/xml/cfx_xmlelement.h" #include "core/fxcrt/xml/cfx_xmlparser.h" #include "third_party/base/ptr_util.h" @@ -20,12 +21,13 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { auto stream = pdfium::MakeRetain<CFX_MemoryStream>(const_cast<uint8_t*>(data), size, false); - auto root = pdfium::MakeUnique<CFX_XMLElement>(L"ROOT"); - CFX_XMLParser parser(root.get(), stream); - if (!parser.Parse()) + + CFX_XMLParser parser(stream); + std::unique_ptr<CFX_XMLDocument> doc = parser.Parse(); + if (!doc || !doc->GetRoot()) return 0; - for (CFX_XMLNode* pXMLNode = root->GetFirstChild(); pXMLNode; + for (CFX_XMLNode* pXMLNode = doc->GetRoot()->GetFirstChild(); pXMLNode; pXMLNode = pXMLNode->GetNextSibling()) { if (pXMLNode->GetType() == FX_XMLNODE_Element) break; diff --git a/xfa/fxfa/cxfa_ffdoc.cpp b/xfa/fxfa/cxfa_ffdoc.cpp index 3d06c7df27..fb2abe3a72 100644 --- a/xfa/fxfa/cxfa_ffdoc.cpp +++ b/xfa/fxfa/cxfa_ffdoc.cpp @@ -18,6 +18,7 @@ #include "core/fxcrt/cfx_seekablemultistream.h" #include "core/fxcrt/fx_extension.h" #include "core/fxcrt/fx_memory.h" +#include "core/fxcrt/xml/cfx_xmldocument.h" #include "core/fxcrt/xml/cfx_xmlelement.h" #include "core/fxcrt/xml/cfx_xmlnode.h" #include "fxjs/xfa/cjx_object.h" @@ -58,12 +59,11 @@ bool CXFA_FFDoc::ParseDoc(CPDF_Object* pElementXFA) { return false; auto stream = pdfium::MakeRetain<CFX_SeekableMultiStream>(xfaStreams); - CXFA_DocumentParser parser(m_pDocument.get()); if (!parser.Parse(stream, XFA_PacketType::Xdp)) return false; - m_pXMLRoot = parser.GetXMLRoot(); + m_pXMLDoc = parser.GetXMLDoc(); m_pDocument->SetRoot(parser.GetRootNode()); return true; } @@ -146,13 +146,11 @@ void CXFA_FFDoc::CloseDoc() { m_DocView->RunDocClose(); m_DocView.reset(); } - if (m_pDocument) { - m_pDocument->ReleaseXMLNodesIfNeeded(); + if (m_pDocument) m_pDocument->ClearLayoutData(); - } m_pDocument.reset(); - m_pXMLRoot.reset(); + m_pXMLDoc.reset(); m_pNotify.reset(); m_pPDFFontMgr.reset(); m_HashToDibDpiMap.clear(); diff --git a/xfa/fxfa/cxfa_ffdoc.h b/xfa/fxfa/cxfa_ffdoc.h index dacc246378..356a157437 100644 --- a/xfa/fxfa/cxfa_ffdoc.h +++ b/xfa/fxfa/cxfa_ffdoc.h @@ -19,6 +19,7 @@ class CFGAS_PDFFontMgr; class CFX_ChecksumContext; class CFX_DIBitmap; +class CFX_XMLDocument; class CPDF_Document; class CPDF_Object; class CXFA_FFApp; @@ -59,7 +60,7 @@ class CXFA_FFDoc { return m_pDocEnvironment.Get(); } FormType GetFormType() const { return m_FormType; } - + CFX_XMLDocument* GetXMLDocument() const { return m_pXMLDoc.get(); } CXFA_FFDocView* CreateDocView(); @@ -85,7 +86,7 @@ class CXFA_FFDoc { UnownedPtr<IXFA_DocEnvironment> const m_pDocEnvironment; UnownedPtr<CXFA_FFApp> const m_pApp; UnownedPtr<CPDF_Document> m_pPDFDoc; - std::unique_ptr<CFX_XMLNode> m_pXMLRoot; + std::unique_ptr<CFX_XMLDocument> m_pXMLDoc; std::unique_ptr<CXFA_FFNotify> m_pNotify; std::unique_ptr<CXFA_Document> m_pDocument; std::unique_ptr<CXFA_FFDocView> m_DocView; diff --git a/xfa/fxfa/parser/cxfa_document.cpp b/xfa/fxfa/parser/cxfa_document.cpp index 9e5143c5a4..15458826ce 100644 --- a/xfa/fxfa/parser/cxfa_document.cpp +++ b/xfa/fxfa/parser/cxfa_document.cpp @@ -11,7 +11,9 @@ #include "core/fxcrt/fx_extension.h" #include "core/fxcrt/fx_fallthrough.h" +#include "core/fxcrt/xml/cfx_xmldocument.h" #include "fxjs/cfxjse_engine.h" +#include "xfa/fxfa/cxfa_ffdoc.h" #include "xfa/fxfa/cxfa_ffnotify.h" #include "xfa/fxfa/parser/cscript_datawindow.h" #include "xfa/fxfa/parser/cscript_eventpseudomodel.h" @@ -1628,7 +1630,9 @@ void CXFA_Document::DoDataMerge() { CXFA_Node* pDatasetsRoot = ToNode(GetXFAObject(XFA_HASHCODE_Datasets)); if (!pDatasetsRoot) { // Ownership will be passed in the AppendChild below to the XML tree. - auto pDatasetsXMLNode = pdfium::MakeUnique<CFX_XMLElement>(L"xfa:datasets"); + auto* pDatasetsXMLNode = + notify_->GetHDOC()->GetXMLDocument()->CreateNode<CFX_XMLElement>( + L"xfa:datasets"); pDatasetsXMLNode->SetAttribute(L"xmlns:xfa", L"http://www.xfa.org/schema/xfa-data/1.0/"); pDatasetsRoot = @@ -1636,10 +1640,10 @@ void CXFA_Document::DoDataMerge() { pDatasetsRoot->JSObject()->SetCData(XFA_Attribute::Name, L"datasets", false, false); - CFX_XMLElement* ref = pDatasetsXMLNode.get(); - m_pRootNode->GetXMLMappingNode()->AppendChild(std::move(pDatasetsXMLNode)); + m_pRootNode->GetXMLMappingNode()->AppendChild(pDatasetsXMLNode); m_pRootNode->InsertChild(pDatasetsRoot, nullptr); - pDatasetsRoot->SetXMLMappingNode(ref); + + pDatasetsRoot->SetXMLMappingNode(pDatasetsXMLNode); } CXFA_Node *pDataRoot = nullptr, *pDDRoot = nullptr; @@ -1672,8 +1676,11 @@ void CXFA_Document::DoDataMerge() { if (!pDataRoot) { pDataRoot = CreateNode(XFA_PacketType::Datasets, XFA_Element::DataGroup); pDataRoot->JSObject()->SetCData(XFA_Attribute::Name, L"data", false, false); - pDataRoot->SetXMLMappingNode( - pdfium::MakeUnique<CFX_XMLElement>(L"xfa:data")); + + auto* elem = + notify_->GetHDOC()->GetXMLDocument()->CreateNode<CFX_XMLElement>( + L"xfa:data"); + pDataRoot->SetXMLMappingNode(elem); pDatasetsRoot->InsertChild(pDataRoot, nullptr); } @@ -1727,8 +1734,11 @@ void CXFA_Document::DoDataMerge() { CreateNode(XFA_PacketType::Datasets, XFA_Element::DataGroup)); pDataTopLevel->JSObject()->SetCData(XFA_Attribute::Name, wsDataTopLevelName, false, false); - pDataTopLevel->SetXMLMappingNode( - pdfium::MakeUnique<CFX_XMLElement>(wsDataTopLevelName)); + + auto* elem = + notify_->GetHDOC()->GetXMLDocument()->CreateNode<CFX_XMLElement>( + wsDataTopLevelName); + pDataTopLevel->SetXMLMappingNode(elem); CXFA_Node* pBeforeNode = pDataRoot->GetFirstChild(); pDataRoot->InsertChild(pDataTopLevel, pBeforeNode); diff --git a/xfa/fxfa/parser/cxfa_document_parser.cpp b/xfa/fxfa/parser/cxfa_document_parser.cpp index fe246d2300..6f3e56ce4c 100644 --- a/xfa/fxfa/parser/cxfa_document_parser.cpp +++ b/xfa/fxfa/parser/cxfa_document_parser.cpp @@ -110,16 +110,6 @@ CFX_XMLNode* GetDocumentNode(CFX_XMLNode* pRootNode) { return nullptr; } -WideString GetElementTagNamespaceURI(CFX_XMLElement* pElement) { - WideString wsNodeStr = pElement->GetNamespacePrefix(); - WideString wsNamespaceURI; - if (!XFA_FDEExtension_ResolveNamespaceQualifier(pElement, wsNodeStr, - &wsNamespaceURI)) { - return WideString(); - } - return wsNamespaceURI; -} - bool MatchNodeName(CFX_XMLNode* pNode, const WideStringView& wsLocalTagName, const WideStringView& wsNamespaceURIPrefix, @@ -132,7 +122,7 @@ bool MatchNodeName(CFX_XMLNode* pNode, if (wsNodeStr != wsLocalTagName) return false; - wsNodeStr = GetElementTagNamespaceURI(pElement); + wsNodeStr = pElement->GetNamespaceURI(); if (eMatchFlags & XFA_XDPPACKET_FLAGS_NOMATCH) return true; if (eMatchFlags & XFA_XDPPACKET_FLAGS_PREFIXMATCH) { @@ -318,8 +308,8 @@ WideString GetPlainTextFromRichText(CFX_XMLNode* pXMLNode) { } // namespace bool XFA_RecognizeRichText(CFX_XMLElement* pRichTextXMLNode) { - return pRichTextXMLNode && GetElementTagNamespaceURI(pRichTextXMLNode) == - L"http://www.w3.org/1999/xhtml"; + return pRichTextXMLNode && + pRichTextXMLNode->GetNamespaceURI() == L"http://www.w3.org/1999/xhtml"; } CXFA_DocumentParser::CXFA_DocumentParser(CXFA_Document* pFactory) @@ -329,30 +319,38 @@ CXFA_DocumentParser::~CXFA_DocumentParser() = default; bool CXFA_DocumentParser::Parse(const RetainPtr<IFX_SeekableStream>& pStream, XFA_PacketType ePacketID) { - m_pNodeTree = LoadXML(pStream); - if (!m_pNodeTree) + xml_doc_ = LoadXML(pStream); + if (!xml_doc_) + return false; + + CFX_XMLNode* root = GetDocumentNode(xml_doc_->GetRoot()); + if (!root) return false; - m_pRootNode = ParseAsXDPPacket(GetDocumentNode(m_pNodeTree.get()), ePacketID); + m_pRootNode = ParseAsXDPPacket(root, ePacketID); return !!m_pRootNode; } CFX_XMLNode* CXFA_DocumentParser::ParseXMLData(const ByteString& wsXML) { auto pStream = pdfium::MakeRetain<CFX_MemoryStream>( const_cast<uint8_t*>(wsXML.raw_str()), wsXML.GetLength(), false); - m_pNodeTree = LoadXML(pStream); - return m_pNodeTree ? GetDocumentNode(m_pNodeTree.get()) : nullptr; + xml_doc_ = LoadXML(pStream); + if (!xml_doc_) + return nullptr; + return GetDocumentNode(xml_doc_->GetRoot()); } -std::unique_ptr<CFX_XMLNode> CXFA_DocumentParser::LoadXML( +std::unique_ptr<CFX_XMLDocument> CXFA_DocumentParser::LoadXML( const RetainPtr<IFX_SeekableStream>& pStream) { ASSERT(pStream); - auto root = pdfium::MakeUnique<CFX_XMLElement>(L"ROOT"); - root->AppendChild(pdfium::MakeUnique<CFX_XMLInstruction>(L"xml")); - - CFX_XMLParser parser(root.get(), pStream); - return parser.Parse() ? std::move(root) : nullptr; + CFX_XMLParser parser(pStream); + std::unique_ptr<CFX_XMLDocument> doc = parser.Parse(); + if (doc) { + doc->GetRoot()->InsertChildNode(doc->CreateNode<CFX_XMLInstruction>(L"xml"), + 0); + } + return doc; } void CXFA_DocumentParser::ConstructXFANode(CXFA_Node* pXFANode, @@ -461,14 +459,15 @@ CXFA_Node* CXFA_DocumentParser::ParseAsXDPPacket_XDP( CFX_XMLNode* pXMLConfigDOMRoot = nullptr; CXFA_Node* pXFAConfigDOMRoot = nullptr; + const PacketInfo* config_packet_info = + GetPacketByIndex(XFA_PacketType::Config); for (CFX_XMLNode* pChildItem = pXMLDocumentNode->GetFirstChild(); pChildItem; pChildItem = pChildItem->GetNextSibling()) { - const PacketInfo* pPacketInfo = GetPacketByIndex(XFA_PacketType::Config); - if (!MatchNodeName(pChildItem, pPacketInfo->name, pPacketInfo->uri, - pPacketInfo->flags)) { + if (!MatchNodeName(pChildItem, config_packet_info->name, + config_packet_info->uri, config_packet_info->flags)) { continue; } - if (pXFARootNode->GetFirstChildByName(pPacketInfo->hash)) + if (pXFARootNode->GetFirstChildByName(config_packet_info->hash)) return nullptr; pXMLConfigDOMRoot = pChildItem; @@ -516,7 +515,7 @@ CXFA_Node* CXFA_DocumentParser::ParseAsXDPPacket_XDP( if (pXMLTemplateDOMRoot) return nullptr; - CXFA_Node* pPacketNode = ParseAsXDPPacket(pElement, ePacket); + CXFA_Node* pPacketNode = ParseAsXDPPacket_Template(pElement); if (pPacketNode) { pXMLTemplateDOMRoot = pElement; pXFARootNode->InsertChild(pPacketNode, nullptr); @@ -658,13 +657,13 @@ CXFA_Node* CXFA_DocumentParser::ParseAsXDPPacket_Data( return pNode; } - MaybeOwned<CFX_XMLNode> pDataXMLNode; + CFX_XMLNode* pDataXMLNode = nullptr; if (MatchNodeName(pXMLDocumentNode, L"data", packet->uri, packet->flags)) { static_cast<CFX_XMLElement*>(pXMLDocumentNode) ->RemoveAttribute(L"xmlns:xfa"); - pDataXMLNode.Reset(pXMLDocumentNode); + pDataXMLNode = pXMLDocumentNode; } else { - auto pDataElement = pdfium::MakeUnique<CFX_XMLElement>(L"xfa:data"); + auto* pDataElement = xml_doc_->CreateNode<CFX_XMLElement>(L"xfa:data"); CFX_XMLNode* pParentXMLNode = pXMLDocumentNode->GetParent(); if (pParentXMLNode) pParentXMLNode->RemoveChildNode(pXMLDocumentNode); @@ -676,9 +675,8 @@ CXFA_Node* CXFA_DocumentParser::ParseAsXDPPacket_Data( } // The node was either removed from the parent above, or already has no // parent so we can take ownership. - pDataElement->AppendChild( - pdfium::WrapUnique<CFX_XMLNode>(pXMLDocumentNode)); - pDataXMLNode.Reset(std::move(pDataElement)); + pDataElement->AppendChild(pXMLDocumentNode); + pDataXMLNode = pDataElement; } if (!pDataXMLNode) return nullptr; @@ -689,12 +687,12 @@ CXFA_Node* CXFA_DocumentParser::ParseAsXDPPacket_Data( return nullptr; WideString wsLocalName = - static_cast<CFX_XMLElement*>(pDataXMLNode.Get())->GetLocalTagName(); + static_cast<CFX_XMLElement*>(pDataXMLNode)->GetLocalTagName(); pNode->JSObject()->SetCData(XFA_Attribute::Name, wsLocalName, false, false); - if (!DataLoader(pNode, pDataXMLNode.Get(), true)) + if (!DataLoader(pNode, pDataXMLNode, true)) return nullptr; - pNode->SetXMLMappingNode(std::move(pDataXMLNode)); + pNode->SetXMLMappingNode(pDataXMLNode); return pNode; } @@ -915,7 +913,7 @@ void CXFA_DocumentParser::ParseDataGroup(CXFA_Node* pXFANode, case FX_XMLNODE_Element: { CFX_XMLElement* pXMLElement = static_cast<CFX_XMLElement*>(pXMLChild); { - WideString wsNamespaceURI = GetElementTagNamespaceURI(pXMLElement); + WideString wsNamespaceURI = pXMLElement->GetNamespaceURI(); if (wsNamespaceURI == L"http://www.xfa.com/schema/xfa-package/" || wsNamespaceURI == L"http://www.xfa.org/schema/xfa-package/" || wsNamespaceURI == L"http://www.w3.org/2001/XMLSchema-instance") { diff --git a/xfa/fxfa/parser/cxfa_document_parser.h b/xfa/fxfa/parser/cxfa_document_parser.h index d76d5953ac..04ed5abb15 100644 --- a/xfa/fxfa/parser/cxfa_document_parser.h +++ b/xfa/fxfa/parser/cxfa_document_parser.h @@ -10,12 +10,12 @@ #include <memory> #include <utility> +#include "core/fxcrt/xml/cfx_xmldocument.h" #include "core/fxcrt/xml/cfx_xmlnode.h" #include "xfa/fxfa/fxfa_basic.h" class CXFA_Document; class CXFA_Node; -class CFX_XMLDoc; class CFX_XMLInstruction; class IFX_SeekableStream; @@ -30,11 +30,11 @@ class CXFA_DocumentParser { CFX_XMLNode* ParseXMLData(const ByteString& wsXML); void ConstructXFANode(CXFA_Node* pXFANode, CFX_XMLNode* pXMLNode); - std::unique_ptr<CFX_XMLNode> GetXMLRoot() { return std::move(m_pNodeTree); } + std::unique_ptr<CFX_XMLDocument> GetXMLDoc() { return std::move(xml_doc_); } CXFA_Node* GetRootNode() const; private: - std::unique_ptr<CFX_XMLNode> LoadXML( + std::unique_ptr<CFX_XMLDocument> LoadXML( const RetainPtr<IFX_SeekableStream>& pStream); CXFA_Node* ParseAsXDPPacket(CFX_XMLNode* pXMLDocumentNode, @@ -72,7 +72,7 @@ class CXFA_DocumentParser { XFA_PacketType ePacketID); UnownedPtr<CXFA_Document> m_pFactory; - std::unique_ptr<CFX_XMLNode> m_pNodeTree; + std::unique_ptr<CFX_XMLDocument> xml_doc_; // TODO(dsinclair): Figure out who owns this. CXFA_Node* m_pRootNode = nullptr; }; diff --git a/xfa/fxfa/parser/cxfa_document_parser_unittest.cpp b/xfa/fxfa/parser/cxfa_document_parser_unittest.cpp index 9f68fc143b..b150abc3ea 100644 --- a/xfa/fxfa/parser/cxfa_document_parser_unittest.cpp +++ b/xfa/fxfa/parser/cxfa_document_parser_unittest.cpp @@ -17,7 +17,7 @@ class CXFA_DocumentParserTest : public testing::Test { void TearDown() override { // Hold the XML tree until we cleanup the document. - std::unique_ptr<CFX_XMLNode> root = parser_->GetXMLRoot(); + std::unique_ptr<CFX_XMLDocument> doc = parser_->GetXMLDoc(); parser_ = nullptr; doc_ = nullptr; } @@ -30,7 +30,7 @@ class CXFA_DocumentParserTest : public testing::Test { std::unique_ptr<CXFA_DocumentParser> parser_; }; -TEST_F(CXFA_DocumentParserTest, XMLInstructionScriptOff) { +TEST_F(CXFA_DocumentParserTest, XMLInstructionsScriptOff) { const char* input = "<config>\n" "<?originalXFAVersion http://www.xfa.org/schema/xfa-template/2.7 " diff --git a/xfa/fxfa/parser/cxfa_node.cpp b/xfa/fxfa/parser/cxfa_node.cpp index 7a6a2ba7d6..fef6caeb75 100644 --- a/xfa/fxfa/parser/cxfa_node.cpp +++ b/xfa/fxfa/parser/cxfa_node.cpp @@ -539,12 +539,6 @@ CXFA_Node::CXFA_Node(CXFA_Document* pDoc, CXFA_Node::~CXFA_Node() = default; -void CXFA_Node::ReleaseXMLNodeIfUnowned() { - // Note, this is intentionally non-recursive. The caller is responsible for - // triggering this on all the nodes which need it. - xml_node_.ResetIfUnowned(); -} - CXFA_Node* CXFA_Node::Clone(bool bRecursive) { CXFA_Node* pClone = m_pDocument->CreateNode(m_ePacket, m_elementType); if (!pClone) @@ -553,23 +547,36 @@ CXFA_Node* CXFA_Node::Clone(bool bRecursive) { JSObject()->MergeAllData(pClone); pClone->UpdateNameHash(); if (IsNeedSavingXMLNode()) { - std::unique_ptr<CFX_XMLNode> pCloneXML; + CFX_XMLNode* pCloneXML; if (IsAttributeInXML()) { WideString wsName = JSObject() ->TryAttribute(XFA_Attribute::Name, false) .value_or(WideString()); - auto pCloneXMLElement = pdfium::MakeUnique<CFX_XMLElement>(wsName); + auto* pCloneXMLElement = GetDocument() + ->GetNotify() + ->GetHDOC() + ->GetXMLDocument() + ->CreateNode<CFX_XMLElement>(wsName); + WideString wsValue = JSObject()->GetCData(XFA_Attribute::Value); - if (!wsValue.IsEmpty()) - pCloneXMLElement->SetTextData(WideString(wsValue)); + if (!wsValue.IsEmpty()) { + auto* text = GetDocument() + ->GetNotify() + ->GetHDOC() + ->GetXMLDocument() + ->CreateNode<CFX_XMLText>(wsValue); + pCloneXMLElement->AppendChild(text); + } + + pCloneXML = pCloneXMLElement; - pCloneXML.reset(pCloneXMLElement.release()); pClone->JSObject()->SetEnum(XFA_Attribute::Contains, XFA_AttributeEnum::Unknown, false); } else { - pCloneXML = xml_node_->Clone(); + pCloneXML = xml_node_->Clone( + GetDocument()->GetNotify()->GetHDOC()->GetXMLDocument()); } - pClone->SetXMLMappingNode(std::move(pCloneXML)); + pClone->SetXMLMappingNode(pCloneXML); } if (bRecursive) { for (CXFA_Node* pChild = GetFirstChild(); pChild; @@ -1158,8 +1165,7 @@ void CXFA_Node::InsertChild(int32_t index, CXFA_Node* pNode) { return; ASSERT(!pNode->xml_node_->GetParent()); - ASSERT(pNode->xml_node_.IsOwned()); - xml_node_->InsertChildNode(pNode->xml_node_.Release(), index); + xml_node_->InsertChildNode(pNode->xml_node_.Get(), index); } void CXFA_Node::InsertChild(CXFA_Node* pNode, CXFA_Node* pBeforeNode) { @@ -1214,13 +1220,7 @@ void CXFA_Node::RemoveChild(CXFA_Node* pNode, bool bNotify) { return; if (!pNode->IsAttributeInXML()) { - // This XML node _must_ be in the xml tree if we arrived here, so it is - // unowned by the XFA_Node. We turn the nodes pointer into a unique_ptr, - // remove from the XML tree and then assign the owned pointer back onto the - // XFA_Node. - auto node = pdfium::WrapUnique<CFX_XMLNode>(pNode->xml_node_.Get()); - xml_node_->RemoveChildNode(node.get()); - pNode->xml_node_.Reset(std::move(node)); + xml_node_->RemoveChildNode(pNode->xml_node_.Get()); return; } @@ -1237,12 +1237,22 @@ void CXFA_Node::RemoveChild(CXFA_Node* pNode, bool bNotify) { WideString wsName = pNode->JSObject() ->TryAttribute(XFA_Attribute::Name, false) .value_or(WideString()); - auto pNewXMLElement = pdfium::MakeUnique<CFX_XMLElement>(wsName); - WideString wsValue = JSObject()->GetCData(XFA_Attribute::Value); - if (!wsValue.IsEmpty()) - pNewXMLElement->SetTextData(WideString(wsValue)); - pNode->xml_node_.Reset(std::move(pNewXMLElement)); + auto* pNewXMLElement = GetDocument() + ->GetNotify() + ->GetHDOC() + ->GetXMLDocument() + ->CreateNode<CFX_XMLElement>(wsName); + WideString wsValue = JSObject()->GetCData(XFA_Attribute::Value); + if (!wsValue.IsEmpty()) { + auto* text = GetDocument() + ->GetNotify() + ->GetHDOC() + ->GetXMLDocument() + ->CreateNode<CFX_XMLText>(wsValue); + pNewXMLElement->AppendChild(text); + } + pNode->xml_node_ = pNewXMLElement; pNode->JSObject()->SetEnum(XFA_Attribute::Contains, XFA_AttributeEnum::Unknown, false); } @@ -1384,8 +1394,12 @@ void CXFA_Node::UpdateNameHash() { CFX_XMLNode* CXFA_Node::CreateXMLMappingNode() { if (!xml_node_) { - xml_node_ = pdfium::MakeUnique<CFX_XMLElement>( - JSObject()->GetCData(XFA_Attribute::Name)); + xml_node_ = GetDocument() + ->GetNotify() + ->GetHDOC() + ->GetXMLDocument() + ->CreateNode<CFX_XMLElement>( + JSObject()->GetCData(XFA_Attribute::Name)); } return xml_node_.Get(); } @@ -4666,7 +4680,12 @@ void CXFA_Node::SetToXML(const WideString& value) { if (bDeleteChildren) elem->DeleteChildren(); - elem->SetTextData(value); + auto* text = GetDocument() + ->GetNotify() + ->GetHDOC() + ->GetXMLDocument() + ->CreateNode<CFX_XMLText>(value); + elem->AppendChild(text); break; } case FX_XMLNODE_Text: diff --git a/xfa/fxfa/parser/cxfa_node.h b/xfa/fxfa/parser/cxfa_node.h index 11fc38f4cc..db01983a1a 100644 --- a/xfa/fxfa/parser/cxfa_node.h +++ b/xfa/fxfa/parser/cxfa_node.h @@ -157,14 +157,7 @@ class CXFA_Node : public CXFA_Object { return m_ePacket == XFA_PacketType::Form && IsContainerNode(); } - void ReleaseXMLNodeIfUnowned(); - void SetXMLMappingNode(MaybeOwned<CFX_XMLNode> node) { - xml_node_ = std::move(node); - } - void SetXMLMappingNode(std::unique_ptr<CFX_XMLNode> node) { - xml_node_.Reset(std::move(node)); - } - void SetXMLMappingNode(CFX_XMLNode* node) { xml_node_.Reset(node); } + void SetXMLMappingNode(CFX_XMLNode* node) { xml_node_ = node; } CFX_XMLNode* GetXMLMappingNode() const { return xml_node_.Get(); } CFX_XMLNode* CreateXMLMappingNode(); bool IsNeedSavingXMLNode(); @@ -510,7 +503,7 @@ class CXFA_Node : public CXFA_Object { CXFA_Node* first_child_; CXFA_Node* last_child_; - MaybeOwned<CFX_XMLNode> xml_node_; + UnownedPtr<CFX_XMLNode> xml_node_; const XFA_PacketType m_ePacket; uint8_t m_ExecuteRecursionDepth = 0; uint16_t m_uNodeFlags; diff --git a/xfa/fxfa/parser/cxfa_nodeowner.cpp b/xfa/fxfa/parser/cxfa_nodeowner.cpp index dba32dc889..ae6f589616 100644 --- a/xfa/fxfa/parser/cxfa_nodeowner.cpp +++ b/xfa/fxfa/parser/cxfa_nodeowner.cpp @@ -15,19 +15,6 @@ CXFA_NodeOwner::CXFA_NodeOwner() = default; CXFA_NodeOwner::~CXFA_NodeOwner() = default; -void CXFA_NodeOwner::ReleaseXMLNodesIfNeeded() { - // Because we don't know what order we'll free the nodes we may end up - // destroying the XML tree before nodes have been cleaned up that point into - // it. This will cause the ProbeForLowSeverityLifetimeIssue to fire. - // - // This doesn't happen in the destructor because of the ownership semantics - // between the CXFA_Document and CXFA_DocumentParser. It has to happen before - // the simple parser is destroyed, but the document has to live longer then - // the simple parser. - for (auto& it : nodes_) - it->ReleaseXMLNodeIfUnowned(); -} - CXFA_Node* CXFA_NodeOwner::AddOwnedNode(std::unique_ptr<CXFA_Node> node) { if (!node) return nullptr; diff --git a/xfa/fxfa/parser/cxfa_nodeowner.h b/xfa/fxfa/parser/cxfa_nodeowner.h index 329b1e3284..2aaabe4118 100644 --- a/xfa/fxfa/parser/cxfa_nodeowner.h +++ b/xfa/fxfa/parser/cxfa_nodeowner.h @@ -16,8 +16,6 @@ class CXFA_NodeOwner { public: virtual ~CXFA_NodeOwner(); - void ReleaseXMLNodesIfNeeded(); - CXFA_Node* AddOwnedNode(std::unique_ptr<CXFA_Node> node); void FreeOwnedNode(CXFA_Node* node); diff --git a/xfa/fxfa/parser/cxfa_xmllocale.cpp b/xfa/fxfa/parser/cxfa_xmllocale.cpp index ffb4cd9dde..1922b31d7e 100644 --- a/xfa/fxfa/parser/cxfa_xmllocale.cpp +++ b/xfa/fxfa/parser/cxfa_xmllocale.cpp @@ -10,6 +10,7 @@ #include "core/fxcrt/cfx_memorystream.h" #include "core/fxcrt/fx_codepage.h" +#include "core/fxcrt/xml/cfx_xmldocument.h" #include "core/fxcrt/xml/cfx_xmlelement.h" #include "core/fxcrt/xml/cfx_xmlparser.h" #include "xfa/fxfa/parser/cxfa_document.h" @@ -30,18 +31,19 @@ constexpr wchar_t kCurrencySymbol[] = L"currencySymbol"; // static std::unique_ptr<CXFA_XMLLocale> CXFA_XMLLocale::Create( pdfium::span<uint8_t> data) { - auto root = pdfium::MakeUnique<CFX_XMLElement>(L"root"); auto stream = pdfium::MakeRetain<CFX_MemoryStream>(data.data(), data.size(), false); - CFX_XMLParser parser(root.get(), stream); - if (!parser.Parse()) + CFX_XMLParser parser(stream); + auto doc = parser.Parse(); + if (!doc) return nullptr; CFX_XMLElement* locale = nullptr; - for (auto* child = root->GetFirstChild(); child; + for (auto* child = doc->GetRoot()->GetFirstChild(); child; child = child->GetNextSibling()) { if (child->GetType() != FX_XMLNODE_Element) continue; + CFX_XMLElement* elem = static_cast<CFX_XMLElement*>(child); if (elem->GetName() == L"locale") { locale = elem; @@ -50,14 +52,13 @@ std::unique_ptr<CXFA_XMLLocale> CXFA_XMLLocale::Create( } if (!locale) return nullptr; - - return pdfium::MakeUnique<CXFA_XMLLocale>(std::move(root), locale); + return pdfium::MakeUnique<CXFA_XMLLocale>(std::move(doc), locale); } -CXFA_XMLLocale::CXFA_XMLLocale(std::unique_ptr<CFX_XMLElement> root, +CXFA_XMLLocale::CXFA_XMLLocale(std::unique_ptr<CFX_XMLDocument> doc, CFX_XMLElement* locale) - : xml_root_(std::move(root)), locale_(locale) { - ASSERT(xml_root_); + : xml_doc_(std::move(doc)), locale_(locale) { + ASSERT(xml_doc_); ASSERT(locale_); } diff --git a/xfa/fxfa/parser/cxfa_xmllocale.h b/xfa/fxfa/parser/cxfa_xmllocale.h index 757c5349d4..de3de41a90 100644 --- a/xfa/fxfa/parser/cxfa_xmllocale.h +++ b/xfa/fxfa/parser/cxfa_xmllocale.h @@ -13,13 +13,14 @@ #include "third_party/base/ptr_util.h" #include "third_party/base/span.h" +class CFX_XMLDocument; class CFX_XMLElement; class CXFA_XMLLocale : public LocaleIface { public: static std::unique_ptr<CXFA_XMLLocale> Create(pdfium::span<uint8_t> data); - explicit CXFA_XMLLocale(std::unique_ptr<CFX_XMLElement> root, + explicit CXFA_XMLLocale(std::unique_ptr<CFX_XMLDocument> root, CFX_XMLElement* locale); ~CXFA_XMLLocale() override; @@ -49,7 +50,7 @@ class CXFA_XMLLocale : public LocaleIface { size_t index, bool bAbbr) const; - std::unique_ptr<CFX_XMLElement> xml_root_; + std::unique_ptr<CFX_XMLDocument> xml_doc_; UnownedPtr<CFX_XMLElement> locale_; }; |