Funnel CPDF_NameTree creation through Create().
Now CPDF_NameTree creation can fail for bad name trees, and objects can
no longer be in an invalid state without a root dictionary.
Change-Id: Iaab96ba46bc49c5b0d77edaf4d137bd4977c9aae
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/68251
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fpdfdoc/cpdf_dest.cpp b/core/fpdfdoc/cpdf_dest.cpp
index 45279e5..e815c02 100644
--- a/core/fpdfdoc/cpdf_dest.cpp
+++ b/core/fpdfdoc/cpdf_dest.cpp
@@ -45,8 +45,10 @@
return CPDF_Dest();
if (pDest->IsString() || pDest->IsName()) {
- CPDF_NameTree name_tree(pDoc, "Dests");
- return CPDF_Dest(name_tree.LookupNamedDest(pDoc, pDest->GetUnicodeText()));
+ auto name_tree = CPDF_NameTree::Create(pDoc, "Dests");
+ if (!name_tree)
+ return CPDF_Dest();
+ return CPDF_Dest(name_tree->LookupNamedDest(pDoc, pDest->GetUnicodeText()));
}
const CPDF_Array* pArray = pDest->AsArray();
diff --git a/core/fpdfdoc/cpdf_nametree.cpp b/core/fpdfdoc/cpdf_nametree.cpp
index a0fc935..aef6301 100644
--- a/core/fpdfdoc/cpdf_nametree.cpp
+++ b/core/fpdfdoc/cpdf_nametree.cpp
@@ -20,19 +20,6 @@
constexpr int kNameTreeMaxRecursion = 32;
-CPDF_Dictionary* GetNameTreeRoot(CPDF_Document* pDoc,
- const ByteString& category) {
- CPDF_Dictionary* pRoot = pDoc->GetRoot();
- if (!pRoot)
- return nullptr;
-
- CPDF_Dictionary* pNames = pRoot->GetDictFor("Names");
- if (!pNames)
- return nullptr;
-
- return pNames->GetDictFor(category);
-}
-
std::pair<WideString, WideString> GetNodeLimitsMaybeSwap(CPDF_Array* pLimits) {
ASSERT(pLimits);
WideString csLeft = pLimits->GetUnicodeTextAt(0);
@@ -310,14 +297,32 @@
} // namespace
-CPDF_NameTree::CPDF_NameTree(CPDF_Dictionary* pRoot) : m_pRoot(pRoot) {}
-
-CPDF_NameTree::CPDF_NameTree(CPDF_Document* pDoc, const ByteString& category)
- : CPDF_NameTree(GetNameTreeRoot(pDoc, category)) {}
+CPDF_NameTree::CPDF_NameTree(CPDF_Dictionary* pRoot) : m_pRoot(pRoot) {
+ ASSERT(m_pRoot);
+}
CPDF_NameTree::~CPDF_NameTree() = default;
// static
+std::unique_ptr<CPDF_NameTree> CPDF_NameTree::Create(
+ CPDF_Document* pDoc,
+ const ByteString& category) {
+ CPDF_Dictionary* pRoot = pDoc->GetRoot();
+ if (!pRoot)
+ return nullptr;
+
+ CPDF_Dictionary* pNames = pRoot->GetDictFor("Names");
+ if (!pNames)
+ return nullptr;
+
+ CPDF_Dictionary* pCategory = pNames->GetDictFor(category);
+ if (!pCategory)
+ return nullptr;
+
+ return pdfium::WrapUnique(new CPDF_NameTree(pCategory)); // Private ctor.
+}
+
+// static
std::unique_ptr<CPDF_NameTree> CPDF_NameTree::CreateWithRootNameArray(
CPDF_Document* pDoc,
const ByteString& category) {
@@ -333,13 +338,14 @@
}
// Create the |category| dictionary if missing.
- if (!pNames->GetDictFor(category)) {
- CPDF_Dictionary* pFiles = pDoc->NewIndirect<CPDF_Dictionary>();
- pFiles->SetNewFor<CPDF_Array>("Names");
- pNames->SetNewFor<CPDF_Reference>(category, pDoc, pFiles->GetObjNum());
+ CPDF_Dictionary* pCategory = pNames->GetDictFor(category);
+ if (!pCategory) {
+ pCategory = pDoc->NewIndirect<CPDF_Dictionary>();
+ pCategory->SetNewFor<CPDF_Array>("Names");
+ pNames->SetNewFor<CPDF_Reference>(category, pDoc, pCategory->GetObjNum());
}
- return pdfium::MakeUnique<CPDF_NameTree>(pDoc, category);
+ return pdfium::WrapUnique(new CPDF_NameTree(pCategory)); // Private ctor.
}
// static
@@ -349,14 +355,11 @@
}
size_t CPDF_NameTree::GetCount() const {
- return m_pRoot ? CountNamesInternal(m_pRoot.Get(), 0) : 0;
+ return CountNamesInternal(m_pRoot.Get(), 0);
}
bool CPDF_NameTree::AddValueAndName(RetainPtr<CPDF_Object> pObj,
const WideString& name) {
- if (!m_pRoot)
- return false;
-
size_t nIndex = 0;
CPDF_Array* pFind = nullptr;
int nFindIndex = -1;
@@ -413,9 +416,6 @@
}
bool CPDF_NameTree::DeleteValueAndName(int nIndex) {
- if (!m_pRoot)
- return false;
-
size_t nCurIndex = 0;
WideString csName;
CPDF_Array* pFind = nullptr;
@@ -438,18 +438,12 @@
CPDF_Object* CPDF_NameTree::LookupValueAndName(int nIndex,
WideString* csName) const {
csName->clear();
- if (!m_pRoot)
- return nullptr;
-
size_t nCurIndex = 0;
return SearchNameNodeByIndex(m_pRoot.Get(), nIndex, 0, &nCurIndex, csName,
nullptr, nullptr);
}
CPDF_Object* CPDF_NameTree::LookupValue(const WideString& csName) const {
- if (!m_pRoot)
- return nullptr;
-
size_t nIndex = 0;
return SearchNameNodeByName(m_pRoot.Get(), csName, 0, &nIndex, nullptr,
nullptr);
diff --git a/core/fpdfdoc/cpdf_nametree.h b/core/fpdfdoc/cpdf_nametree.h
index 043a2dc..96a89a0 100644
--- a/core/fpdfdoc/cpdf_nametree.h
+++ b/core/fpdfdoc/cpdf_nametree.h
@@ -19,11 +19,13 @@
class CPDF_NameTree {
public:
- 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> Create(CPDF_Document* pDoc,
+ const ByteString& category);
+
// If necessary, create missing Names dictionary in |pDoc|, and/or missing
// Names array in the dictionary that corresponds to |category|, if necessary.
// Returns nullptr on failure.
diff --git a/fpdfsdk/cpdfsdk_formfillenvironment.cpp b/fpdfsdk/cpdfsdk_formfillenvironment.cpp
index d36d628..0f7f48c 100644
--- a/fpdfsdk/cpdfsdk_formfillenvironment.cpp
+++ b/fpdfsdk/cpdfsdk_formfillenvironment.cpp
@@ -592,11 +592,14 @@
}
void CPDFSDK_FormFillEnvironment::ProcJavascriptAction() {
- CPDF_NameTree docJS(m_pCPDFDoc.Get(), "JavaScript");
- int iCount = docJS.GetCount();
- for (int i = 0; i < iCount; i++) {
+ auto name_tree = CPDF_NameTree::Create(m_pCPDFDoc.Get(), "JavaScript");
+ if (!name_tree)
+ return;
+
+ size_t count = name_tree->GetCount();
+ for (size_t i = 0; i < count; ++i) {
WideString name;
- CPDF_Action action(ToDictionary(docJS.LookupValueAndName(i, &name)));
+ CPDF_Action action(ToDictionary(name_tree->LookupValueAndName(i, &name)));
GetActionHandler()->DoAction_JavaScript(action, name, this);
}
}
diff --git a/fpdfsdk/fpdf_attachment.cpp b/fpdfsdk/fpdf_attachment.cpp
index a8d4eeb..529f3e6 100644
--- a/fpdfsdk/fpdf_attachment.cpp
+++ b/fpdfsdk/fpdf_attachment.cpp
@@ -55,7 +55,8 @@
if (!pDoc)
return 0;
- return CPDF_NameTree(pDoc, "EmbeddedFiles").GetCount();
+ auto name_tree = CPDF_NameTree::Create(pDoc, "EmbeddedFiles");
+ return name_tree ? name_tree->GetCount() : 0;
}
FPDF_EXPORT FPDF_ATTACHMENT FPDF_CALLCONV
@@ -92,13 +93,13 @@
if (!pDoc || index < 0)
return nullptr;
- CPDF_NameTree name_tree(pDoc, "EmbeddedFiles");
- if (static_cast<size_t>(index) >= name_tree.GetCount())
+ auto name_tree = CPDF_NameTree::Create(pDoc, "EmbeddedFiles");
+ if (!name_tree || static_cast<size_t>(index) >= name_tree->GetCount())
return nullptr;
WideString csName;
return FPDFAttachmentFromCPDFObject(
- name_tree.LookupValueAndName(index, &csName));
+ name_tree->LookupValueAndName(index, &csName));
}
FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV
@@ -107,11 +108,11 @@
if (!pDoc || index < 0)
return false;
- CPDF_NameTree name_tree(pDoc, "EmbeddedFiles");
- if (static_cast<size_t>(index) >= name_tree.GetCount())
+ auto name_tree = CPDF_NameTree::Create(pDoc, "EmbeddedFiles");
+ if (!name_tree || static_cast<size_t>(index) >= name_tree->GetCount())
return false;
- return name_tree.DeleteValueAndName(index);
+ return name_tree->DeleteValueAndName(index);
}
FPDF_EXPORT unsigned long FPDF_CALLCONV
diff --git a/fpdfsdk/fpdf_javascript.cpp b/fpdfsdk/fpdf_javascript.cpp
index c2d119b..ebe8008 100644
--- a/fpdfsdk/fpdf_javascript.cpp
+++ b/fpdfsdk/fpdf_javascript.cpp
@@ -21,7 +21,11 @@
FPDF_EXPORT int FPDF_CALLCONV
FPDFDoc_GetJavaScriptActionCount(FPDF_DOCUMENT document) {
CPDF_Document* doc = CPDFDocumentFromFPDFDocument(document);
- return doc ? CPDF_NameTree(doc, "JavaScript").GetCount() : -1;
+ if (!doc)
+ return -1;
+
+ auto name_tree = CPDF_NameTree::Create(doc, "JavaScript");
+ return name_tree ? name_tree->GetCount() : 0;
}
FPDF_EXPORT FPDF_JAVASCRIPT_ACTION FPDF_CALLCONV
@@ -30,13 +34,13 @@
if (!doc || index < 0)
return nullptr;
- CPDF_NameTree name_tree(doc, "JavaScript");
- if (static_cast<size_t>(index) >= name_tree.GetCount())
+ auto name_tree = CPDF_NameTree::Create(doc, "JavaScript");
+ if (!name_tree || static_cast<size_t>(index) >= name_tree->GetCount())
return nullptr;
WideString name;
CPDF_Dictionary* obj =
- ToDictionary(name_tree.LookupValueAndName(index, &name));
+ ToDictionary(name_tree->LookupValueAndName(index, &name));
if (!obj)
return nullptr;
diff --git a/fpdfsdk/fpdf_view.cpp b/fpdfsdk/fpdf_view.cpp
index 6415426..d7b808a 100644
--- a/fpdfsdk/fpdf_view.cpp
+++ b/fpdfsdk/fpdf_view.cpp
@@ -995,8 +995,11 @@
if (!pRoot)
return 0;
- CPDF_NameTree name_tree(pDoc, "Dests");
- pdfium::base::CheckedNumeric<FPDF_DWORD> count = name_tree.GetCount();
+ auto name_tree = CPDF_NameTree::Create(pDoc, "Dests");
+ if (!name_tree)
+ return 0;
+
+ pdfium::base::CheckedNumeric<FPDF_DWORD> count = name_tree->GetCount();
const CPDF_Dictionary* pDest = pRoot->GetDictFor("Dests");
if (pDest)
count += pDest->size();
@@ -1016,10 +1019,13 @@
if (!pDoc)
return nullptr;
- CPDF_NameTree name_tree(pDoc, "Dests");
+ auto name_tree = CPDF_NameTree::Create(pDoc, "Dests");
+ if (!name_tree)
+ return nullptr;
+
ByteStringView name_view(name);
return FPDFDestFromCPDFArray(
- name_tree.LookupNamedDest(pDoc, PDF_DecodeText(name_view.raw_span())));
+ name_tree->LookupNamedDest(pDoc, PDF_DecodeText(name_view.raw_span())));
}
#ifdef PDF_ENABLE_V8
@@ -1098,9 +1104,12 @@
CPDF_Object* pDestObj = nullptr;
WideString wsName;
- CPDF_NameTree name_tree(pDoc, "Dests");
- int count = name_tree.GetCount();
- if (index >= count) {
+ auto name_tree = CPDF_NameTree::Create(pDoc, "Dests");
+ if (!name_tree)
+ return nullptr;
+
+ size_t count = name_tree->GetCount();
+ if (static_cast<size_t>(index) >= count) {
const CPDF_Dictionary* pDest = pRoot->GetDictFor("Dests");
if (!pDest)
return nullptr;
@@ -1125,7 +1134,7 @@
}
wsName = PDF_DecodeText(bsName.raw_span());
} else {
- pDestObj = name_tree.LookupValueAndName(index, &wsName);
+ pDestObj = name_tree->LookupValueAndName(index, &wsName);
}
if (!pDestObj)
return nullptr;
diff --git a/fxjs/cjs_document.cpp b/fxjs/cjs_document.cpp
index 0eb261d..62b6908 100644
--- a/fxjs/cjs_document.cpp
+++ b/fxjs/cjs_document.cpp
@@ -1387,9 +1387,12 @@
return CJS_Result::Failure(JSMessage::kBadObjectError);
CPDF_Document* pDocument = m_pFormFillEnv->GetPDFDocument();
- CPDF_NameTree name_tree(pDocument, "Dests");
+ auto name_tree = CPDF_NameTree::Create(pDocument, "Dests");
+ if (!name_tree)
+ return CJS_Result::Failure(JSMessage::kBadObjectError);
+
CPDF_Array* destArray =
- name_tree.LookupNamedDest(pDocument, pRuntime->ToWideString(params[0]));
+ name_tree->LookupNamedDest(pDocument, pRuntime->ToWideString(params[0]));
if (!destArray)
return CJS_Result::Failure(JSMessage::kBadObjectError);
diff --git a/xfa/fxfa/cxfa_ffdoc.cpp b/xfa/fxfa/cxfa_ffdoc.cpp
index f885644..695d366 100644
--- a/xfa/fxfa/cxfa_ffdoc.cpp
+++ b/xfa/fxfa/cxfa_ffdoc.cpp
@@ -179,16 +179,16 @@
return it->second.pDibSource.As<CFX_DIBitmap>();
}
- CPDF_NameTree nametree(m_pPDFDoc.Get(), "XFAImages");
- size_t count = nametree.GetCount();
+ auto name_tree = CPDF_NameTree::Create(m_pPDFDoc.Get(), "XFAImages");
+ size_t count = name_tree ? name_tree->GetCount() : 0;
if (count == 0)
return nullptr;
- CPDF_Object* pObject = nametree.LookupValue(WideString(wsName));
+ CPDF_Object* pObject = name_tree->LookupValue(WideString(wsName));
if (!pObject) {
for (size_t i = 0; i < count; ++i) {
WideString wsTemp;
- CPDF_Object* pTempObject = nametree.LookupValueAndName(i, &wsTemp);
+ CPDF_Object* pTempObject = name_tree->LookupValueAndName(i, &wsTemp);
if (wsTemp == wsName) {
pObject = pTempObject;
break;