Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
chore: cherry-pick 3466cc056b05 from pdfium
- Loading branch information
Showing
2 changed files
with
284 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
cherry-pick-3466cc056b05.patch |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,283 @@ | ||
From 3466cc056b052eb5e8e9d7d5f8f2aa3ab33e3bd6 Mon Sep 17 00:00:00 2001 | ||
From: Tom Sepez <tsepez@chromium.org> | ||
Date: Tue, 12 Jul 2022 18:13:14 +0000 | ||
Subject: [PATCH] M102: Retain nodes when manipulating their dictionaries in CPDF_NameTree. | ||
|
||
-- Pass retain ptrs consistently in a few other places. | ||
|
||
Bug: chromium:1335861 | ||
Change-Id: If23cf6b6ec39ef02384beaa6745e1c7256160cba | ||
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/94430 | ||
Reviewed-by: Lei Zhang <thestig@chromium.org> | ||
Commit-Queue: Tom Sepez <tsepez@chromium.org> | ||
(cherry picked from commit ebebb757c1f210ccc16e0cb2b425860a141a4e9f) | ||
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/95271 | ||
Reviewed-by: Nigi <nigi@chromium.org> | ||
--- | ||
|
||
diff --git a/core/fpdfapi/parser/cpdf_array.cpp b/core/fpdfapi/parser/cpdf_array.cpp | ||
index 708e677..6aa6f30 100644 | ||
--- a/core/fpdfapi/parser/cpdf_array.cpp | ||
+++ b/core/fpdfapi/parser/cpdf_array.cpp | ||
@@ -10,6 +10,7 @@ | ||
#include <utility> | ||
|
||
#include "core/fpdfapi/parser/cpdf_boolean.h" | ||
+#include "core/fpdfapi/parser/cpdf_dictionary.h" | ||
#include "core/fpdfapi/parser/cpdf_name.h" | ||
#include "core/fpdfapi/parser/cpdf_number.h" | ||
#include "core/fpdfapi/parser/cpdf_reference.h" | ||
@@ -153,6 +154,10 @@ | ||
return m_Objects[index]->GetNumber(); | ||
} | ||
|
||
+RetainPtr<CPDF_Dictionary> CPDF_Array::GetMutableDictAt(size_t index) { | ||
+ return pdfium::WrapRetain(GetDictAt(index)); | ||
+} | ||
+ | ||
CPDF_Dictionary* CPDF_Array::GetDictAt(size_t index) { | ||
CPDF_Object* p = GetDirectObjectAt(index); | ||
if (!p) | ||
diff --git a/core/fpdfapi/parser/cpdf_array.h b/core/fpdfapi/parser/cpdf_array.h | ||
index 2270c7d..223cd59 100644 | ||
--- a/core/fpdfapi/parser/cpdf_array.h | ||
+++ b/core/fpdfapi/parser/cpdf_array.h | ||
@@ -62,7 +62,8 @@ | ||
bool GetBooleanAt(size_t index, bool bDefault) const; | ||
int GetIntegerAt(size_t index) const; | ||
float GetNumberAt(size_t index) const; | ||
- CPDF_Dictionary* GetDictAt(size_t index); | ||
+ RetainPtr<CPDF_Dictionary> GetMutableDictAt(size_t index); | ||
+ CPDF_Dictionary* GetDictAt(size_t index); // prefer previous form. | ||
const CPDF_Dictionary* GetDictAt(size_t index) const; | ||
CPDF_Stream* GetStreamAt(size_t index); | ||
const CPDF_Stream* GetStreamAt(size_t index) const; | ||
diff --git a/core/fpdfapi/parser/cpdf_dictionary.cpp b/core/fpdfapi/parser/cpdf_dictionary.cpp | ||
index fc671e7..047b25e 100644 | ||
--- a/core/fpdfapi/parser/cpdf_dictionary.cpp | ||
+++ b/core/fpdfapi/parser/cpdf_dictionary.cpp | ||
@@ -148,6 +148,11 @@ | ||
return p ? p->GetNumber() : 0; | ||
} | ||
|
||
+RetainPtr<CPDF_Dictionary> CPDF_Dictionary::GetMutableDictFor( | ||
+ const ByteString& key) { | ||
+ return pdfium::WrapRetain(GetDictFor(key)); | ||
+} | ||
+ | ||
const CPDF_Dictionary* CPDF_Dictionary::GetDictFor( | ||
const ByteString& key) const { | ||
const CPDF_Object* p = GetDirectObjectFor(key); | ||
@@ -165,6 +170,11 @@ | ||
static_cast<const CPDF_Dictionary*>(this)->GetDictFor(key)); | ||
} | ||
|
||
+RetainPtr<CPDF_Array> CPDF_Dictionary::GetMutableArrayFor( | ||
+ const ByteString& key) { | ||
+ return pdfium::WrapRetain(GetArrayFor(key)); | ||
+} | ||
+ | ||
const CPDF_Array* CPDF_Dictionary::GetArrayFor(const ByteString& key) const { | ||
return ToArray(GetDirectObjectFor(key)); | ||
} | ||
diff --git a/core/fpdfapi/parser/cpdf_dictionary.h b/core/fpdfapi/parser/cpdf_dictionary.h | ||
index 653b48c..c7ccdc6 100644 | ||
--- a/core/fpdfapi/parser/cpdf_dictionary.h | ||
+++ b/core/fpdfapi/parser/cpdf_dictionary.h | ||
@@ -67,9 +67,11 @@ | ||
int GetIntegerFor(const ByteString& key, int default_int) const; | ||
float GetNumberFor(const ByteString& key) const; | ||
const CPDF_Dictionary* GetDictFor(const ByteString& key) const; | ||
- CPDF_Dictionary* GetDictFor(const ByteString& key); | ||
+ CPDF_Dictionary* GetDictFor(const ByteString& key); // Prefer next form. | ||
+ RetainPtr<CPDF_Dictionary> GetMutableDictFor(const ByteString& key); | ||
const CPDF_Array* GetArrayFor(const ByteString& key) const; | ||
- CPDF_Array* GetArrayFor(const ByteString& key); | ||
+ CPDF_Array* GetArrayFor(const ByteString& key); // Prefer next form. | ||
+ RetainPtr<CPDF_Array> GetMutableArrayFor(const ByteString& key); | ||
const CPDF_Stream* GetStreamFor(const ByteString& key) const; | ||
CPDF_Stream* GetStreamFor(const ByteString& key); | ||
CFX_FloatRect GetRectFor(const ByteString& key) const; | ||
diff --git a/core/fpdfdoc/cpdf_nametree.cpp b/core/fpdfdoc/cpdf_nametree.cpp | ||
index ffec5fe..09d4a87 100644 | ||
--- a/core/fpdfdoc/cpdf_nametree.cpp | ||
+++ b/core/fpdfdoc/cpdf_nametree.cpp | ||
@@ -43,7 +43,7 @@ | ||
// Get the limit arrays that leaf array |pFind| is under in the tree with root | ||
// |pNode|. |pLimits| will hold all the limit arrays from the leaf up to before | ||
// the root. Return true if successful. | ||
-bool GetNodeAncestorsLimitsInternal(CPDF_Dictionary* pNode, | ||
+bool GetNodeAncestorsLimitsInternal(const RetainPtr<CPDF_Dictionary>& pNode, | ||
const CPDF_Array* pFind, | ||
int nLevel, | ||
std::vector<CPDF_Array*>* pLimits) { | ||
@@ -60,7 +60,7 @@ | ||
return false; | ||
|
||
for (size_t i = 0; i < pKids->size(); ++i) { | ||
- CPDF_Dictionary* pKid = pKids->GetDictAt(i); | ||
+ RetainPtr<CPDF_Dictionary> pKid = pKids->GetMutableDictAt(i); | ||
if (!pKid) | ||
continue; | ||
|
||
@@ -74,8 +74,9 @@ | ||
|
||
// Wrapper for GetNodeAncestorsLimitsInternal() so callers do not need to know | ||
// about the details. | ||
-std::vector<CPDF_Array*> GetNodeAncestorsLimits(CPDF_Dictionary* pNode, | ||
- const CPDF_Array* pFind) { | ||
+std::vector<CPDF_Array*> GetNodeAncestorsLimits( | ||
+ const RetainPtr<CPDF_Dictionary>& pNode, | ||
+ const CPDF_Array* pFind) { | ||
std::vector<CPDF_Array*> results; | ||
GetNodeAncestorsLimitsInternal(pNode, pFind, 0, &results); | ||
return results; | ||
@@ -169,21 +170,22 @@ | ||
// will be the index of |csName| in |ppFind|. If |csName| is not found, |ppFind| | ||
// will be the leaf array that |csName| should be added to, and |pFindIndex| | ||
// will be the index that it should be added at. | ||
-CPDF_Object* SearchNameNodeByNameInternal(CPDF_Dictionary* pNode, | ||
- const WideString& csName, | ||
- int nLevel, | ||
- size_t* nIndex, | ||
- CPDF_Array** ppFind, | ||
- int* pFindIndex) { | ||
+CPDF_Object* SearchNameNodeByNameInternal( | ||
+ const RetainPtr<CPDF_Dictionary>& pNode, | ||
+ const WideString& csName, | ||
+ int nLevel, | ||
+ size_t* nIndex, | ||
+ RetainPtr<CPDF_Array>* ppFind, | ||
+ int* pFindIndex) { | ||
if (nLevel > kNameTreeMaxRecursion) | ||
return nullptr; | ||
|
||
- CPDF_Array* pLimits = pNode->GetArrayFor("Limits"); | ||
- CPDF_Array* pNames = pNode->GetArrayFor("Names"); | ||
+ RetainPtr<CPDF_Array> pLimits = pNode->GetMutableArrayFor("Limits"); | ||
+ RetainPtr<CPDF_Array> pNames = pNode->GetMutableArrayFor("Names"); | ||
if (pLimits) { | ||
WideString csLeft; | ||
WideString csRight; | ||
- std::tie(csLeft, csRight) = GetNodeLimitsAndSanitize(pLimits); | ||
+ std::tie(csLeft, csRight) = GetNodeLimitsAndSanitize(pLimits.Get()); | ||
// Skip this node if the name to look for is smaller than its lower limit. | ||
if (csName.Compare(csLeft) < 0) | ||
return nullptr; | ||
@@ -222,12 +224,12 @@ | ||
} | ||
|
||
// Search through the node's children. | ||
- CPDF_Array* pKids = pNode->GetArrayFor("Kids"); | ||
+ RetainPtr<CPDF_Array> pKids = pNode->GetMutableArrayFor("Kids"); | ||
if (!pKids) | ||
return nullptr; | ||
|
||
for (size_t i = 0; i < pKids->size(); i++) { | ||
- CPDF_Dictionary* pKid = pKids->GetDictAt(i); | ||
+ RetainPtr<CPDF_Dictionary> pKid = pKids->GetMutableDictAt(i); | ||
if (!pKid) | ||
continue; | ||
|
||
@@ -241,9 +243,9 @@ | ||
|
||
// Wrapper for SearchNameNodeByNameInternal() so callers do not need to know | ||
// about the details. | ||
-CPDF_Object* SearchNameNodeByName(CPDF_Dictionary* pNode, | ||
+CPDF_Object* SearchNameNodeByName(const RetainPtr<CPDF_Dictionary>& pNode, | ||
const WideString& csName, | ||
- CPDF_Array** ppFind, | ||
+ RetainPtr<CPDF_Array>* ppFind, | ||
int* pFindIndex) { | ||
size_t nIndex = 0; | ||
return SearchNameNodeByNameInternal(pNode, csName, 0, &nIndex, ppFind, | ||
@@ -439,17 +441,17 @@ | ||
|
||
bool CPDF_NameTree::AddValueAndName(RetainPtr<CPDF_Object> pObj, | ||
const WideString& name) { | ||
- CPDF_Array* pFind = nullptr; | ||
+ RetainPtr<CPDF_Array> pFind; | ||
int nFindIndex = -1; | ||
// Handle the corner case where the root node is empty. i.e. No kids and no | ||
// names. In which case, just insert into it and skip all the searches. | ||
- CPDF_Array* pNames = m_pRoot->GetArrayFor("Names"); | ||
+ RetainPtr<CPDF_Array> pNames = m_pRoot->GetMutableArrayFor("Names"); | ||
if (pNames && pNames->IsEmpty() && !m_pRoot->GetArrayFor("Kids")) | ||
pFind = pNames; | ||
|
||
if (!pFind) { | ||
// Fail if the tree already contains this name or if the tree is too deep. | ||
- if (SearchNameNodeByName(m_pRoot.Get(), name, &pFind, &nFindIndex)) | ||
+ if (SearchNameNodeByName(m_pRoot, name, &pFind, &nFindIndex)) | ||
return false; | ||
} | ||
|
||
@@ -479,7 +481,7 @@ | ||
// Expand the limits that the newly added name is under, if the name falls | ||
// outside of the limits of its leaf array or any arrays above it. | ||
std::vector<CPDF_Array*> all_limits = | ||
- GetNodeAncestorsLimits(m_pRoot.Get(), pFind); | ||
+ GetNodeAncestorsLimits(m_pRoot, pFind.Get()); | ||
for (auto* pLimits : all_limits) { | ||
if (!pLimits) | ||
continue; | ||
@@ -525,7 +527,7 @@ | ||
} | ||
|
||
CPDF_Object* CPDF_NameTree::LookupValue(const WideString& csName) const { | ||
- return SearchNameNodeByName(m_pRoot.Get(), csName, nullptr, nullptr); | ||
+ return SearchNameNodeByName(m_pRoot, csName, nullptr, nullptr); | ||
} | ||
|
||
CPDF_Array* CPDF_NameTree::LookupNewStyleNamedDest(const ByteString& sName) { | ||
diff --git a/testing/resources/javascript/bug_1335681.in b/testing/resources/javascript/bug_1335681.in | ||
new file mode 100644 | ||
index 0000000..254e596 | ||
--- /dev/null | ||
+++ b/testing/resources/javascript/bug_1335681.in | ||
@@ -0,0 +1,38 @@ | ||
+{{header}} | ||
+{{object 1 0}} << | ||
+ /Pages 1 0 R | ||
+ /OpenAction 2 0 R | ||
+ /Names << | ||
+ /Dests 3 0 R | ||
+ >> | ||
+>> | ||
+endobj | ||
+{{object 2 0}} << | ||
+ /Type /Action | ||
+ /S /JavaScript | ||
+ /JS ( | ||
+ app.alert\("Starting"\); | ||
+ this.gotoNamedDest\(""\); | ||
+ ) | ||
+>> | ||
+endobj | ||
+{{object 3 0}} << | ||
+ /Kids 4 0 R | ||
+>> | ||
+endobj | ||
+{{object 4 0}} [ | ||
+ << >> | ||
+ << >> | ||
+ << | ||
+ /Kids [ | ||
+ << | ||
+ /Limits 4 0 R | ||
+ >> | ||
+ ] | ||
+ >> | ||
+] | ||
+endobj | ||
+{{xref}} | ||
+{{trailer}} | ||
+{{startxref}} | ||
+%%EOF | ||
diff --git a/testing/resources/javascript/bug_1335681_expected.txt b/testing/resources/javascript/bug_1335681_expected.txt | ||
new file mode 100644 | ||
index 0000000..80a6951 | ||
--- /dev/null | ||
+++ b/testing/resources/javascript/bug_1335681_expected.txt | ||
@@ -0,0 +1 @@ | ||
+Alert: Starting |