From f19ae5dcd0b618cdeb80d6a1df5b13610d0ff7da Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Sat, 28 Jul 2018 00:00:24 +0000 Subject: Add ToXML{Instruction,Text,CharData}() checked conversion functions All usages were previously checked correctly, but this consolidates some code as well. Change-Id: I63711748b31b698a3f21f98fdb536db1e9e0b1cf Reviewed-on: https://pdfium-review.googlesource.com/39010 Commit-Queue: Lei Zhang Reviewed-by: Lei Zhang --- core/fxcrt/xml/cfx_xmlchardata.h | 6 ++ core/fxcrt/xml/cfx_xmlchardata_unittest.cpp | 2 +- core/fxcrt/xml/cfx_xmlelement.cpp | 8 +-- core/fxcrt/xml/cfx_xmlelement_unittest.cpp | 2 +- core/fxcrt/xml/cfx_xmlinstruction.h | 6 ++ core/fxcrt/xml/cfx_xmlinstruction_unittest.cpp | 13 ++-- core/fxcrt/xml/cfx_xmlparser.cpp | 8 +-- core/fxcrt/xml/cfx_xmlparser_unittest.cpp | 3 +- core/fxcrt/xml/cfx_xmltext.h | 9 +++ core/fxcrt/xml/cfx_xmltext_unittest.cpp | 2 +- xfa/fxfa/cxfa_textlayout.cpp | 2 +- xfa/fxfa/parser/cxfa_document_parser.cpp | 99 +++++++++++++------------- xfa/fxfa/parser/cxfa_node.cpp | 2 +- 13 files changed, 86 insertions(+), 76 deletions(-) diff --git a/core/fxcrt/xml/cfx_xmlchardata.h b/core/fxcrt/xml/cfx_xmlchardata.h index 013414bc82..9e5669a464 100644 --- a/core/fxcrt/xml/cfx_xmlchardata.h +++ b/core/fxcrt/xml/cfx_xmlchardata.h @@ -25,4 +25,10 @@ class CFX_XMLCharData : public CFX_XMLText { void Save(const RetainPtr& pXMLStream) override; }; +inline CFX_XMLCharData* ToXMLCharData(CFX_XMLNode* pNode) { + return pNode && pNode->GetType() == FX_XMLNODE_CharData + ? static_cast(pNode) + : nullptr; +} + #endif // CORE_FXCRT_XML_CFX_XMLCHARDATA_H_ diff --git a/core/fxcrt/xml/cfx_xmlchardata_unittest.cpp b/core/fxcrt/xml/cfx_xmlchardata_unittest.cpp index 4f0c49eaac..41906edbfd 100644 --- a/core/fxcrt/xml/cfx_xmlchardata_unittest.cpp +++ b/core/fxcrt/xml/cfx_xmlchardata_unittest.cpp @@ -26,7 +26,7 @@ TEST(CFX_XMLCharDataTest, Clone) { EXPECT_TRUE(clone != nullptr); EXPECT_NE(&data, clone); ASSERT_EQ(FX_XMLNODE_CharData, clone->GetType()); - EXPECT_EQ(L"My Data", static_cast(clone)->GetText()); + EXPECT_EQ(L"My Data", ToXMLCharData(clone)->GetText()); } TEST(CFX_XMLCharDataTest, Save) { diff --git a/core/fxcrt/xml/cfx_xmlelement.cpp b/core/fxcrt/xml/cfx_xmlelement.cpp index 4bb4eae1bd..bd24091205 100644 --- a/core/fxcrt/xml/cfx_xmlelement.cpp +++ b/core/fxcrt/xml/cfx_xmlelement.cpp @@ -76,13 +76,11 @@ WideString CFX_XMLElement::GetNamespaceURI() const { WideString CFX_XMLElement::GetTextData() const { CFX_WideTextBuf buffer; - for (CFX_XMLNode* pChild = GetFirstChild(); pChild; pChild = pChild->GetNextSibling()) { - if (pChild->GetType() == FX_XMLNODE_Text || - pChild->GetType() == FX_XMLNODE_CharData) { - buffer << static_cast(pChild)->GetText(); - } + CFX_XMLText* pText = ToXMLText(pChild); + if (pText) + buffer << pText->GetText(); } return buffer.MakeString(); } diff --git a/core/fxcrt/xml/cfx_xmlelement_unittest.cpp b/core/fxcrt/xml/cfx_xmlelement_unittest.cpp index dfc60a10c3..a1c63589ec 100644 --- a/core/fxcrt/xml/cfx_xmlelement_unittest.cpp +++ b/core/fxcrt/xml/cfx_xmlelement_unittest.cpp @@ -104,7 +104,7 @@ TEST(CFX_XMLElementTest, Clone) { EXPECT_TRUE(inst->GetFirstChild()->GetNextSibling() == nullptr); ASSERT_EQ(FX_XMLNODE_Text, inst->GetFirstChild()->GetType()); - auto* text = static_cast(inst->GetFirstChild()); + auto* text = ToXMLText(inst->GetFirstChild()); EXPECT_EQ(L"Text Child", text->GetText()); } diff --git a/core/fxcrt/xml/cfx_xmlinstruction.h b/core/fxcrt/xml/cfx_xmlinstruction.h index 153ef2b625..8962d73ac1 100644 --- a/core/fxcrt/xml/cfx_xmlinstruction.h +++ b/core/fxcrt/xml/cfx_xmlinstruction.h @@ -36,4 +36,10 @@ class CFX_XMLInstruction : public CFX_XMLNode { std::vector m_TargetData; }; +inline CFX_XMLInstruction* ToXMLInstruction(CFX_XMLNode* pNode) { + return pNode && pNode->GetType() == FX_XMLNODE_Instruction + ? static_cast(pNode) + : nullptr; +} + #endif // CORE_FXCRT_XML_CFX_XMLINSTRUCTION_H_ diff --git a/core/fxcrt/xml/cfx_xmlinstruction_unittest.cpp b/core/fxcrt/xml/cfx_xmlinstruction_unittest.cpp index b25a18be9c..0109ab2985 100644 --- a/core/fxcrt/xml/cfx_xmlinstruction_unittest.cpp +++ b/core/fxcrt/xml/cfx_xmlinstruction_unittest.cpp @@ -53,8 +53,7 @@ TEST(CFX_XMLInstructionTest, Clone) { EXPECT_TRUE(clone != nullptr); ASSERT_EQ(FX_XMLNODE_Instruction, clone->GetType()); - CFX_XMLInstruction* inst = static_cast(clone); - + CFX_XMLInstruction* inst = ToXMLInstruction(clone); EXPECT_TRUE(inst->IsAcrobat()); auto& data = inst->GetTargetData(); @@ -98,8 +97,7 @@ TEST(CFX_XMLInstructionTest, ParseAndReSave) { ASSERT_TRUE(root->GetFirstChild() != nullptr); ASSERT_EQ(FX_XMLNODE_Instruction, root->GetFirstChild()->GetType()); - CFX_XMLInstruction* node = - static_cast(root->GetFirstChild()); + CFX_XMLInstruction* node = ToXMLInstruction(root->GetFirstChild()); ASSERT_TRUE(node != nullptr); EXPECT_TRUE(node->IsAcrobat()); @@ -136,12 +134,9 @@ TEST(CFX_XMLInstructionTest, ParseAndReSaveInnerInstruction) { EXPECT_EQ(L"node", node->GetName()); CFX_XMLInstruction* instruction = nullptr; - for (auto* elem = node->GetFirstChild(); elem; + for (auto* elem = node->GetFirstChild(); elem && !instruction; elem = elem->GetNextSibling()) { - if (elem->GetType() == FX_XMLNODE_Instruction) { - instruction = static_cast(elem); - break; - } + instruction = ToXMLInstruction(elem); } ASSERT_TRUE(instruction != nullptr); EXPECT_TRUE(instruction->IsAcrobat()); diff --git a/core/fxcrt/xml/cfx_xmlparser.cpp b/core/fxcrt/xml/cfx_xmlparser.cpp index dd28cf8adc..094daac889 100644 --- a/core/fxcrt/xml/cfx_xmlparser.cpp +++ b/core/fxcrt/xml/cfx_xmlparser.cpp @@ -518,12 +518,10 @@ void CFX_XMLParser::ProcessTargetData() { WideString target_data = GetTextData(); if (target_data.IsEmpty()) return; - if (!current_node_) - return; - if (current_node_->GetType() != FX_XMLNODE_Instruction) - return; - static_cast(current_node_)->AppendData(target_data); + CFX_XMLInstruction* instruction = ToXMLInstruction(current_node_); + if (instruction) + instruction->AppendData(target_data); } WideString CFX_XMLParser::GetTextData() { diff --git a/core/fxcrt/xml/cfx_xmlparser_unittest.cpp b/core/fxcrt/xml/cfx_xmlparser_unittest.cpp index 300131db95..c1236ce182 100644 --- a/core/fxcrt/xml/cfx_xmlparser_unittest.cpp +++ b/core/fxcrt/xml/cfx_xmlparser_unittest.cpp @@ -325,8 +325,7 @@ TEST_F(CFX_XMLParserTest, ParseInstruction) { ASSERT_TRUE(root->GetFirstChild() != nullptr); ASSERT_EQ(FX_XMLNODE_Instruction, root->GetFirstChild()->GetType()); - CFX_XMLInstruction* instruction = - static_cast(root->GetFirstChild()); + CFX_XMLInstruction* instruction = ToXMLInstruction(root->GetFirstChild()); EXPECT_TRUE(instruction->IsOriginalXFAVersion()); } diff --git a/core/fxcrt/xml/cfx_xmltext.h b/core/fxcrt/xml/cfx_xmltext.h index c5c168059d..f7feae669e 100644 --- a/core/fxcrt/xml/cfx_xmltext.h +++ b/core/fxcrt/xml/cfx_xmltext.h @@ -31,4 +31,13 @@ class CFX_XMLText : public CFX_XMLNode { WideString m_wsText; }; +inline bool IsXMLText(CFX_XMLNode* pNode) { + FX_XMLNODETYPE type = pNode->GetType(); + return type == FX_XMLNODE_Text || type == FX_XMLNODE_CharData; +} + +inline CFX_XMLText* ToXMLText(CFX_XMLNode* pNode) { + return pNode && IsXMLText(pNode) ? static_cast(pNode) : nullptr; +} + #endif // CORE_FXCRT_XML_CFX_XMLTEXT_H_ diff --git a/core/fxcrt/xml/cfx_xmltext_unittest.cpp b/core/fxcrt/xml/cfx_xmltext_unittest.cpp index 6e53f66612..04b1ee8571 100644 --- a/core/fxcrt/xml/cfx_xmltext_unittest.cpp +++ b/core/fxcrt/xml/cfx_xmltext_unittest.cpp @@ -25,7 +25,7 @@ TEST(CFX_XMLTextTest, 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(clone)->GetText()); + EXPECT_EQ(L"My Data", ToXMLText(clone)->GetText()); } TEST(CFX_XMLTextTest, Save) { diff --git a/xfa/fxfa/cxfa_textlayout.cpp b/xfa/fxfa/cxfa_textlayout.cpp index 8bf955bd32..4463a34097 100644 --- a/xfa/fxfa/cxfa_textlayout.cpp +++ b/xfa/fxfa/cxfa_textlayout.cpp @@ -756,7 +756,7 @@ bool CXFA_TextLayout::LoadRichText( bContentNode ? pParentStyle.Get() : pStyle.Get()); WideString wsText; if (bContentNode && iTabCount == 0) { - wsText = static_cast(pXMLNode)->GetText(); + wsText = ToXMLText(pXMLNode)->GetText(); } else if (wsName == L"br") { wsText = L'\n'; } else if (wsName == L"li") { diff --git a/xfa/fxfa/parser/cxfa_document_parser.cpp b/xfa/fxfa/parser/cxfa_document_parser.cpp index 34955829e9..96c031079b 100644 --- a/xfa/fxfa/parser/cxfa_document_parser.cpp +++ b/xfa/fxfa/parser/cxfa_document_parser.cpp @@ -243,18 +243,16 @@ void ConvertXMLToPlainText(CFX_XMLElement* pRootXMLNode, WideString& wsOutput) { pXMLChild = pXMLChild->GetNextSibling()) { switch (pXMLChild->GetType()) { case FX_XMLNODE_Element: { - WideString wsTextData = - static_cast(pXMLChild)->GetTextData(); + WideString wsTextData = ToXMLElement(pXMLChild)->GetTextData(); wsTextData += L"\n"; wsOutput += wsTextData; break; } case FX_XMLNODE_Text: case FX_XMLNODE_CharData: { - WideString wsText = static_cast(pXMLChild)->GetText(); + WideString wsText = ToXMLText(pXMLChild)->GetText(); if (IsStringAllWhitespace(wsText)) continue; - wsOutput = std::move(wsText); break; } @@ -291,7 +289,7 @@ WideString GetPlainTextFromRichText(CFX_XMLNode* pXMLNode) { } case FX_XMLNODE_Text: case FX_XMLNODE_CharData: { - WideString wsContent = static_cast(pXMLNode)->GetText(); + WideString wsContent = ToXMLText(pXMLNode)->GetText(); wsPlainText += wsContent; break; } @@ -837,8 +835,7 @@ CXFA_Node* CXFA_DocumentParser::NormalLoader(CXFA_Node* pXFANode, } } break; case FX_XMLNODE_Instruction: - ParseInstruction(pXFANode, static_cast(pXMLChild), - ePacketID); + ParseInstruction(pXFANode, ToXMLInstruction(pXMLChild), ePacketID); break; default: break; @@ -882,8 +879,9 @@ void CXFA_DocumentParser::ParseContentNode(CXFA_Node* pXFANode, } else { if (pElement) break; - if (eNodeType == FX_XMLNODE_Text || eNodeType == FX_XMLNODE_CharData) - wsValue = static_cast(pXMLChild)->GetText(); + CFX_XMLText* pText = ToXMLText(pXMLChild); + if (pText) + wsValue = pText->GetText(); } break; } @@ -1011,7 +1009,7 @@ void CXFA_DocumentParser::ParseDataGroup(CXFA_Node* pXFANode, } case FX_XMLNODE_CharData: case FX_XMLNODE_Text: { - CFX_XMLText* pXMLText = static_cast(pXMLChild); + CFX_XMLText* pXMLText = ToXMLText(pXMLChild); WideString wsText = pXMLText->GetText(); if (IsStringAllWhitespace(wsText)) continue; @@ -1047,57 +1045,58 @@ void CXFA_DocumentParser::ParseDataValue(CXFA_Node* pXFANode, if (eNodeType == FX_XMLNODE_Instruction) continue; - if (eNodeType == FX_XMLNODE_Text || eNodeType == FX_XMLNODE_CharData) { - WideString wsText = static_cast(pXMLChild)->GetText(); + CFX_XMLText* pText = ToXMLText(pXMLChild); + if (pText) { + WideString wsText = pText->GetText(); if (!pXMLCurValueNode) pXMLCurValueNode = pXMLChild; - wsCurValueTextBuf << wsText; - } else if (XFA_RecognizeRichText(ToXMLElement(pXMLChild))) { + continue; + } + if (XFA_RecognizeRichText(ToXMLElement(pXMLChild))) { WideString wsText = GetPlainTextFromRichText(ToXMLElement(pXMLChild)); if (!pXMLCurValueNode) pXMLCurValueNode = pXMLChild; - wsCurValueTextBuf << wsText; - } else { - bMarkAsCompound = true; - if (pXMLCurValueNode) { - WideString wsCurValue = wsCurValueTextBuf.MakeString(); - if (!wsCurValue.IsEmpty()) { - CXFA_Node* pXFAChild = - m_pFactory->CreateNode(ePacketID, XFA_Element::DataValue); - if (!pXFAChild) - return; + continue; + } + bMarkAsCompound = true; + if (pXMLCurValueNode) { + WideString wsCurValue = wsCurValueTextBuf.MakeString(); + if (!wsCurValue.IsEmpty()) { + CXFA_Node* pXFAChild = + m_pFactory->CreateNode(ePacketID, XFA_Element::DataValue); + if (!pXFAChild) + return; - pXFAChild->JSObject()->SetCData(XFA_Attribute::Name, L"", false, - false); - pXFAChild->JSObject()->SetCData(XFA_Attribute::Value, wsCurValue, - false, false); - pXFANode->InsertChild(pXFAChild, nullptr); - pXFAChild->SetXMLMappingNode(pXMLCurValueNode); - pXFAChild->SetFlag(XFA_NodeFlag_Initialized); - wsValueTextBuf << wsCurValue; - wsCurValueTextBuf.Clear(); - } - pXMLCurValueNode = nullptr; + pXFAChild->JSObject()->SetCData(XFA_Attribute::Name, L"", false, false); + pXFAChild->JSObject()->SetCData(XFA_Attribute::Value, wsCurValue, false, + false); + pXFANode->InsertChild(pXFAChild, nullptr); + pXFAChild->SetXMLMappingNode(pXMLCurValueNode); + pXFAChild->SetFlag(XFA_NodeFlag_Initialized); + wsValueTextBuf << wsCurValue; + wsCurValueTextBuf.Clear(); } - CXFA_Node* pXFAChild = - m_pFactory->CreateNode(ePacketID, XFA_Element::DataValue); - if (!pXFAChild) - return; - - WideString wsNodeStr = ToXMLElement(pXMLChild)->GetLocalTagName(); - pXFAChild->JSObject()->SetCData(XFA_Attribute::Name, wsNodeStr, false, - false); - ParseDataValue(pXFAChild, pXMLChild, ePacketID); - pXFANode->InsertChild(pXFAChild, nullptr); - pXFAChild->SetXMLMappingNode(pXMLChild); - pXFAChild->SetFlag(XFA_NodeFlag_Initialized); - WideString wsCurValue = - pXFAChild->JSObject()->GetCData(XFA_Attribute::Value); - wsValueTextBuf << wsCurValue; + pXMLCurValueNode = nullptr; } + CXFA_Node* pXFAChild = + m_pFactory->CreateNode(ePacketID, XFA_Element::DataValue); + if (!pXFAChild) + return; + + WideString wsNodeStr = ToXMLElement(pXMLChild)->GetLocalTagName(); + pXFAChild->JSObject()->SetCData(XFA_Attribute::Name, wsNodeStr, false, + false); + ParseDataValue(pXFAChild, pXMLChild, ePacketID); + pXFANode->InsertChild(pXFAChild, nullptr); + pXFAChild->SetXMLMappingNode(pXMLChild); + pXFAChild->SetFlag(XFA_NodeFlag_Initialized); + WideString wsCurValue = + pXFAChild->JSObject()->GetCData(XFA_Attribute::Value); + wsValueTextBuf << wsCurValue; } + if (pXMLCurValueNode) { WideString wsCurValue = wsCurValueTextBuf.MakeString(); if (!wsCurValue.IsEmpty()) { diff --git a/xfa/fxfa/parser/cxfa_node.cpp b/xfa/fxfa/parser/cxfa_node.cpp index 9fdea14cae..3fe4f915bc 100644 --- a/xfa/fxfa/parser/cxfa_node.cpp +++ b/xfa/fxfa/parser/cxfa_node.cpp @@ -4730,7 +4730,7 @@ void CXFA_Node::SetToXML(const WideString& value) { break; } case FX_XMLNODE_Text: - static_cast(GetXMLMappingNode())->SetText(value); + ToXMLText(GetXMLMappingNode())->SetText(value); break; default: NOTREACHED(); -- cgit v1.2.3