Add CPDF_NameTree::CreateForTesting().
Make the test-only ctor private, and migrate non-test code that use the
test-only ctor to use the non-test ctor. Also disallow copying
CPDF_NameTree.
Change-Id: Ie95aec563ce6312a6a259bd2ef294fd4d8f5d7a2
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/68192
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fpdfdoc/cpdf_nametree.cpp b/core/fpdfdoc/cpdf_nametree.cpp
index 80bb5de..a42376e 100644
--- a/core/fpdfdoc/cpdf_nametree.cpp
+++ b/core/fpdfdoc/cpdf_nametree.cpp
@@ -312,6 +312,12 @@
CPDF_NameTree::~CPDF_NameTree() = default;
+// static
+std::unique_ptr<CPDF_NameTree> CPDF_NameTree::CreateForTesting(
+ CPDF_Dictionary* pRoot) {
+ return pdfium::WrapUnique(new CPDF_NameTree(pRoot)); // Private ctor.
+}
+
size_t CPDF_NameTree::GetCount() const {
return m_pRoot ? CountNamesInternal(m_pRoot.Get(), 0) : 0;
}
diff --git a/core/fpdfdoc/cpdf_nametree.h b/core/fpdfdoc/cpdf_nametree.h
index b018ae7..6ad1c1e 100644
--- a/core/fpdfdoc/cpdf_nametree.h
+++ b/core/fpdfdoc/cpdf_nametree.h
@@ -19,10 +19,14 @@
class CPDF_NameTree {
public:
- explicit CPDF_NameTree(CPDF_Dictionary* pRoot);
CPDF_NameTree(CPDF_Document* pDoc, const ByteString& category);
+ CPDF_NameTree(const CPDF_NameTree&) = delete;
+ CPDF_NameTree& operator=(const CPDF_NameTree&) = delete;
~CPDF_NameTree();
+ static std::unique_ptr<CPDF_NameTree> CreateForTesting(
+ CPDF_Dictionary* pRoot);
+
bool AddValueAndName(RetainPtr<CPDF_Object> pObj, const WideString& name);
bool DeleteValueAndName(int nIndex);
@@ -34,6 +38,8 @@
CPDF_Dictionary* GetRootForTest() const { return m_pRoot.Get(); }
private:
+ explicit CPDF_NameTree(CPDF_Dictionary* pRoot);
+
RetainPtr<CPDF_Dictionary> m_pRoot;
};
diff --git a/core/fpdfdoc/cpdf_nametree_unittest.cpp b/core/fpdfdoc/cpdf_nametree_unittest.cpp
index 8b9c95f..6ef0d1b 100644
--- a/core/fpdfdoc/cpdf_nametree_unittest.cpp
+++ b/core/fpdfdoc/cpdf_nametree_unittest.cpp
@@ -89,14 +89,15 @@
pNames->AppendNew<CPDF_Number>(100);
// Check that the key is as expected.
- CPDF_NameTree name_tree(pRootDict.Get());
+ std::unique_ptr<CPDF_NameTree> name_tree =
+ CPDF_NameTree::CreateForTesting(pRootDict.Get());
WideString storedName;
- name_tree.LookupValueAndName(0, &storedName);
+ name_tree->LookupValueAndName(0, &storedName);
EXPECT_STREQ(L"1", storedName.c_str());
// Check that the correct value object can be obtained by looking up "1".
WideString matchName = L"1";
- CPDF_Object* pObj = name_tree.LookupValue(matchName);
+ CPDF_Object* pObj = name_tree->LookupValue(matchName);
ASSERT_TRUE(pObj->IsNumber());
EXPECT_EQ(100, pObj->AsNumber()->GetInteger());
}
@@ -108,23 +109,24 @@
AddNameKeyValue(pNames, "2.txt", 222);
AddNameKeyValue(pNames, "7.txt", 777);
- CPDF_NameTree name_tree(pRootDict.Get());
+ std::unique_ptr<CPDF_NameTree> name_tree =
+ CPDF_NameTree::CreateForTesting(pRootDict.Get());
// Insert a name that already exists in the names array.
- EXPECT_FALSE(name_tree.AddValueAndName(pdfium::MakeRetain<CPDF_Number>(111),
- L"2.txt"));
+ EXPECT_FALSE(name_tree->AddValueAndName(pdfium::MakeRetain<CPDF_Number>(111),
+ L"2.txt"));
// Insert in the beginning of the names array.
- EXPECT_TRUE(name_tree.AddValueAndName(pdfium::MakeRetain<CPDF_Number>(111),
- L"1.txt"));
+ EXPECT_TRUE(name_tree->AddValueAndName(pdfium::MakeRetain<CPDF_Number>(111),
+ L"1.txt"));
// Insert in the middle of the names array.
- EXPECT_TRUE(name_tree.AddValueAndName(pdfium::MakeRetain<CPDF_Number>(555),
- L"5.txt"));
+ EXPECT_TRUE(name_tree->AddValueAndName(pdfium::MakeRetain<CPDF_Number>(555),
+ L"5.txt"));
// Insert at the end of the names array.
- EXPECT_TRUE(name_tree.AddValueAndName(pdfium::MakeRetain<CPDF_Number>(999),
- L"9.txt"));
+ EXPECT_TRUE(name_tree->AddValueAndName(pdfium::MakeRetain<CPDF_Number>(999),
+ L"9.txt"));
// Check that the names array has the expected key-value pairs.
CheckNameKeyValue(pNames, 0, "1.txt", 111);
@@ -139,27 +141,28 @@
auto pRootDict = pdfium::MakeRetain<CPDF_Dictionary>();
CPDF_Array* pNames = pRootDict->SetNewFor<CPDF_Array>("Names");
- CPDF_NameTree name_tree(pRootDict.Get());
+ std::unique_ptr<CPDF_NameTree> name_tree =
+ CPDF_NameTree::CreateForTesting(pRootDict.Get());
// Insert a name should work.
- EXPECT_TRUE(name_tree.AddValueAndName(pdfium::MakeRetain<CPDF_Number>(111),
- L"2.txt"));
-
- // Insert a name that already exists in the names array.
- EXPECT_FALSE(name_tree.AddValueAndName(pdfium::MakeRetain<CPDF_Number>(111),
+ EXPECT_TRUE(name_tree->AddValueAndName(pdfium::MakeRetain<CPDF_Number>(111),
L"2.txt"));
+ // Insert a name that already exists in the names array.
+ EXPECT_FALSE(name_tree->AddValueAndName(pdfium::MakeRetain<CPDF_Number>(111),
+ L"2.txt"));
+
// Insert in the beginning of the names array.
- EXPECT_TRUE(name_tree.AddValueAndName(pdfium::MakeRetain<CPDF_Number>(111),
- L"1.txt"));
+ EXPECT_TRUE(name_tree->AddValueAndName(pdfium::MakeRetain<CPDF_Number>(111),
+ L"1.txt"));
// Insert in the middle of the names array.
- EXPECT_TRUE(name_tree.AddValueAndName(pdfium::MakeRetain<CPDF_Number>(555),
- L"5.txt"));
+ EXPECT_TRUE(name_tree->AddValueAndName(pdfium::MakeRetain<CPDF_Number>(555),
+ L"5.txt"));
// Insert at the end of the names array.
- EXPECT_TRUE(name_tree.AddValueAndName(pdfium::MakeRetain<CPDF_Number>(999),
- L"9.txt"));
+ EXPECT_TRUE(name_tree->AddValueAndName(pdfium::MakeRetain<CPDF_Number>(999),
+ L"9.txt"));
// Check that the names array has the expected key-value pairs.
CheckNameKeyValue(pNames, 0, "1.txt", 111);
@@ -172,39 +175,40 @@
// Set up a name tree with five nodes of three levels.
auto pRootDict = pdfium::MakeRetain<CPDF_Dictionary>();
FillNameTreeDict(pRootDict.Get());
- CPDF_NameTree name_tree(pRootDict.Get());
+ std::unique_ptr<CPDF_NameTree> name_tree =
+ CPDF_NameTree::CreateForTesting(pRootDict.Get());
// Check that adding an existing name would fail.
- EXPECT_FALSE(name_tree.AddValueAndName(pdfium::MakeRetain<CPDF_Number>(444),
- L"9.txt"));
+ EXPECT_FALSE(name_tree->AddValueAndName(pdfium::MakeRetain<CPDF_Number>(444),
+ L"9.txt"));
// Add a name within the limits of a leaf node.
- EXPECT_TRUE(name_tree.AddValueAndName(pdfium::MakeRetain<CPDF_Number>(444),
- L"4.txt"));
- ASSERT_TRUE(name_tree.LookupValue(L"4.txt"));
- EXPECT_EQ(444, name_tree.LookupValue(L"4.txt")->GetInteger());
+ EXPECT_TRUE(name_tree->AddValueAndName(pdfium::MakeRetain<CPDF_Number>(444),
+ L"4.txt"));
+ ASSERT_TRUE(name_tree->LookupValue(L"4.txt"));
+ EXPECT_EQ(444, name_tree->LookupValue(L"4.txt")->GetInteger());
// Add a name that requires changing the limits of two bottom levels.
- EXPECT_TRUE(name_tree.AddValueAndName(pdfium::MakeRetain<CPDF_Number>(666),
- L"6.txt"));
- ASSERT_TRUE(name_tree.LookupValue(L"6.txt"));
- EXPECT_EQ(666, name_tree.LookupValue(L"6.txt")->GetInteger());
+ EXPECT_TRUE(name_tree->AddValueAndName(pdfium::MakeRetain<CPDF_Number>(666),
+ L"6.txt"));
+ ASSERT_TRUE(name_tree->LookupValue(L"6.txt"));
+ EXPECT_EQ(666, name_tree->LookupValue(L"6.txt")->GetInteger());
// Add a name that requires changing the limits of two top levels.
- EXPECT_TRUE(name_tree.AddValueAndName(pdfium::MakeRetain<CPDF_Number>(99),
- L"99.txt"));
- ASSERT_TRUE(name_tree.LookupValue(L"99.txt"));
- EXPECT_EQ(99, name_tree.LookupValue(L"99.txt")->GetInteger());
+ EXPECT_TRUE(name_tree->AddValueAndName(pdfium::MakeRetain<CPDF_Number>(99),
+ L"99.txt"));
+ ASSERT_TRUE(name_tree->LookupValue(L"99.txt"));
+ EXPECT_EQ(99, name_tree->LookupValue(L"99.txt")->GetInteger());
// Add a name that requires changing the lower limit of all levels.
- EXPECT_TRUE(
- name_tree.AddValueAndName(pdfium::MakeRetain<CPDF_Number>(-5), L"0.txt"));
- ASSERT_TRUE(name_tree.LookupValue(L"0.txt"));
- EXPECT_EQ(-5, name_tree.LookupValue(L"0.txt")->GetInteger());
+ EXPECT_TRUE(name_tree->AddValueAndName(pdfium::MakeRetain<CPDF_Number>(-5),
+ L"0.txt"));
+ ASSERT_TRUE(name_tree->LookupValue(L"0.txt"));
+ EXPECT_EQ(-5, name_tree->LookupValue(L"0.txt")->GetInteger());
// Check that the node on the first level has the expected limits.
CPDF_Dictionary* pKid1 =
- name_tree.GetRootForTest()->GetArrayFor("Kids")->GetDictAt(0);
+ name_tree->GetRootForTest()->GetArrayFor("Kids")->GetDictAt(0);
ASSERT_TRUE(pKid1);
CheckLimitsArray(pKid1, "0.txt", "99.txt");
@@ -246,11 +250,12 @@
// Set up a name tree with five nodes of three levels.
auto pRootDict = pdfium::MakeRetain<CPDF_Dictionary>();
FillNameTreeDict(pRootDict.Get());
- CPDF_NameTree name_tree(pRootDict.Get());
+ std::unique_ptr<CPDF_NameTree> name_tree =
+ CPDF_NameTree::CreateForTesting(pRootDict.Get());
// Retrieve the kid dictionaries.
CPDF_Dictionary* pKid1 =
- name_tree.GetRootForTest()->GetArrayFor("Kids")->GetDictAt(0);
+ name_tree->GetRootForTest()->GetArrayFor("Kids")->GetDictAt(0);
ASSERT_TRUE(pKid1);
CPDF_Dictionary* pKid2 = pKid1->GetArrayFor("Kids")->GetDictAt(0);
ASSERT_TRUE(pKid2);
@@ -262,28 +267,28 @@
ASSERT_TRUE(pKid5);
// Check that deleting an out-of-bound index would fail.
- EXPECT_FALSE(name_tree.DeleteValueAndName(5));
+ EXPECT_FALSE(name_tree->DeleteValueAndName(5));
// Delete the name "9.txt", and check that its node gets deleted and its
// parent node's limits get updated.
WideString csName;
- ASSERT_TRUE(name_tree.LookupValue(L"9.txt"));
- EXPECT_EQ(999, name_tree.LookupValue(L"9.txt")->GetInteger());
- EXPECT_TRUE(name_tree.LookupValueAndName(4, &csName));
+ ASSERT_TRUE(name_tree->LookupValue(L"9.txt"));
+ EXPECT_EQ(999, name_tree->LookupValue(L"9.txt")->GetInteger());
+ EXPECT_TRUE(name_tree->LookupValueAndName(4, &csName));
EXPECT_STREQ(L"9.txt", csName.c_str());
EXPECT_EQ(2u, pKid1->GetArrayFor("Kids")->size());
- EXPECT_TRUE(name_tree.DeleteValueAndName(4));
+ EXPECT_TRUE(name_tree->DeleteValueAndName(4));
EXPECT_EQ(1u, pKid1->GetArrayFor("Kids")->size());
CheckLimitsArray(pKid1, "1.txt", "5.txt");
// Delete the name "2.txt", and check that its node does not get deleted, its
// node's limits get updated, and no other limits get updated.
- ASSERT_TRUE(name_tree.LookupValue(L"2.txt"));
- EXPECT_EQ(222, name_tree.LookupValue(L"2.txt")->GetInteger());
- EXPECT_TRUE(name_tree.LookupValueAndName(1, &csName));
+ ASSERT_TRUE(name_tree->LookupValue(L"2.txt"));
+ EXPECT_EQ(222, name_tree->LookupValue(L"2.txt")->GetInteger());
+ EXPECT_TRUE(name_tree->LookupValueAndName(1, &csName));
EXPECT_STREQ(L"2.txt", csName.c_str());
EXPECT_EQ(4u, pKid4->GetArrayFor("Names")->size());
- EXPECT_TRUE(name_tree.DeleteValueAndName(1));
+ EXPECT_TRUE(name_tree->DeleteValueAndName(1));
EXPECT_EQ(2u, pKid4->GetArrayFor("Names")->size());
CheckLimitsArray(pKid4, "1.txt", "1.txt");
CheckLimitsArray(pKid2, "1.txt", "5.txt");
@@ -291,24 +296,24 @@
// Delete the name "1.txt", and check that its node gets deleted, and its
// parent's and gradparent's limits get updated.
- ASSERT_TRUE(name_tree.LookupValue(L"1.txt"));
- EXPECT_EQ(111, name_tree.LookupValue(L"1.txt")->GetInteger());
- EXPECT_TRUE(name_tree.LookupValueAndName(0, &csName));
+ ASSERT_TRUE(name_tree->LookupValue(L"1.txt"));
+ EXPECT_EQ(111, name_tree->LookupValue(L"1.txt")->GetInteger());
+ EXPECT_TRUE(name_tree->LookupValueAndName(0, &csName));
EXPECT_STREQ(L"1.txt", csName.c_str());
EXPECT_EQ(2u, pKid2->GetArrayFor("Kids")->size());
- EXPECT_TRUE(name_tree.DeleteValueAndName(0));
+ EXPECT_TRUE(name_tree->DeleteValueAndName(0));
EXPECT_EQ(1u, pKid2->GetArrayFor("Kids")->size());
CheckLimitsArray(pKid2, "3.txt", "5.txt");
CheckLimitsArray(pKid1, "3.txt", "5.txt");
// Delete the name "3.txt", and check that its node does not get deleted, and
// its node's, its parent's, and its grandparent's limits get updated.
- ASSERT_TRUE(name_tree.LookupValue(L"3.txt"));
- EXPECT_EQ(333, name_tree.LookupValue(L"3.txt")->GetInteger());
- EXPECT_TRUE(name_tree.LookupValueAndName(0, &csName));
+ ASSERT_TRUE(name_tree->LookupValue(L"3.txt"));
+ EXPECT_EQ(333, name_tree->LookupValue(L"3.txt")->GetInteger());
+ EXPECT_TRUE(name_tree->LookupValueAndName(0, &csName));
EXPECT_STREQ(L"3.txt", csName.c_str());
EXPECT_EQ(4u, pKid5->GetArrayFor("Names")->size());
- EXPECT_TRUE(name_tree.DeleteValueAndName(0));
+ EXPECT_TRUE(name_tree->DeleteValueAndName(0));
EXPECT_EQ(2u, pKid5->GetArrayFor("Names")->size());
CheckLimitsArray(pKid5, "5.txt", "5.txt");
CheckLimitsArray(pKid2, "5.txt", "5.txt");
@@ -316,16 +321,16 @@
// Delete the name "5.txt", and check that all nodes in the tree get deleted
// since they are now all empty.
- ASSERT_TRUE(name_tree.LookupValue(L"5.txt"));
- EXPECT_EQ(555, name_tree.LookupValue(L"5.txt")->GetInteger());
- EXPECT_TRUE(name_tree.LookupValueAndName(0, &csName));
+ ASSERT_TRUE(name_tree->LookupValue(L"5.txt"));
+ EXPECT_EQ(555, name_tree->LookupValue(L"5.txt")->GetInteger());
+ EXPECT_TRUE(name_tree->LookupValueAndName(0, &csName));
EXPECT_STREQ(L"5.txt", csName.c_str());
- EXPECT_EQ(1u, name_tree.GetRootForTest()->GetArrayFor("Kids")->size());
- EXPECT_TRUE(name_tree.DeleteValueAndName(0));
- EXPECT_EQ(0u, name_tree.GetRootForTest()->GetArrayFor("Kids")->size());
+ EXPECT_EQ(1u, name_tree->GetRootForTest()->GetArrayFor("Kids")->size());
+ EXPECT_TRUE(name_tree->DeleteValueAndName(0));
+ EXPECT_EQ(0u, name_tree->GetRootForTest()->GetArrayFor("Kids")->size());
// Check that the tree is now empty.
- EXPECT_EQ(0u, name_tree.GetCount());
- EXPECT_FALSE(name_tree.LookupValueAndName(0, &csName));
- EXPECT_FALSE(name_tree.DeleteValueAndName(0));
+ EXPECT_EQ(0u, name_tree->GetCount());
+ EXPECT_FALSE(name_tree->LookupValueAndName(0, &csName));
+ EXPECT_FALSE(name_tree->DeleteValueAndName(0));
}
diff --git a/testing/fuzzers/pdf_nametree_fuzzer.cc b/testing/fuzzers/pdf_nametree_fuzzer.cc
index 9a37024..d810051 100644
--- a/testing/fuzzers/pdf_nametree_fuzzer.cc
+++ b/testing/fuzzers/pdf_nametree_fuzzer.cc
@@ -54,22 +54,23 @@
CPDF_StreamParser parser(remaining);
auto dict = pdfium::MakeRetain<CPDF_Dictionary>();
- CPDF_NameTree name_tree(dict.Get());
+ std::unique_ptr<CPDF_NameTree> name_tree =
+ CPDF_NameTree::CreateForTesting(dict.Get());
for (const auto& name : params.names) {
RetainPtr<CPDF_Object> obj = parser.ReadNextObject(
/*bAllowNestedArray*/ true, /*bInArray=*/false, /*dwRecursionLevel=*/0);
if (!obj)
break;
- name_tree.AddValueAndName(std::move(obj), name);
+ name_tree->AddValueAndName(std::move(obj), name);
}
if (params.delete_backwards) {
for (size_t i = params.count; i > 0; --i)
- name_tree.DeleteValueAndName(i);
+ name_tree->DeleteValueAndName(i);
} else {
for (size_t i = 0; i < params.count; ++i)
- name_tree.DeleteValueAndName(0);
+ name_tree->DeleteValueAndName(0);
}
return 0;
}
diff --git a/xfa/fxfa/cxfa_ffdoc.cpp b/xfa/fxfa/cxfa_ffdoc.cpp
index ac4b35f..f885644 100644
--- a/xfa/fxfa/cxfa_ffdoc.cpp
+++ b/xfa/fxfa/cxfa_ffdoc.cpp
@@ -179,22 +179,14 @@
return it->second.pDibSource.As<CFX_DIBitmap>();
}
- CPDF_Dictionary* pRoot = m_pPDFDoc->GetRoot();
- if (!pRoot)
+ CPDF_NameTree nametree(m_pPDFDoc.Get(), "XFAImages");
+ size_t count = nametree.GetCount();
+ if (count == 0)
return nullptr;
- CPDF_Dictionary* pNames = pRoot->GetDictFor("Names");
- if (!pNames)
- return nullptr;
-
- CPDF_Dictionary* pXFAImages = pNames->GetDictFor("XFAImages");
- if (!pXFAImages)
- return nullptr;
-
- CPDF_NameTree nametree(pXFAImages);
CPDF_Object* pObject = nametree.LookupValue(WideString(wsName));
if (!pObject) {
- for (size_t i = 0; i < nametree.GetCount(); i++) {
+ for (size_t i = 0; i < count; ++i) {
WideString wsTemp;
CPDF_Object* pTempObject = nametree.LookupValueAndName(i, &wsTemp);
if (wsTemp == wsName) {
