Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: cherry-pick 3466cc056b05 from pdfium #35099

Merged
merged 2 commits into from Jul 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion patches/config.json
Expand Up @@ -23,5 +23,7 @@

"src/electron/patches/ReactiveObjC": "src/third_party/squirrel.mac/vendor/ReactiveObjC",

"src/electron/patches/angle": "src/third_party/angle"
"src/electron/patches/angle": "src/third_party/angle",

"src/electron/patches/pdfium": "src/third_party/pdfium"
}
1 change: 1 addition & 0 deletions patches/pdfium/.patches
@@ -0,0 +1 @@
cherry-pick-3466cc056b05.patch
283 changes: 283 additions & 0 deletions patches/pdfium/cherry-pick-3466cc056b05.patch
@@ -0,0 +1,283 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Tom Sepez <tsepez@chromium.org>
Date: Tue, 12 Jul 2022 18:13:14 +0000
Subject: 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 708e6778a8d5383c2fd15518e091bb439212a1c7..6aa6f3089f191799231d49949de7f60adf367b5a 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 @@ float CPDF_Array::GetNumberAt(size_t index) const {
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 2270c7d9c8f22bbac3d67c8c5b6570bc128a72b4..223cd59ab7cb6cc2c08071118d1fbb3241904c6b 100644
--- a/core/fpdfapi/parser/cpdf_array.h
+++ b/core/fpdfapi/parser/cpdf_array.h
@@ -62,7 +62,8 @@ class CPDF_Array final : public CPDF_Object {
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 fc671e76bfb6f17af93a19cca57aa0532c4111e4..047b25e2c0b539a00814de6cd80d8f224413586b 100644
--- a/core/fpdfapi/parser/cpdf_dictionary.cpp
+++ b/core/fpdfapi/parser/cpdf_dictionary.cpp
@@ -148,6 +148,11 @@ float CPDF_Dictionary::GetNumberFor(const ByteString& key) const {
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 @@ CPDF_Dictionary* CPDF_Dictionary::GetDictFor(const ByteString& key) {
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 d2f814327e7b880d68119aa938e73af1260668d5..fe990efaa9844fc6a7aa2c6fd671d74a2446383a 100644
--- a/core/fpdfapi/parser/cpdf_dictionary.h
+++ b/core/fpdfapi/parser/cpdf_dictionary.h
@@ -67,9 +67,11 @@ class CPDF_Dictionary final : public CPDF_Object {
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 dfd4e619c055229135b67bb056cb546992fe1d21..20b68b5874ff14b5625c6fc028211ce44b53a119 100644
--- a/core/fpdfdoc/cpdf_nametree.cpp
+++ b/core/fpdfdoc/cpdf_nametree.cpp
@@ -42,7 +42,7 @@ std::pair<WideString, WideString> GetNodeLimitsAndSanitize(
// 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) {
@@ -59,7 +59,7 @@ bool GetNodeAncestorsLimitsInternal(CPDF_Dictionary* pNode,
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;

@@ -73,8 +73,9 @@ bool GetNodeAncestorsLimitsInternal(CPDF_Dictionary* pNode,

// 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;
@@ -168,21 +169,22 @@ bool UpdateNodesAndLimitsUponDeletion(CPDF_Dictionary* pNode,
// 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;
@@ -221,12 +223,12 @@ CPDF_Object* SearchNameNodeByNameInternal(CPDF_Dictionary* pNode,
}

// 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;

@@ -240,9 +242,9 @@ CPDF_Object* SearchNameNodeByNameInternal(CPDF_Dictionary* pNode,

// 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,
@@ -438,17 +440,17 @@ size_t CPDF_NameTree::GetCount() const {

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

@@ -478,7 +480,7 @@ bool CPDF_NameTree::AddValueAndName(RetainPtr<CPDF_Object> pObj,
// 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;
@@ -524,7 +526,7 @@ CPDF_Object* CPDF_NameTree::LookupValueAndName(size_t nIndex,
}

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 0000000000000000000000000000000000000000..254e5964518c40029fcde1e0f31f3e2703d9b85f
--- /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 0000000000000000000000000000000000000000..80a6951c49c8c8ef9a4e0036f7d4875c38373b89
--- /dev/null
+++ b/testing/resources/javascript/bug_1335681_expected.txt
@@ -0,0 +1 @@
+Alert: Starting