[M102-LTS] Return retained const objects from SearchNameNodeByNameInternal()
M102 merge issues:
core/fpdfapi/parser/cpdf_number.h:
non-const ToNumber() isn't present in M102
core/fpdfdoc/cpdf_nametree.h/cpp:
Conflicting return types of SearchNameNodeByNameInternal(),
SearchNameNodeByNameInternal(), SearchNameNodeByName(),
GetNamedDestFromObject(), LookupOldStyleNamedDest(), LookupNamedDest(),
LookupValue(), and LookupNewStyleNamedDest() - They return non-const
pointers in M102.
fxjs/cjs_document.cpp:
Conflicting type of dest_array in gotoNamedDest().
xfa/fxfa/cxfa_ffdoc.cpp:
Conflicting type of pObject and pStream in GetPDFNamedImage().
Bug: chromium:1358075
Change-Id: I4c06f6c8c148804fa71a7ef156524be4776b2d16
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/97210
Commit-Queue: Tom Sepez <tsepez@chromium.org>
(cherry picked from commit d51720c9bb55d1163ab4fdcdc6981e753aa2354d)
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/97752
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Artem Sumaneev <asumaneev@google.com>
diff --git a/core/fpdfapi/parser/cpdf_array.h b/core/fpdfapi/parser/cpdf_array.h
index 223cd59..a2c0678 100644
--- a/core/fpdfapi/parser/cpdf_array.h
+++ b/core/fpdfapi/parser/cpdf_array.h
@@ -194,4 +194,8 @@
return RetainPtr<CPDF_Array>(ToArray(obj.Get()));
}
+inline RetainPtr<const CPDF_Array> ToArray(RetainPtr<const CPDF_Object> obj) {
+ return RetainPtr<const CPDF_Array>(ToArray(obj.Get()));
+}
+
#endif // CORE_FPDFAPI_PARSER_CPDF_ARRAY_H_
diff --git a/core/fpdfapi/parser/cpdf_dictionary.h b/core/fpdfapi/parser/cpdf_dictionary.h
index c7ccdc6..658ed65 100644
--- a/core/fpdfapi/parser/cpdf_dictionary.h
+++ b/core/fpdfapi/parser/cpdf_dictionary.h
@@ -170,4 +170,9 @@
return RetainPtr<CPDF_Dictionary>(ToDictionary(obj.Get()));
}
+inline RetainPtr<const CPDF_Dictionary> ToDictionary(
+ RetainPtr<const CPDF_Object> obj) {
+ return RetainPtr<const CPDF_Dictionary>(ToDictionary(obj.Get()));
+}
+
#endif // CORE_FPDFAPI_PARSER_CPDF_DICTIONARY_H_
diff --git a/core/fpdfapi/parser/cpdf_number.h b/core/fpdfapi/parser/cpdf_number.h
index 864bbb2..744b1d6 100644
--- a/core/fpdfapi/parser/cpdf_number.h
+++ b/core/fpdfapi/parser/cpdf_number.h
@@ -49,4 +49,8 @@
return obj ? obj->AsNumber() : nullptr;
}
+inline RetainPtr<const CPDF_Number> ToNumber(RetainPtr<const CPDF_Object> obj) {
+ return RetainPtr<const CPDF_Number>(ToNumber(obj.Get()));
+}
+
#endif // CORE_FPDFAPI_PARSER_CPDF_NUMBER_H_
diff --git a/core/fpdfapi/parser/cpdf_stream.h b/core/fpdfapi/parser/cpdf_stream.h
index 497db20..171d93e 100644
--- a/core/fpdfapi/parser/cpdf_stream.h
+++ b/core/fpdfapi/parser/cpdf_stream.h
@@ -92,4 +92,8 @@
return RetainPtr<CPDF_Stream>(ToStream(obj.Get()));
}
+inline RetainPtr<const CPDF_Stream> ToStream(RetainPtr<const CPDF_Object> obj) {
+ return RetainPtr<const CPDF_Stream>(ToStream(obj.Get()));
+}
+
#endif // CORE_FPDFAPI_PARSER_CPDF_STREAM_H_
diff --git a/core/fpdfdoc/cpdf_dest.cpp b/core/fpdfdoc/cpdf_dest.cpp
index f3b1152..fcc09d9 100644
--- a/core/fpdfdoc/cpdf_dest.cpp
+++ b/core/fpdfdoc/cpdf_dest.cpp
@@ -41,9 +41,11 @@
if (!pDest)
return CPDF_Dest(nullptr);
- if (pDest->IsString() || pDest->IsName())
- return CPDF_Dest(CPDF_NameTree::LookupNamedDest(pDoc, pDest->GetString()));
-
+ if (pDest->IsString() || pDest->IsName()) {
+ // TODO(tsepez): make CPDF_Dest constructor take retained args.
+ return CPDF_Dest(
+ CPDF_NameTree::LookupNamedDest(pDoc, pDest->GetString()).Get());
+ }
return CPDF_Dest(pDest->AsArray());
}
diff --git a/core/fpdfdoc/cpdf_nametree.cpp b/core/fpdfdoc/cpdf_nametree.cpp
index 09d4a87..2dfecbb 100644
--- a/core/fpdfdoc/cpdf_nametree.cpp
+++ b/core/fpdfdoc/cpdf_nametree.cpp
@@ -170,7 +170,7 @@
// 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(
+RetainPtr<const CPDF_Object> SearchNameNodeByNameInternal(
const RetainPtr<CPDF_Dictionary>& pNode,
const WideString& csName,
int nLevel,
@@ -217,7 +217,7 @@
continue;
*nIndex += i;
- return pNames->GetDirectObjectAt(i * 2 + 1);
+ return pdfium::WrapRetain(pNames->GetDirectObjectAt(i * 2 + 1));
}
*nIndex += dwCount;
return nullptr;
@@ -233,7 +233,7 @@
if (!pKid)
continue;
- CPDF_Object* pFound = SearchNameNodeByNameInternal(
+ RetainPtr<const CPDF_Object> pFound = SearchNameNodeByNameInternal(
pKid, csName, nLevel + 1, nIndex, ppFind, pFindIndex);
if (pFound)
return pFound;
@@ -243,10 +243,11 @@
// Wrapper for SearchNameNodeByNameInternal() so callers do not need to know
// about the details.
-CPDF_Object* SearchNameNodeByName(const RetainPtr<CPDF_Dictionary>& pNode,
- const WideString& csName,
- RetainPtr<CPDF_Array>* ppFind,
- int* pFindIndex) {
+RetainPtr<const CPDF_Object> SearchNameNodeByName(
+ const RetainPtr<CPDF_Dictionary>& pNode,
+ const WideString& csName,
+ RetainPtr<CPDF_Array>* ppFind,
+ int* pFindIndex) {
size_t nIndex = 0;
return SearchNameNodeByNameInternal(pNode, csName, 0, &nIndex, ppFind,
pFindIndex);
@@ -344,24 +345,25 @@
return nCount;
}
-CPDF_Array* GetNamedDestFromObject(CPDF_Object* obj) {
- if (!obj)
- return nullptr;
- CPDF_Array* array = obj->AsArray();
+RetainPtr<const CPDF_Array> GetNamedDestFromObject(
+ RetainPtr<const CPDF_Object> obj) {
+ RetainPtr<const CPDF_Array> array = ToArray(obj);
if (array)
return array;
- CPDF_Dictionary* dict = obj->AsDictionary();
+ RetainPtr<const CPDF_Dictionary> dict = ToDictionary(obj);
if (dict)
- return dict->GetArrayFor("D");
+ return pdfium::WrapRetain(dict->GetArrayFor("D"));
return nullptr;
}
-CPDF_Array* LookupOldStyleNamedDest(CPDF_Document* pDoc,
- const ByteString& name) {
- CPDF_Dictionary* pDests = pDoc->GetRoot()->GetDictFor("Dests");
+RetainPtr<const CPDF_Array> LookupOldStyleNamedDest(CPDF_Document* pDoc,
+ const ByteString& name) {
+ const CPDF_Dictionary* pDests = pDoc->GetRoot()->GetDictFor("Dests");
if (!pDests)
return nullptr;
- return GetNamedDestFromObject(pDests->GetDirectObjectFor(name));
+ // TODO(tsepez): return const retained objects from CPDF object getters.
+ return GetNamedDestFromObject(
+ pdfium::WrapRetain(pDests->GetDirectObjectFor(name)));
}
} // namespace
@@ -424,9 +426,10 @@
}
// static
-CPDF_Array* CPDF_NameTree::LookupNamedDest(CPDF_Document* pDoc,
- const ByteString& name) {
- CPDF_Array* dest_array = nullptr;
+RetainPtr<const CPDF_Array> CPDF_NameTree::LookupNamedDest(
+ CPDF_Document* pDoc,
+ const ByteString& name) {
+ RetainPtr<const CPDF_Array> dest_array;
std::unique_ptr<CPDF_NameTree> name_tree = Create(pDoc, "Dests");
if (name_tree)
dest_array = name_tree->LookupNewStyleNamedDest(name);
@@ -526,10 +529,12 @@
return result.value().value;
}
-CPDF_Object* CPDF_NameTree::LookupValue(const WideString& csName) const {
+RetainPtr<const CPDF_Object> CPDF_NameTree::LookupValue(
+ const WideString& csName) const {
return SearchNameNodeByName(m_pRoot, csName, nullptr, nullptr);
}
-CPDF_Array* CPDF_NameTree::LookupNewStyleNamedDest(const ByteString& sName) {
+RetainPtr<const CPDF_Array> CPDF_NameTree::LookupNewStyleNamedDest(
+ const ByteString& sName) {
return GetNamedDestFromObject(LookupValue(PDF_DecodeText(sName.raw_span())));
}
diff --git a/core/fpdfdoc/cpdf_nametree.h b/core/fpdfdoc/cpdf_nametree.h
index e27f5b1..30371b4 100644
--- a/core/fpdfdoc/cpdf_nametree.h
+++ b/core/fpdfdoc/cpdf_nametree.h
@@ -38,14 +38,14 @@
static std::unique_ptr<CPDF_NameTree> CreateForTesting(
CPDF_Dictionary* pRoot);
- static CPDF_Array* LookupNamedDest(CPDF_Document* doc,
- const ByteString& name);
+ static RetainPtr<const CPDF_Array> LookupNamedDest(CPDF_Document* doc,
+ const ByteString& name);
bool AddValueAndName(RetainPtr<CPDF_Object> pObj, const WideString& name);
bool DeleteValueAndName(size_t nIndex);
CPDF_Object* LookupValueAndName(size_t nIndex, WideString* csName) const;
- CPDF_Object* LookupValue(const WideString& csName) const;
+ RetainPtr<const CPDF_Object> LookupValue(const WideString& csName) const;
size_t GetCount() const;
CPDF_Dictionary* GetRootForTesting() const { return m_pRoot.Get(); }
@@ -53,7 +53,7 @@
private:
explicit CPDF_NameTree(CPDF_Dictionary* pRoot);
- CPDF_Array* LookupNewStyleNamedDest(const ByteString& name);
+ RetainPtr<const CPDF_Array> LookupNewStyleNamedDest(const ByteString& name);
const RetainPtr<CPDF_Dictionary> m_pRoot;
};
diff --git a/core/fpdfdoc/cpdf_nametree_unittest.cpp b/core/fpdfdoc/cpdf_nametree_unittest.cpp
index 36617e7..e144033 100644
--- a/core/fpdfdoc/cpdf_nametree_unittest.cpp
+++ b/core/fpdfdoc/cpdf_nametree_unittest.cpp
@@ -120,7 +120,7 @@
EXPECT_STREQ(L"1", stored_name.c_str());
// Check that the correct value object can be obtained by looking up "1".
- const CPDF_Number* pNumber = ToNumber(name_tree->LookupValue(L"1"));
+ RetainPtr<const CPDF_Number> pNumber = ToNumber(name_tree->LookupValue(L"1"));
ASSERT_TRUE(pNumber);
EXPECT_EQ(100, pNumber->GetInteger());
}
@@ -140,7 +140,8 @@
std::unique_ptr<CPDF_NameTree> name_tree =
CPDF_NameTree::CreateForTesting(pRootDict.Get());
- const CPDF_Number* pNumber = ToNumber(name_tree->LookupValue(L"9.txt"));
+ RetainPtr<const CPDF_Number> pNumber =
+ ToNumber(name_tree->LookupValue(L"9.txt"));
ASSERT_TRUE(pNumber);
EXPECT_EQ(999, pNumber->GetInteger());
CheckLimitsArray(pKid1, "1.txt", "9.txt");
diff --git a/fpdfsdk/fpdf_view.cpp b/fpdfsdk/fpdf_view.cpp
index 161340b..b89efdd 100644
--- a/fpdfsdk/fpdf_view.cpp
+++ b/fpdfsdk/fpdf_view.cpp
@@ -1050,7 +1050,9 @@
return nullptr;
ByteString dest_name(name);
- return FPDFDestFromCPDFArray(CPDF_NameTree::LookupNamedDest(pDoc, dest_name));
+ // TODO(tsepez): murky ownership, should caller get a reference?
+ return FPDFDestFromCPDFArray(
+ CPDF_NameTree::LookupNamedDest(pDoc, dest_name).Get());
}
#ifdef PDF_ENABLE_V8
diff --git a/fxjs/cjs_document.cpp b/fxjs/cjs_document.cpp
index a0bcf86..991c3aa 100644
--- a/fxjs/cjs_document.cpp
+++ b/fxjs/cjs_document.cpp
@@ -1395,12 +1395,13 @@
return CJS_Result::Failure(JSMessage::kBadObjectError);
CPDF_Document* pDocument = m_pFormFillEnv->GetPDFDocument();
- CPDF_Array* dest_array = CPDF_NameTree::LookupNamedDest(
+ RetainPtr<const CPDF_Array> dest_array = CPDF_NameTree::LookupNamedDest(
pDocument, pRuntime->ToByteString(params[0]));
if (!dest_array)
return CJS_Result::Failure(JSMessage::kBadObjectError);
- CPDF_Dest dest(dest_array);
+ // TODO(tsepez): make CPDF_Dest constructor take retained argument.
+ CPDF_Dest dest(dest_array.Get());
const CPDF_Array* arrayObject = dest.GetArray();
std::vector<float> scrollPositionArray;
if (arrayObject) {
diff --git a/testing/resources/javascript/bug_1358075.in b/testing/resources/javascript/bug_1358075.in
new file mode 100644
index 0000000..b503bf2
--- /dev/null
+++ b/testing/resources/javascript/bug_1358075.in
@@ -0,0 +1,39 @@
+{{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 (
+ this.gotoNamedDest\("2"\);
+ app.alert\("completed"\);
+ )
+>>
+endobj
+{{object 3 0}} <<
+ /Kids 4 0 R
+>>
+endobj
+{{object 4 0}} [
+ (1)
+ (3)
+ <<
+ /Kids [
+ <<
+ /Limits 4 0 R
+ /Names [(2) []]
+ >>
+ ]
+ >>
+]
+endobj
+{{xref}}
+{{trailer}}
+{{startxref}}
+%%EOF
diff --git a/testing/resources/javascript/bug_1358075_expected.txt b/testing/resources/javascript/bug_1358075_expected.txt
new file mode 100644
index 0000000..13d460b
--- /dev/null
+++ b/testing/resources/javascript/bug_1358075_expected.txt
@@ -0,0 +1 @@
+Alert: completed
diff --git a/xfa/fxfa/cxfa_ffdoc.cpp b/xfa/fxfa/cxfa_ffdoc.cpp
index 691248d..5693d23 100644
--- a/xfa/fxfa/cxfa_ffdoc.cpp
+++ b/xfa/fxfa/cxfa_ffdoc.cpp
@@ -279,7 +279,8 @@
if (count == 0)
return nullptr;
- CPDF_Object* pObject = name_tree->LookupValue(WideString(wsName));
+ RetainPtr<const CPDF_Object> pObject =
+ name_tree->LookupValue(WideString(wsName));
if (!pObject) {
for (size_t i = 0; i < count; ++i) {
WideString wsTemp;
@@ -291,11 +292,12 @@
}
}
- CPDF_Stream* pStream = ToStream(pObject);
+ RetainPtr<const CPDF_Stream> pStream = ToStream(pObject);
if (!pStream)
return nullptr;
- auto pAcc = pdfium::MakeRetain<CPDF_StreamAcc>(pStream);
+ // TODO(tsepez): make CPDF_StreamAcc constructor take retained argument.
+ auto pAcc = pdfium::MakeRetain<CPDF_StreamAcc>(pStream.Get());
pAcc->LoadAllDataFiltered();
auto pImageFileRead =
