Make CPDF_ContentMarkItem stop caching the properties dict.
It could be aliased with some other dictionary in the file. We
note that the dictionary one level up will always be an indirect
object in the sharing case, and indirect objects are persisted
by the IndirectObjectHolder, so hold a pointer to that and retrieve
the specific property_name field on the fly.
Bug: chromium:900552
Change-Id: I2e300020d6a7191648dd139a485b6d284e259976
Reviewed-on: https://pdfium-review.googlesource.com/c/44970
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/core/fpdfapi/page/cpdf_contentmarkitem.cpp b/core/fpdfapi/page/cpdf_contentmarkitem.cpp
index 4b084b6..d831fc2 100644
--- a/core/fpdfapi/page/cpdf_contentmarkitem.cpp
+++ b/core/fpdfapi/page/cpdf_contentmarkitem.cpp
@@ -18,7 +18,7 @@
const CPDF_Dictionary* CPDF_ContentMarkItem::GetParam() const {
switch (m_ParamType) {
case kPropertiesDict:
- return m_pPropertiesDict.Get();
+ return m_pPropertiesHolder->GetDictFor(m_PropertyName);
case kDirectDict:
return m_pDirectDict.get();
case kNone:
@@ -43,9 +43,10 @@
m_pDirectDict = std::move(pDict);
}
-void CPDF_ContentMarkItem::SetPropertiesDict(CPDF_Dictionary* pDict,
- const ByteString& property_name) {
+void CPDF_ContentMarkItem::SetPropertiesHolder(
+ CPDF_Dictionary* pHolder,
+ const ByteString& property_name) {
m_ParamType = kPropertiesDict;
- m_pPropertiesDict = pDict;
+ m_pPropertiesHolder = pHolder;
m_PropertyName = property_name;
}
diff --git a/core/fpdfapi/page/cpdf_contentmarkitem.h b/core/fpdfapi/page/cpdf_contentmarkitem.h
index 939dd31..c01ca21 100644
--- a/core/fpdfapi/page/cpdf_contentmarkitem.h
+++ b/core/fpdfapi/page/cpdf_contentmarkitem.h
@@ -32,14 +32,14 @@
bool HasMCID() const;
void SetDirectDict(std::unique_ptr<CPDF_Dictionary> pDict);
- void SetPropertiesDict(CPDF_Dictionary* pDict,
- const ByteString& property_name);
+ void SetPropertiesHolder(CPDF_Dictionary* pHolder,
+ const ByteString& property_name);
private:
ParamType m_ParamType = kNone;
ByteString m_MarkName;
ByteString m_PropertyName;
- UnownedPtr<CPDF_Dictionary> m_pPropertiesDict;
+ UnownedPtr<CPDF_Dictionary> m_pPropertiesHolder;
std::unique_ptr<CPDF_Dictionary> m_pDirectDict;
};
diff --git a/core/fpdfapi/page/cpdf_contentmarks.cpp b/core/fpdfapi/page/cpdf_contentmarks.cpp
index caaf6e3..75d5034 100644
--- a/core/fpdfapi/page/cpdf_contentmarks.cpp
+++ b/core/fpdfapi/page/cpdf_contentmarks.cpp
@@ -56,12 +56,12 @@
m_pMarkData->AddMarkWithDirectDict(std::move(name), pDict);
}
-void CPDF_ContentMarks::AddMarkWithPropertiesDict(
- ByteString name,
+void CPDF_ContentMarks::AddMarkWithPropertiesHolder(
+ const ByteString& name,
CPDF_Dictionary* pDict,
const ByteString& property_name) {
EnsureMarkDataExists();
- m_pMarkData->AddMarkWithPropertiesDict(std::move(name), pDict, property_name);
+ m_pMarkData->AddMarkWithPropertiesHolder(name, pDict, property_name);
}
bool CPDF_ContentMarks::RemoveMark(CPDF_ContentMarkItem* pMarkItem) {
@@ -147,12 +147,12 @@
m_Marks.push_back(pItem);
}
-void CPDF_ContentMarks::MarkData::AddMarkWithPropertiesDict(
- ByteString name,
+void CPDF_ContentMarks::MarkData::AddMarkWithPropertiesHolder(
+ const ByteString& name,
CPDF_Dictionary* pDict,
const ByteString& property_name) {
auto pItem = pdfium::MakeRetain<CPDF_ContentMarkItem>(std::move(name));
- pItem->SetPropertiesDict(pDict, property_name);
+ pItem->SetPropertiesHolder(pDict, property_name);
m_Marks.push_back(pItem);
}
diff --git a/core/fpdfapi/page/cpdf_contentmarks.h b/core/fpdfapi/page/cpdf_contentmarks.h
index 335592b..5c05936 100644
--- a/core/fpdfapi/page/cpdf_contentmarks.h
+++ b/core/fpdfapi/page/cpdf_contentmarks.h
@@ -32,9 +32,9 @@
void AddMark(ByteString name);
void AddMarkWithDirectDict(ByteString name, CPDF_Dictionary* pDict);
- void AddMarkWithPropertiesDict(ByteString name,
- CPDF_Dictionary* pDict,
- const ByteString& property_name);
+ void AddMarkWithPropertiesHolder(const ByteString& name,
+ CPDF_Dictionary* pHolder,
+ const ByteString& property_name);
bool RemoveMark(CPDF_ContentMarkItem* pMarkItem);
void DeleteLastMark();
size_t FindFirstDifference(const CPDF_ContentMarks* other) const;
@@ -54,9 +54,9 @@
int GetMarkedContentID() const;
void AddMark(ByteString name);
void AddMarkWithDirectDict(ByteString name, CPDF_Dictionary* pDict);
- void AddMarkWithPropertiesDict(ByteString name,
- CPDF_Dictionary* pDict,
- const ByteString& property_name);
+ void AddMarkWithPropertiesHolder(const ByteString& name,
+ CPDF_Dictionary* pHolder,
+ const ByteString& property_name);
bool RemoveMark(CPDF_ContentMarkItem* pMarkItem);
void DeleteLastMark();
diff --git a/core/fpdfapi/page/cpdf_streamcontentparser.cpp b/core/fpdfapi/page/cpdf_streamcontentparser.cpp
index 5a9d6b6..df9fa3f 100644
--- a/core/fpdfapi/page/cpdf_streamcontentparser.cpp
+++ b/core/fpdfapi/page/cpdf_streamcontentparser.cpp
@@ -591,30 +591,26 @@
}
void CPDF_StreamContentParser::Handle_BeginMarkedContent_Dictionary() {
- ByteString tag = GetString(1);
CPDF_Object* pProperty = GetObject(0);
if (!pProperty)
return;
- bool bIndirect = pProperty->IsName();
- ByteString property_name;
- if (bIndirect) {
- property_name = pProperty->GetString();
- pProperty = FindResourceObj("Properties", property_name);
- if (!pProperty)
+ ByteString tag = GetString(1);
+ std::unique_ptr<CPDF_ContentMarks> new_marks =
+ m_ContentMarksStack.top()->Clone();
+
+ if (pProperty->IsName()) {
+ ByteString property_name = pProperty->GetString();
+ CPDF_Dictionary* pHolder = FindResourceHolder("Properties");
+ if (!pHolder || !pHolder->GetDictFor(property_name))
return;
+ new_marks->AddMarkWithPropertiesHolder(tag, pHolder, property_name);
+ } else if (pProperty->IsDictionary()) {
+ new_marks->AddMarkWithDirectDict(tag, pProperty->AsDictionary());
+ } else {
+ return;
}
- if (CPDF_Dictionary* pDict = pProperty->AsDictionary()) {
- std::unique_ptr<CPDF_ContentMarks> new_marks =
- m_ContentMarksStack.top()->Clone();
- if (bIndirect) {
- new_marks->AddMarkWithPropertiesDict(std::move(tag), pDict,
- property_name);
- } else {
- new_marks->AddMarkWithDirectDict(std::move(tag), pDict);
- }
- m_ContentMarksStack.push(std::move(new_marks));
- }
+ m_ContentMarksStack.push(std::move(new_marks));
}
void CPDF_StreamContentParser::Handle_BeginImage() {
@@ -1139,18 +1135,25 @@
}
}
-CPDF_Object* CPDF_StreamContentParser::FindResourceObj(const ByteString& type,
- const ByteString& name) {
+CPDF_Dictionary* CPDF_StreamContentParser::FindResourceHolder(
+ const ByteString& type) {
if (!m_pResources)
return nullptr;
+
CPDF_Dictionary* pDict = m_pResources->GetDictFor(type);
if (pDict)
- return pDict->GetDirectObjectFor(name);
+ return pDict;
+
if (m_pResources == m_pPageResources || !m_pPageResources)
return nullptr;
- CPDF_Dictionary* pPageDict = m_pPageResources->GetDictFor(type);
- return pPageDict ? pPageDict->GetDirectObjectFor(name) : nullptr;
+ return m_pPageResources->GetDictFor(type);
+}
+
+CPDF_Object* CPDF_StreamContentParser::FindResourceObj(const ByteString& type,
+ const ByteString& name) {
+ CPDF_Dictionary* pHolder = FindResourceHolder(type);
+ return pHolder ? pHolder->GetDirectObjectFor(name) : nullptr;
}
CPDF_Font* CPDF_StreamContentParser::FindFont(const ByteString& name) {
diff --git a/core/fpdfapi/page/cpdf_streamcontentparser.h b/core/fpdfapi/page/cpdf_streamcontentparser.h
index 05556ff..53253c0 100644
--- a/core/fpdfapi/page/cpdf_streamcontentparser.h
+++ b/core/fpdfapi/page/cpdf_streamcontentparser.h
@@ -119,6 +119,7 @@
bool bGraph);
CPDF_ColorSpace* FindColorSpace(const ByteString& name);
CPDF_Pattern* FindPattern(const ByteString& name, bool bShading);
+ CPDF_Dictionary* FindResourceHolder(const ByteString& type);
CPDF_Object* FindResourceObj(const ByteString& type, const ByteString& name);
// Takes ownership of |pImageObj|, returns unowned pointer to it.
diff --git a/fpdfsdk/fpdf_formfill_embeddertest.cpp b/fpdfsdk/fpdf_formfill_embeddertest.cpp
index f5b39ae..da0adb2 100644
--- a/fpdfsdk/fpdf_formfill_embeddertest.cpp
+++ b/fpdfsdk/fpdf_formfill_embeddertest.cpp
@@ -12,6 +12,7 @@
#include "public/cpp/fpdf_scopers.h"
#include "public/fpdf_formfill.h"
#include "public/fpdf_fwlevent.h"
+#include "public/fpdf_progressive.h"
#include "testing/embedder_test.h"
#include "testing/embedder_test_mock_delegate.h"
#include "testing/embedder_test_timer_handling_delegate.h"
@@ -385,6 +386,24 @@
UnloadPage(page);
}
+TEST_F(FPDFFormFillEmbeddertest, BUG_900552) {
+ EmbedderTestTimerHandlingDelegate delegate;
+ SetDelegate(&delegate);
+
+ EXPECT_TRUE(OpenDocument("bug_900552.pdf"));
+ FPDF_PAGE page = LoadPage(0);
+ ASSERT_TRUE(page);
+ DoOpenActions();
+ delegate.AdvanceTime(4000);
+
+ // Simulate a repaint.
+ FPDF_BITMAP bitmap = FPDFBitmap_Create(512, 512, 0);
+ ASSERT_TRUE(bitmap);
+ FPDF_RenderPageBitmap_Start(bitmap, page, 0, 0, 512, 512, 0, 0, nullptr);
+ FPDFBitmap_Destroy(bitmap);
+ UnloadPage(page);
+}
+
class DoURIActionBlockedDelegate final : public EmbedderTest::Delegate {
public:
void DoURIAction(FPDF_BYTESTRING uri) override {
diff --git a/testing/resources/bug_900552.in b/testing/resources/bug_900552.in
new file mode 100644
index 0000000..08763f8
--- /dev/null
+++ b/testing/resources/bug_900552.in
@@ -0,0 +1,76 @@
+{{header}}
+{{object 1 0}} <<
+ /Type /Catalog
+ /Pages 2 0 R
+ /OpenAction 8 0 R
+>>
+{{object 2 0}} <<
+ /Type /Pages
+ /Count 1
+ /Kids [3 0 R]
+>>
+endobj
+{{object 3 0}} <<
+ /Type /Page
+ /Parent 2 0 R
+ /MediaBox [0 0 600 700]
+ /Resources 5 0 R
+ /Contents 4 0 R
+ /Annots [6 0 R]
+>>
+endobj
+{{object 4 0}} <<
+ {{streamlen}}
+>>
+stream
+ (OC)
+ /V
+ BDC
+ BT
+ /F1 20 Tf
+ 100 100 Td
+ (Test) Tj
+ ET
+endstream
+endobj
+{{object 5 0}} <<
+ /Properties 6 0 R
+ /Font <<F1 7 0 R>>
+>>
+endobj
+{{object 6 0}} <<
+ /FT /Tx
+ /Type /Annot
+ /Subtype /Widget
+ /T (txt1)
+ /F 4
+ /Rect [200 200 400 400]
+ /V <</A (b)>>
+>>
+endobj
+{{object 7 0}} <<
+ /Type /Font
+ /Subtype /Type1
+ /BaseFont /Helvetica
+>>
+endobj
+{{object 8 0}} <<
+ /Type /Action
+ /S /JavaScript
+ /JS 9 0 R
+>>
+endobj
+{{object 9 0}} <<
+ {{streamlen}}
+>>
+stream
+function run() {
+ this.getField('txt1').value = 'a';
+}
+app.setTimeOut('run()', 1000)
+endstream
+endobj
+{{xref}}
+{{trailer}}
+{{startxref}}
+%%EOF
diff --git a/testing/resources/bug_900552.pdf b/testing/resources/bug_900552.pdf
new file mode 100644
index 0000000..573e8ae
--- /dev/null
+++ b/testing/resources/bug_900552.pdf
@@ -0,0 +1,92 @@
+%PDF-1.7
+% ò¤ô
+1 0 obj <<
+ /Type /Catalog
+ /Pages 2 0 R
+ /OpenAction 8 0 R
+>>
+2 0 obj <<
+ /Type /Pages
+ /Count 1
+ /Kids [3 0 R]
+>>
+endobj
+3 0 obj <<
+ /Type /Page
+ /Parent 2 0 R
+ /MediaBox [0 0 600 700]
+ /Resources 5 0 R
+ /Contents 4 0 R
+ /Annots [6 0 R]
+>>
+endobj
+4 0 obj <<
+ /Length 68
+>>
+stream
+ (OC)
+ /V
+ BDC
+ BT
+ /F1 20 Tf
+ 100 100 Td
+ (Test) Tj
+ ET
+endstream
+endobj
+5 0 obj <<
+ /Properties 6 0 R
+ /Font <<F1 7 0 R>>
+>>
+endobj
+6 0 obj <<
+ /FT /Tx
+ /Type /Annot
+ /Subtype /Widget
+ /T (txt1)
+ /F 4
+ /Rect [200 200 400 400]
+ /V <</A (b)>>
+>>
+endobj
+7 0 obj <<
+ /Type /Font
+ /Subtype /Type1
+ /BaseFont /Helvetica
+>>
+endobj
+8 0 obj <<
+ /Type /Action
+ /S /JavaScript
+ /JS 9 0 R
+>>
+endobj
+9 0 obj <<
+ /Length 86
+>>
+stream
+function run() {
+ this.getField('txt1').value = 'a';
+}
+app.setTimeOut('run()', 1000)
+endstream
+endobj
+xref
+0 10
+0000000000 65535 f
+0000000015 00000 n
+0000000081 00000 n
+0000000144 00000 n
+0000000276 00000 n
+0000000395 00000 n
+0000000457 00000 n
+0000000583 00000 n
+0000000659 00000 n
+0000000725 00000 n
+trailer <<
+ /Root 1 0 R
+ /Size 10
+>>
+startxref
+862
+%%EOF
