Move packet info from cxfa_document_parser.cpp to xfa_basic_data.cpp. Put it in the same file as the other generated table. Introduce XFA_PACKETINFO struct to separate the results of a query from the underlying table itself. Omit the hash from this struct since it is an implementation detail that may someday go away. This requires us to rehash a few strings, but the code that it calls with this hash should someday get refactored to take a saner argument. When looking up packets by name, do a final string comparison instead of a hash comparison to avoid aliasing alternate forms of user input to these names. Add unit tests for API. Change-Id: I7659ba79058591e1c0a4417c318cdcb696ba37c5 Reviewed-on: https://pdfium-review.googlesource.com/c/47231 Reviewed-by: Lei Zhang <thestig@chromium.org> Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/xfa/fxfa/parser/cxfa_document_parser.cpp b/xfa/fxfa/parser/cxfa_document_parser.cpp index 8c79bab..861a789 100644 --- a/xfa/fxfa/parser/cxfa_document_parser.cpp +++ b/xfa/fxfa/parser/cxfa_document_parser.cpp
@@ -33,40 +33,6 @@ namespace { -struct PacketInfo { - uint32_t hash; - const wchar_t* name; - XFA_PacketType packet_type; - const wchar_t* uri; - uint32_t flags; -}; - -const PacketInfo PacketData[] = { -#undef PCKT____ -#define PCKT____(a, b, c, d, e, f) \ - {a, L##b, XFA_PacketType::c, d, \ - XFA_XDPPACKET_FLAGS_##e | XFA_XDPPACKET_FLAGS_##f}, -#include "xfa/fxfa/parser/packets.inc" -#undef PCKT____ -}; - -const PacketInfo* GetPacketByIndex(XFA_PacketType ePacket) { - return PacketData + static_cast<uint8_t>(ePacket); -} - -const PacketInfo* GetPacketByName(const WideStringView& wsName) { - if (wsName.IsEmpty()) - return nullptr; - - uint32_t hash = FX_HashCode_GetW(wsName, false); - auto* elem = std::lower_bound( - std::begin(PacketData), std::end(PacketData), hash, - [](const PacketInfo& a, uint32_t hash) { return a.hash < hash; }); - if (elem != std::end(PacketData) && elem->hash == hash) - return elem; - return nullptr; -} - CFX_XMLNode* GetDocumentNode(CFX_XMLNode* pRootNode) { for (CFX_XMLNode* pXMLNode = pRootNode->GetFirstChild(); pXMLNode; pXMLNode = pXMLNode->GetNextSibling()) { @@ -177,23 +143,21 @@ } CFX_XMLNode* GetDataSetsFromXDP(CFX_XMLNode* pXMLDocumentNode) { - const PacketInfo* datasets_packet = - GetPacketByIndex(XFA_PacketType::Datasets); - if (MatchNodeName(pXMLDocumentNode, datasets_packet->name, - datasets_packet->uri, datasets_packet->flags)) { + XFA_PACKETINFO datasets_packet = + XFA_GetPacketByIndex(XFA_PacketType::Datasets); + if (MatchNodeName(pXMLDocumentNode, datasets_packet.name, datasets_packet.uri, + datasets_packet.flags)) { return pXMLDocumentNode; } - - const PacketInfo* packet = GetPacketByIndex(XFA_PacketType::Xdp); - if (!MatchNodeName(pXMLDocumentNode, packet->name, packet->uri, - packet->flags)) { + XFA_PACKETINFO xdp_packet = XFA_GetPacketByIndex(XFA_PacketType::Xdp); + if (!MatchNodeName(pXMLDocumentNode, xdp_packet.name, xdp_packet.uri, + xdp_packet.flags)) { return nullptr; } - for (CFX_XMLNode* pDatasetsNode = pXMLDocumentNode->GetFirstChild(); pDatasetsNode; pDatasetsNode = pDatasetsNode->GetNextSibling()) { - if (MatchNodeName(pDatasetsNode, datasets_packet->name, - datasets_packet->uri, datasets_packet->flags)) { + if (MatchNodeName(pDatasetsNode, datasets_packet.name, datasets_packet.uri, + datasets_packet.flags)) { return pDatasetsNode; } } @@ -400,11 +364,9 @@ CXFA_Node* CXFA_DocumentParser::ParseAsXDPPacket_XDP( CFX_XMLNode* pXMLDocumentNode) { - const PacketInfo* packet = GetPacketByIndex(XFA_PacketType::Xdp); - if (!MatchNodeName(pXMLDocumentNode, packet->name, packet->uri, - packet->flags)) { + XFA_PACKETINFO packet = XFA_GetPacketByIndex(XFA_PacketType::Xdp); + if (!MatchNodeName(pXMLDocumentNode, packet.name, packet.uri, packet.flags)) return nullptr; - } CXFA_Node* pXFARootNode = m_pFactory->CreateNode(XFA_PacketType::Xdp, XFA_Element::Xfa); @@ -426,15 +388,16 @@ CFX_XMLNode* pXMLConfigDOMRoot = nullptr; CXFA_Node* pXFAConfigDOMRoot = nullptr; - const PacketInfo* config_packet_info = - GetPacketByIndex(XFA_PacketType::Config); + XFA_PACKETINFO config_packet = XFA_GetPacketByIndex(XFA_PacketType::Config); for (CFX_XMLNode* pChildItem = pXMLDocumentNode->GetFirstChild(); pChildItem; pChildItem = pChildItem->GetNextSibling()) { - if (!MatchNodeName(pChildItem, config_packet_info->name, - config_packet_info->uri, config_packet_info->flags)) { + if (!MatchNodeName(pChildItem, config_packet.name, config_packet.uri, + config_packet.flags)) { continue; } - if (pXFARootNode->GetFirstChildByName(config_packet_info->hash)) + // TODO(tsepez): make GetFirstChildByName() take a name. + uint32_t hash = FX_HashCode_GetW(config_packet.name, false); + if (pXFARootNode->GetFirstChildByName(hash)) return nullptr; pXMLConfigDOMRoot = pChildItem; @@ -453,16 +416,16 @@ continue; WideString wsPacketName = pElement->GetLocalTagName(); - const PacketInfo* pPacketInfo = - GetPacketByName(wsPacketName.AsStringView()); - if (pPacketInfo && pPacketInfo->uri) { - if (!MatchNodeName(pElement, pPacketInfo->name, pPacketInfo->uri, - pPacketInfo->flags)) { - pPacketInfo = nullptr; - } + Optional<XFA_PACKETINFO> packet_info = + XFA_GetPacketByName(wsPacketName.AsStringView()); + if (packet_info.has_value() && packet_info.value().uri && + !MatchNodeName(pElement, packet_info.value().name, + packet_info.value().uri, packet_info.value().flags)) { + packet_info = {}; } - XFA_PacketType ePacket = - pPacketInfo ? pPacketInfo->packet_type : XFA_PacketType::User; + XFA_PacketType ePacket = XFA_PacketType::User; + if (packet_info.has_value()) + ePacket = packet_info.value().packet_type; if (ePacket == XFA_PacketType::Xdp) continue; if (ePacket == XFA_PacketType::Datasets) { @@ -488,9 +451,10 @@ } else { CXFA_Node* pPacketNode = ParseAsXDPPacket(pElement, ePacket); if (pPacketNode) { - if (pPacketInfo && - (pPacketInfo->flags & XFA_XDPPACKET_FLAGS_SUPPORTONE) && - pXFARootNode->GetFirstChildByName(pPacketInfo->hash)) { + if (packet_info.has_value() && + (packet_info.value().flags & XFA_XDPPACKET_FLAGS_SUPPORTONE) && + pXFARootNode->GetFirstChildByName( + FX_HashCode_GetW(packet_info.value().name, false))) { return nullptr; } pXFARootNode->InsertChild(pPacketNode, nullptr); @@ -521,17 +485,16 @@ CXFA_Node* CXFA_DocumentParser::ParseAsXDPPacket_Config( CFX_XMLNode* pXMLDocumentNode) { - const PacketInfo* packet = GetPacketByIndex(XFA_PacketType::Config); - if (!MatchNodeName(pXMLDocumentNode, packet->name, packet->uri, - packet->flags)) { + XFA_PACKETINFO packet = XFA_GetPacketByIndex(XFA_PacketType::Config); + if (!MatchNodeName(pXMLDocumentNode, packet.name, packet.uri, packet.flags)) return nullptr; - } + CXFA_Node* pNode = m_pFactory->CreateNode(XFA_PacketType::Config, XFA_Element::Config); if (!pNode) return nullptr; - pNode->JSObject()->SetCData(XFA_Attribute::Name, packet->name, false, false); + pNode->JSObject()->SetCData(XFA_Attribute::Name, packet.name, false, false); if (!NormalLoader(pNode, pXMLDocumentNode, XFA_PacketType::Config, true)) return nullptr; @@ -541,18 +504,16 @@ CXFA_Node* CXFA_DocumentParser::ParseAsXDPPacket_Template( CFX_XMLNode* pXMLDocumentNode) { - const PacketInfo* packet = GetPacketByIndex(XFA_PacketType::Template); - if (!MatchNodeName(pXMLDocumentNode, packet->name, packet->uri, - packet->flags)) { + XFA_PACKETINFO packet = XFA_GetPacketByIndex(XFA_PacketType::Template); + if (!MatchNodeName(pXMLDocumentNode, packet.name, packet.uri, packet.flags)) return nullptr; - } CXFA_Node* pNode = m_pFactory->CreateNode(XFA_PacketType::Template, XFA_Element::Template); if (!pNode) return nullptr; - pNode->JSObject()->SetCData(XFA_Attribute::Name, packet->name, false, false); + pNode->JSObject()->SetCData(XFA_Attribute::Name, packet.name, false, false); CFX_XMLElement* pXMLDocumentElement = ToXMLElement(pXMLDocumentNode); WideString wsNamespaceURI = pXMLDocumentElement->GetNamespaceURI(); @@ -570,18 +531,16 @@ CXFA_Node* CXFA_DocumentParser::ParseAsXDPPacket_Form( CFX_XMLNode* pXMLDocumentNode) { - const PacketInfo* packet = GetPacketByIndex(XFA_PacketType::Form); - if (!MatchNodeName(pXMLDocumentNode, packet->name, packet->uri, - packet->flags)) { + XFA_PACKETINFO packet = XFA_GetPacketByIndex(XFA_PacketType::Form); + if (!MatchNodeName(pXMLDocumentNode, packet.name, packet.uri, packet.flags)) return nullptr; - } CXFA_Node* pNode = m_pFactory->CreateNode(XFA_PacketType::Form, XFA_Element::Form); if (!pNode) return nullptr; - pNode->JSObject()->SetCData(XFA_Attribute::Name, packet->name, false, false); + pNode->JSObject()->SetCData(XFA_Attribute::Name, packet.name, false, false); CXFA_Template* pTemplateRoot = m_pRootNode->GetFirstChildByClass<CXFA_Template>(XFA_Element::Template); CXFA_Subform* pTemplateChosen = @@ -604,16 +563,15 @@ CXFA_Node* CXFA_DocumentParser::ParseAsXDPPacket_Data( CFX_XMLNode* pXMLDocumentNode) { + XFA_PACKETINFO packet = XFA_GetPacketByIndex(XFA_PacketType::Datasets); CFX_XMLNode* pDatasetsXMLNode = GetDataSetsFromXDP(pXMLDocumentNode); - const PacketInfo* packet = GetPacketByIndex(XFA_PacketType::Datasets); if (pDatasetsXMLNode) { CXFA_Node* pNode = m_pFactory->CreateNode(XFA_PacketType::Datasets, XFA_Element::DataModel); if (!pNode) return nullptr; - pNode->JSObject()->SetCData(XFA_Attribute::Name, packet->name, false, - false); + pNode->JSObject()->SetCData(XFA_Attribute::Name, packet.name, false, false); if (!DataLoader(pNode, pDatasetsXMLNode, false)) return nullptr; @@ -622,7 +580,7 @@ } CFX_XMLNode* pDataXMLNode = nullptr; - if (MatchNodeName(pXMLDocumentNode, L"data", packet->uri, packet->flags)) { + if (MatchNodeName(pXMLDocumentNode, L"data", packet.uri, packet.flags)) { ToXMLElement(pXMLDocumentNode)->RemoveAttribute(L"xmlns:xfa"); pDataXMLNode = pXMLDocumentNode; } else { @@ -660,17 +618,15 @@ CFX_XMLNode* pXMLDocumentNode, XFA_PacketType packet_type, XFA_Element element) { - const PacketInfo* packet = GetPacketByIndex(packet_type); - if (!MatchNodeName(pXMLDocumentNode, packet->name, packet->uri, - packet->flags)) { + XFA_PACKETINFO packet = XFA_GetPacketByIndex(packet_type); + if (!MatchNodeName(pXMLDocumentNode, packet.name, packet.uri, packet.flags)) return nullptr; - } CXFA_Node* pNode = m_pFactory->CreateNode(packet_type, element); if (!pNode) return nullptr; - pNode->JSObject()->SetCData(XFA_Attribute::Name, packet->name, false, false); + pNode->JSObject()->SetCData(XFA_Attribute::Name, packet.name, false, false); if (!NormalLoader(pNode, pXMLDocumentNode, packet_type, true)) return nullptr; @@ -680,9 +636,8 @@ CXFA_Node* CXFA_DocumentParser::ParseAsXDPPacket_Xdc( CFX_XMLNode* pXMLDocumentNode) { - const PacketInfo* packet = GetPacketByIndex(XFA_PacketType::Xdc); - if (!MatchNodeName(pXMLDocumentNode, packet->name, packet->uri, - packet->flags)) + XFA_PACKETINFO packet = XFA_GetPacketByIndex(XFA_PacketType::Xdc); + if (!MatchNodeName(pXMLDocumentNode, packet.name, packet.uri, packet.flags)) return nullptr; CXFA_Node* pNode = @@ -690,7 +645,7 @@ if (!pNode) return nullptr; - pNode->JSObject()->SetCData(XFA_Attribute::Name, packet->name, false, false); + pNode->JSObject()->SetCData(XFA_Attribute::Name, packet.name, false, false); pNode->SetXMLMappingNode(pXMLDocumentNode); return pNode; }
diff --git a/xfa/fxfa/parser/xfa_basic_data.cpp b/xfa/fxfa/parser/xfa_basic_data.cpp index 6c53325..655c11f 100644 --- a/xfa/fxfa/parser/xfa_basic_data.cpp +++ b/xfa/fxfa/parser/xfa_basic_data.cpp
@@ -153,6 +153,23 @@ namespace { +struct PacketRecord { + uint32_t hash; + const wchar_t* name; + XFA_PacketType packet_type; + const wchar_t* uri; + uint32_t flags; +}; + +const PacketRecord g_PacketTable[] = { +#undef PCKT____ +#define PCKT____(a, b, c, d, e, f) \ + {a, L##b, XFA_PacketType::c, d, \ + XFA_XDPPACKET_FLAGS_##e | XFA_XDPPACKET_FLAGS_##f}, +#include "xfa/fxfa/parser/packets.inc" +#undef PCKT____ +}; + struct ElementRecord { uint32_t hash; // Hashed as wide string. XFA_Element element; @@ -211,6 +228,21 @@ } // namespace +XFA_PACKETINFO XFA_GetPacketByIndex(XFA_PacketType ePacket) { + const PacketRecord* pRecord = &g_PacketTable[static_cast<uint8_t>(ePacket)]; + return {pRecord->name, pRecord->packet_type, pRecord->uri, pRecord->flags}; +} + +Optional<XFA_PACKETINFO> XFA_GetPacketByName(const WideStringView& wsName) { + uint32_t hash = FX_HashCode_GetW(wsName, false); + auto* elem = std::lower_bound( + std::begin(g_PacketTable), std::end(g_PacketTable), hash, + [](const PacketRecord& a, uint32_t hash) { return a.hash < hash; }); + if (elem != std::end(g_PacketTable) && elem->name == wsName) + return XFA_GetPacketByIndex(elem->packet_type); + return {}; +} + ByteStringView XFA_ElementToName(XFA_Element elem) { return g_ElementTable[static_cast<size_t>(elem)].name; }
diff --git a/xfa/fxfa/parser/xfa_basic_data.h b/xfa/fxfa/parser/xfa_basic_data.h index d17e512..da892d6 100644 --- a/xfa/fxfa/parser/xfa_basic_data.h +++ b/xfa/fxfa/parser/xfa_basic_data.h
@@ -14,6 +14,13 @@ #include "third_party/base/optional.h" #include "xfa/fxfa/fxfa_basic.h" +struct XFA_PACKETINFO { + const wchar_t* name; + XFA_PacketType packet_type; + const wchar_t* uri; + uint32_t flags; +}; + struct XFA_ATTRIBUTEINFO { XFA_Attribute attribute; XFA_ScriptType eValueType; @@ -25,6 +32,9 @@ XFA_ATTRIBUTE_CALLBACK callback = nullptr; }; +XFA_PACKETINFO XFA_GetPacketByIndex(XFA_PacketType ePacket); +Optional<XFA_PACKETINFO> XFA_GetPacketByName(const WideStringView& wsName); + ByteStringView XFA_ElementToName(XFA_Element elem); XFA_Element XFA_GetElementByName(const WideString& name);
diff --git a/xfa/fxfa/parser/xfa_basic_data_unittest.cpp b/xfa/fxfa/parser/xfa_basic_data_unittest.cpp index d2a2872..b9307d5 100644 --- a/xfa/fxfa/parser/xfa_basic_data_unittest.cpp +++ b/xfa/fxfa/parser/xfa_basic_data_unittest.cpp
@@ -8,6 +8,30 @@ #include "testing/gtest/include/gtest/gtest.h" +TEST(XFABasicDataTest, GetPacketByName) { + Optional<XFA_PACKETINFO> result = XFA_GetPacketByName(L""); + EXPECT_FALSE(result.has_value()); + + result = XFA_GetPacketByName(L"nonesuch"); + EXPECT_FALSE(result.has_value()); + + result = XFA_GetPacketByName(L"datasets"); + ASSERT_TRUE(result.has_value()); + EXPECT_EQ(XFA_PacketType::Datasets, result.value().packet_type); + + result = XFA_GetPacketByName(L"sourceSet"); + ASSERT_TRUE(result.has_value()); + EXPECT_EQ(XFA_PacketType::SourceSet, result.value().packet_type); +} + +TEST(XFABasicDataTest, PacketToName) { + XFA_PACKETINFO result = XFA_GetPacketByIndex(XFA_PacketType::Datasets); + EXPECT_STREQ(L"datasets", result.name); + + result = XFA_GetPacketByIndex(XFA_PacketType::ConnectionSet); + EXPECT_STREQ(L"connectionSet", result.name); +} + TEST(XFABasicDataTest, GetElementByName) { EXPECT_EQ(XFA_Element::Unknown, XFA_GetElementByName(L"")); EXPECT_EQ(XFA_Element::Unknown, XFA_GetElementByName(L"nonesuch")); @@ -46,7 +70,7 @@ EXPECT_EQ(XFA_Attribute::DecipherOnly, result.value().attribute); } -TEST(XFABasicDataTest, AttributeToNamee) { +TEST(XFABasicDataTest, AttributeToName) { EXPECT_EQ("spaceBelow", XFA_AttributeToName(XFA_Attribute::SpaceBelow)); EXPECT_EQ("decipherOnly", XFA_AttributeToName(XFA_Attribute::DecipherOnly)); }