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));
}
