From f351ba03ebf31103c0a6a0c00b1477d39c060139 Mon Sep 17 00:00:00 2001 From: Dan Sinclair Date: Mon, 27 Nov 2017 18:28:06 +0000 Subject: Add some helpers for attribute lookup This CL adds helpers to CXFA_Node to convert from strings to attributes and from attributes to their string names. A static_assert was added to make sure the list of attributes is the same size as the attribute data so the checks can be removed. Change-Id: Idebc65021d71f604bcf498e4cf42252af00d802b Reviewed-on: https://pdfium-review.googlesource.com/19270 Reviewed-by: Henrique Nakashima Commit-Queue: dsinclair --- BUILD.gn | 1 + fxjs/cjx_node.cpp | 35 +++++++++------------------ xfa/fxfa/fxfa_basic.h | 2 +- xfa/fxfa/parser/cxfa_dataexporter.cpp | 16 ++++++------ xfa/fxfa/parser/cxfa_node.cpp | 13 ++++++++++ xfa/fxfa/parser/cxfa_node.h | 2 ++ xfa/fxfa/parser/cxfa_node_unittest.cpp | 17 +++++++++++++ xfa/fxfa/parser/cxfa_simple_parser.cpp | 14 +++++------ xfa/fxfa/parser/xfa_basic_data_attributes.cpp | 3 +++ xfa/fxfa/parser/xfa_utils.cpp | 5 ++-- 10 files changed, 65 insertions(+), 43 deletions(-) create mode 100644 xfa/fxfa/parser/cxfa_node_unittest.cpp diff --git a/BUILD.gn b/BUILD.gn index 167872a4af..228271d534 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -2637,6 +2637,7 @@ test("pdfium_unittests") { "xfa/fxfa/fm2js/cxfa_fmlexer_unittest.cpp", "xfa/fxfa/fm2js/cxfa_fmparser_unittest.cpp", "xfa/fxfa/fm2js/cxfa_fmsimpleexpression_unittest.cpp", + "xfa/fxfa/parser/cxfa_node_unittest.cpp", "xfa/fxfa/parser/cxfa_nodeiteratortemplate_unittest.cpp", "xfa/fxfa/parser/xfa_utils_unittest.cpp", ] diff --git a/fxjs/cjx_node.cpp b/fxjs/cjx_node.cpp index 5e00d525db..f01ead60a9 100644 --- a/fxjs/cjx_node.cpp +++ b/fxjs/cjx_node.cpp @@ -191,8 +191,6 @@ bool CJX_Node::SetAttribute(XFA_Attribute eAttr, const WideStringView& wsValue, bool bNotify) { const XFA_ATTRIBUTEINFO* pAttr = XFA_GetAttributeByID(eAttr); - if (!pAttr) - return false; XFA_AttributeType eType = pAttr->eType; if (eType == XFA_AttributeType::NotSure) { @@ -228,9 +226,9 @@ bool CJX_Node::SetAttribute(XFA_Attribute eAttr, bool CJX_Node::SetAttribute(const WideStringView& wsAttr, const WideStringView& wsValue, bool bNotify) { - const XFA_ATTRIBUTEINFO* pAttributeInfo = XFA_GetAttributeByName(wsValue); - if (pAttributeInfo) - return SetAttribute(pAttributeInfo->eName, wsValue, bNotify); + XFA_Attribute attr = CXFA_Node::NameToAttribute(wsValue); + if (attr != XFA_Attribute::Unknown) + return SetAttribute(attr, wsValue, bNotify); void* pKey = GetMapKey_Custom(wsAttr); SetMapModuleString(pKey, wsValue); @@ -248,8 +246,6 @@ WideString CJX_Node::GetAttribute(XFA_Attribute attr) { pdfium::Optional CJX_Node::TryAttribute(XFA_Attribute eAttr, bool bUseDefault) { const XFA_ATTRIBUTEINFO* pAttr = XFA_GetAttributeByID(eAttr); - if (!pAttr) - return {}; XFA_AttributeType eType = pAttr->eType; if (eType == XFA_AttributeType::NotSure) { @@ -298,9 +294,9 @@ pdfium::Optional CJX_Node::TryAttribute(XFA_Attribute eAttr, pdfium::Optional CJX_Node::TryAttribute( const WideStringView& wsAttr, bool bUseDefault) { - const XFA_ATTRIBUTEINFO* pAttributeInfo = XFA_GetAttributeByName(wsAttr); - if (pAttributeInfo) - return TryAttribute(pAttributeInfo->eName, bUseDefault); + XFA_Attribute attr = CXFA_Node::NameToAttribute(wsAttr); + if (attr != XFA_Attribute::Unknown) + return TryAttribute(attr, bUseDefault); void* pKey = GetMapKey_Custom(wsAttr); WideStringView wsValueC; @@ -687,9 +683,8 @@ void CJX_Node::Script_NodeClass_IsPropertySpecified( WideString wsExpression = WideString::FromUTF8(pArguments->GetUTF8String(0).AsStringView()); - const XFA_ATTRIBUTEINFO* pAttributeInfo = - XFA_GetAttributeByName(wsExpression.AsStringView()); - if (pAttributeInfo && HasAttribute(pAttributeInfo->eName)) { + XFA_Attribute attr = CXFA_Node::NameToAttribute(wsExpression.AsStringView()); + if (attr != XFA_Attribute::Unknown && HasAttribute(attr)) { pValue->SetBoolean(true); return; } @@ -701,7 +696,7 @@ void CJX_Node::Script_NodeClass_IsPropertySpecified( if (!bHas && bParent && GetXFANode()->GetParent()) { // Also check on the parent. auto* jsnode = GetXFANode()->GetParent()->JSNode(); - bHas = jsnode->HasAttribute(pAttributeInfo->eName) || + bHas = jsnode->HasAttribute(attr) || !!jsnode->GetProperty(iIndex, eType, true); } pValue->SetBoolean(bHas); @@ -3125,9 +3120,6 @@ bool CJX_Node::SetCData(XFA_Attribute eAttr, } const XFA_ATTRIBUTEINFO* pInfo = XFA_GetAttributeByID(eAttr); - if (!pInfo) - return true; - ASSERT(elem->GetType() == FX_XMLNODE_Element); WideString wsAttrName = pInfo->pName; if (pInfo->eName == XFA_Attribute::ContentType) @@ -3223,22 +3215,19 @@ bool CJX_Node::SetValue(XFA_Attribute eAttr, auto* elem = static_cast(GetXFANode()->GetXMLMappingNode()); ASSERT(elem->GetType() == FX_XMLNODE_Element); - const XFA_ATTRIBUTEINFO* pInfo = XFA_GetAttributeByID(eAttr); - if (!pInfo) - return true; switch (eType) { case XFA_AttributeType::Enum: elem->SetString( - pInfo->pName, + CXFA_Node::AttributeToName(eAttr), GetAttributeEnumByID((XFA_ATTRIBUTEENUM)(uintptr_t)pValue)->pName); break; case XFA_AttributeType::Boolean: - elem->SetString(pInfo->pName, pValue ? L"1" : L"0"); + elem->SetString(CXFA_Node::AttributeToName(eAttr), pValue ? L"1" : L"0"); break; case XFA_AttributeType::Integer: { elem->SetString( - pInfo->pName, + CXFA_Node::AttributeToName(eAttr), WideString::Format(L"%d", static_cast( reinterpret_cast(pValue)))); break; diff --git a/xfa/fxfa/fxfa_basic.h b/xfa/fxfa/fxfa_basic.h index e2d4056095..cdd53a965b 100644 --- a/xfa/fxfa/fxfa_basic.h +++ b/xfa/fxfa/fxfa_basic.h @@ -618,7 +618,7 @@ enum class XFA_Attribute : uint8_t { Intact, XdpContent, DecipherOnly, - + LastAttributePlaceholder, Unknown = 255, }; diff --git a/xfa/fxfa/parser/cxfa_dataexporter.cpp b/xfa/fxfa/parser/cxfa_dataexporter.cpp index f28649fffa..edb7e1e186 100644 --- a/xfa/fxfa/parser/cxfa_dataexporter.cpp +++ b/xfa/fxfa/parser/cxfa_dataexporter.cpp @@ -87,7 +87,7 @@ WideString ExportEncodeContent(const WideString& str) { void SaveAttribute(CXFA_Node* pNode, XFA_Attribute eName, - const WideStringView& wsName, + const WideString& wsName, bool bProto, WideString& wsOutput) { if (!bProto && !pNode->JSNode()->HasAttribute(eName)) @@ -175,13 +175,13 @@ void RegenerateFormFile_Changed(CXFA_Node* pNode, if (attr == XFA_Attribute::Unknown) break; - const XFA_ATTRIBUTEINFO* pAttr = XFA_GetAttributeByID(attr); - if (pAttr->eName == XFA_Attribute::Name || - (AttributeSaveInDataModel(pNode, pAttr->eName) && !bSaveXML)) { + if (attr == XFA_Attribute::Name || + (AttributeSaveInDataModel(pNode, attr) && !bSaveXML)) { continue; } WideString wsAttr; - SaveAttribute(pNode, pAttr->eName, pAttr->pName, bSaveXML, wsAttr); + SaveAttribute(pNode, attr, CXFA_Node::AttributeToName(attr), bSaveXML, + wsAttr); wsAttrs += wsAttr; } @@ -360,13 +360,11 @@ void RegenerateFormFile_Container( XFA_Attribute attr = pNode->GetAttribute(i); if (attr == XFA_Attribute::Unknown) break; - - const XFA_ATTRIBUTEINFO* pAttr = XFA_GetAttributeByID(attr); - if (pAttr->eName == XFA_Attribute::Name) + if (attr == XFA_Attribute::Name) continue; WideString wsAttr; - SaveAttribute(pNode, pAttr->eName, pAttr->pName, false, wsAttr); + SaveAttribute(pNode, attr, CXFA_Node::AttributeToName(attr), false, wsAttr); wsOutput += wsAttr; } diff --git a/xfa/fxfa/parser/cxfa_node.cpp b/xfa/fxfa/parser/cxfa_node.cpp index ccbf31048d..4402208900 100644 --- a/xfa/fxfa/parser/cxfa_node.cpp +++ b/xfa/fxfa/parser/cxfa_node.cpp @@ -138,6 +138,19 @@ const XFA_ATTRIBUTEENUMINFO* GetAttributeEnumByID(XFA_ATTRIBUTEENUM eName) { return g_XFAEnumData + eName; } +// static +WideString CXFA_Node::AttributeToName(XFA_Attribute attr) { + return XFA_GetAttributeByID(attr)->pName; +} + +// static +XFA_Attribute CXFA_Node::NameToAttribute(const WideStringView& name) { + const XFA_ATTRIBUTEINFO* attr = XFA_GetAttributeByName(name); + if (!attr) + return XFA_Attribute::Unknown; + return attr->eName; +} + CXFA_Node::CXFA_Node(CXFA_Document* pDoc, uint16_t ePacket, uint32_t validPackets, diff --git a/xfa/fxfa/parser/cxfa_node.h b/xfa/fxfa/parser/cxfa_node.h index 7b7086ecb5..e5f47a1828 100644 --- a/xfa/fxfa/parser/cxfa_node.h +++ b/xfa/fxfa/parser/cxfa_node.h @@ -53,6 +53,8 @@ class CXFA_Node : public CXFA_Object { uint8_t flags; }; + static WideString AttributeToName(XFA_Attribute attr); + static XFA_Attribute NameToAttribute(const WideStringView& name); static XFA_Element NameToElement(const WideString& name); static std::unique_ptr Create(CXFA_Document* doc, XFA_Element element, diff --git a/xfa/fxfa/parser/cxfa_node_unittest.cpp b/xfa/fxfa/parser/cxfa_node_unittest.cpp new file mode 100644 index 0000000000..93df084391 --- /dev/null +++ b/xfa/fxfa/parser/cxfa_node_unittest.cpp @@ -0,0 +1,17 @@ +// Copyright 2017 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 "xfa/fxfa/parser/cxfa_node.h" + +#include "testing/gtest/include/gtest/gtest.h" +#include "testing/test_support.h" + +TEST(CXFA_NodeTest, NameToAttribute) { + EXPECT_EQ(XFA_Attribute::Unknown, CXFA_Node::NameToAttribute(L"")); + EXPECT_EQ(XFA_Attribute::Unknown, CXFA_Node::NameToAttribute(L"nonesuch")); + EXPECT_EQ(XFA_Attribute::H, CXFA_Node::NameToAttribute(L"h")); + EXPECT_EQ(XFA_Attribute::Short, CXFA_Node::NameToAttribute(L"short")); + EXPECT_EQ(XFA_Attribute::DecipherOnly, + CXFA_Node::NameToAttribute(L"decipherOnly")); +} diff --git a/xfa/fxfa/parser/cxfa_simple_parser.cpp b/xfa/fxfa/parser/cxfa_simple_parser.cpp index 4ee9ca76b3..453ae8d059 100644 --- a/xfa/fxfa/parser/cxfa_simple_parser.cpp +++ b/xfa/fxfa/parser/cxfa_simple_parser.cpp @@ -898,17 +898,17 @@ CXFA_Node* CXFA_SimpleParser::NormalLoader(CXFA_Node* pXFANode, if (wsAttrName == L"nil" && it.second == L"true") IsNeedValue = false; - const XFA_ATTRIBUTEINFO* lpAttrInfo = - XFA_GetAttributeByName(wsAttrName.AsStringView()); - if (!lpAttrInfo) + XFA_Attribute attr = + CXFA_Node::NameToAttribute(wsAttrName.AsStringView()); + if (attr == XFA_Attribute::Unknown) continue; - if (!bUseAttribute && lpAttrInfo->eName != XFA_Attribute::Name && - lpAttrInfo->eName != XFA_Attribute::Save) { + if (!bUseAttribute && attr != XFA_Attribute::Name && + attr != XFA_Attribute::Save) { continue; } - pXFAChild->JSNode()->SetAttribute(lpAttrInfo->eName, - it.second.AsStringView(), false); + pXFAChild->JSNode()->SetAttribute(attr, it.second.AsStringView(), + false); } pXFANode->InsertChild(pXFAChild, nullptr); if (eType == XFA_Element::Validate || eType == XFA_Element::Locale) { diff --git a/xfa/fxfa/parser/xfa_basic_data_attributes.cpp b/xfa/fxfa/parser/xfa_basic_data_attributes.cpp index 66f5c893ba..7b371a0239 100644 --- a/xfa/fxfa/parser/xfa_basic_data_attributes.cpp +++ b/xfa/fxfa/parser/xfa_basic_data_attributes.cpp @@ -704,6 +704,9 @@ const XFA_ATTRIBUTEINFO g_XFAAttributeData[] = { }; const int32_t g_iXFAAttributeCount = sizeof(g_XFAAttributeData) / sizeof(XFA_ATTRIBUTEINFO); +static_assert(g_iXFAAttributeCount == + static_cast(XFA_Attribute::LastAttributePlaceholder), + "Attribute count mismatch"); const XFA_NOTSUREATTRIBUTE g_XFANotsureAttributes[] = { {XFA_Element::SubformSet, XFA_Attribute::Relation, XFA_AttributeType::Enum, diff --git a/xfa/fxfa/parser/xfa_utils.cpp b/xfa/fxfa/parser/xfa_utils.cpp index b17a7535eb..03b8943a1f 100644 --- a/xfa/fxfa/parser/xfa_utils.cpp +++ b/xfa/fxfa/parser/xfa_utils.cpp @@ -284,9 +284,8 @@ const XFA_ATTRIBUTEINFO* XFA_GetAttributeByName(const WideStringView& wsName) { } const XFA_ATTRIBUTEINFO* XFA_GetAttributeByID(XFA_Attribute eName) { - return (static_cast(eName) < g_iXFAAttributeCount) - ? (g_XFAAttributeData + static_cast(eName)) - : nullptr; + ASSERT(static_cast(eName) < g_iXFAAttributeCount); + return g_XFAAttributeData + static_cast(eName); } const XFA_ATTRIBUTEENUMINFO* XFA_GetAttributeEnumByName( -- cgit v1.2.3