Create new stream objects when changing annotation appearance streams.
Avoid editing existing streams, as they may be shared.
Update affected JS tests as a result. The new behavior better matches
other PDF JS implementations.
Bug: chromium:1302455
Change-Id: Ib0f56b6f9eda29a94a2b98e758c77a84b2ef36ec
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/91290
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/core/fpdfapi/parser/cpdf_document.cpp b/core/fpdfapi/parser/cpdf_document.cpp
index 4a2ec62..4e71482 100644
--- a/core/fpdfapi/parser/cpdf_document.cpp
+++ b/core/fpdfapi/parser/cpdf_document.cpp
@@ -14,6 +14,7 @@
#include "core/fpdfapi/parser/cpdf_parser.h"
#include "core/fpdfapi/parser/cpdf_read_validator.h"
#include "core/fpdfapi/parser/cpdf_reference.h"
+#include "core/fpdfapi/parser/cpdf_stream.h"
#include "core/fpdfapi/parser/fpdf_parser_utility.h"
#include "core/fxcodec/jbig2/JBig2_DocumentContext.h"
#include "core/fxcrt/fx_codepage.h"
@@ -312,6 +313,16 @@
return m_pCodecContext.get();
}
+CPDF_Stream* CPDF_Document::CreateModifiedAPStream() {
+ auto* stream = NewIndirect<CPDF_Stream>();
+ m_ModifiedAPStreamIDs.insert(stream->GetObjNum());
+ return stream;
+}
+
+bool CPDF_Document::IsModifiedAPStream(const CPDF_Stream* stream) const {
+ return stream && pdfium::Contains(m_ModifiedAPStreamIDs, stream->GetObjNum());
+}
+
int CPDF_Document::GetPageIndex(uint32_t objnum) {
uint32_t skip_count = 0;
bool bSkipped = false;
diff --git a/core/fpdfapi/parser/cpdf_document.h b/core/fpdfapi/parser/cpdf_document.h
index 9eaa5ca..d5b59aa 100644
--- a/core/fpdfapi/parser/cpdf_document.h
+++ b/core/fpdfapi/parser/cpdf_document.h
@@ -112,6 +112,12 @@
m_pLinksContext = std::move(pContext);
}
+ // Behaves like NewIndirect<CPDF_Stream>(), but keeps track of the new stream.
+ CPDF_Stream* CreateModifiedAPStream();
+
+ // Returns whether CreateModifiedAPStream() created `stream`.
+ bool IsModifiedAPStream(const CPDF_Stream* stream) const;
+
// CPDF_Parser::ParsedObjectsHolder:
bool TryInit() override;
RetainPtr<CPDF_Object> ParseIndirectObject(uint32_t objnum) override;
@@ -191,6 +197,7 @@
std::unique_ptr<PageDataIface> m_pDocPage; // Must be after |m_pDocRender|.
std::unique_ptr<JBig2_DocumentContext> m_pCodecContext;
std::unique_ptr<LinkListIface> m_pLinksContext;
+ std::set<uint32_t> m_ModifiedAPStreamIDs;
std::vector<uint32_t> m_PageList; // Page number to page's dict objnum.
// Must be second to last.
diff --git a/fpdfsdk/cpdfsdk_appstream.cpp b/fpdfsdk/cpdfsdk_appstream.cpp
index 41f2330..027cb1f 100644
--- a/fpdfsdk/cpdfsdk_appstream.cpp
+++ b/fpdfsdk/cpdfsdk_appstream.cpp
@@ -1844,24 +1844,27 @@
void CPDFSDK_AppStream::Write(const ByteString& sAPType,
const ByteString& sContents,
const ByteString& sAPState) {
- CPDF_Stream* pStream = nullptr;
- CPDF_Dictionary* pParentDict = nullptr;
+ CPDF_Dictionary* pParentDict;
+ ByteString key;
if (sAPState.IsEmpty()) {
pParentDict = dict_.Get();
- pStream = dict_->GetStreamFor(sAPType);
+ key = sAPType;
} else {
CPDF_Dictionary* pAPTypeDict = dict_->GetDictFor(sAPType);
if (!pAPTypeDict)
pAPTypeDict = dict_->SetNewFor<CPDF_Dictionary>(sAPType);
pParentDict = pAPTypeDict;
- pStream = pAPTypeDict->GetStreamFor(sAPState);
+ key = sAPState;
}
- if (!pStream) {
- CPDF_Document* doc = widget_->GetPageView()->GetPDFDocument();
- pStream = doc->NewIndirect<CPDF_Stream>();
- pParentDict->SetNewFor<CPDF_Reference>(sAPType, doc, pStream->GetObjNum());
+ // If `pStream` is created by CreateModifiedAPStream(), then it is safe to
+ // edit, as it is not shared.
+ CPDF_Stream* pStream = pParentDict->GetStreamFor(key);
+ CPDF_Document* doc = widget_->GetPageView()->GetPDFDocument();
+ if (!doc->IsModifiedAPStream(pStream)) {
+ pStream = doc->CreateModifiedAPStream();
+ pParentDict->SetNewFor<CPDF_Reference>(key, doc, pStream->GetObjNum());
}
CPDF_Dictionary* pStreamDict = pStream->GetDict();
diff --git a/fpdfsdk/fpdf_formfill_embeddertest.cpp b/fpdfsdk/fpdf_formfill_embeddertest.cpp
index 2dcfcd7..269187f 100644
--- a/fpdfsdk/fpdf_formfill_embeddertest.cpp
+++ b/fpdfsdk/fpdf_formfill_embeddertest.cpp
@@ -1405,6 +1405,127 @@
UnloadPage(page);
}
+TEST_F(FPDFFormFillEmbedderTest, Bug1302455RenderOnly) {
+#if defined(_SKIA_SUPPORT_) || defined(_SKIA_SUPPORT_PATHS_)
+ const char kChecksum[] = "520c4415c9977f40d6b4af5a0a94d764";
+#else
+ const char kChecksum[] = "bbee92af1daec2340c81f482878744d8";
+#endif
+ {
+ ASSERT_TRUE(OpenDocument("bug_1302455.pdf"));
+ FPDF_PAGE page = LoadPage(0);
+ ASSERT_TRUE(page);
+
+ ScopedFPDFBitmap bitmap = RenderLoadedPage(page);
+ CompareBitmap(bitmap.get(), 300, 300, kChecksum);
+
+ EXPECT_TRUE(FPDF_SaveAsCopy(document(), this, 0));
+
+ UnloadPage(page);
+ }
+ VerifySavedDocument(300, 300, kChecksum);
+}
+
+TEST_F(FPDFFormFillEmbedderTest, Bug1302455EditFirstForm) {
+#if defined(_SKIA_SUPPORT_) || defined(_SKIA_SUPPORT_PATHS_)
+ const char kChecksum[] = "29a06da3e47f67535e266b090a5ac82d";
+#elif BUILDFLAG(IS_APPLE)
+ const char kChecksum[] = "bf5423874f188427d2500a2bc4abebbe";
+#else
+ const char kChecksum[] = "6a4ac9a15d2c34589616c8f2b05fbedd";
+#endif
+ {
+ ASSERT_TRUE(OpenDocument("bug_1302455.pdf"));
+ FPDF_PAGE page = LoadPage(0);
+ ASSERT_TRUE(page);
+
+ EXPECT_EQ(FPDF_FORMFIELD_TEXTFIELD,
+ FPDFPage_HasFormFieldAtPoint(form_handle(), page, 110, 110));
+ FORM_OnMouseMove(form_handle(), page, 0, 110, 110);
+ FORM_OnLButtonDown(form_handle(), page, 0, 110, 110);
+ FORM_OnLButtonUp(form_handle(), page, 0, 110, 110);
+ FORM_OnChar(form_handle(), page, 'A', 0);
+
+ FORM_ForceToKillFocus(form_handle());
+ ScopedFPDFBitmap bitmap = RenderLoadedPage(page);
+ CompareBitmap(bitmap.get(), 300, 300, kChecksum);
+
+ EXPECT_TRUE(FPDF_SaveAsCopy(document(), this, 0));
+
+ UnloadPage(page);
+ }
+ VerifySavedDocument(300, 300, kChecksum);
+}
+
+TEST_F(FPDFFormFillEmbedderTest, Bug1302455EditSecondForm) {
+#if defined(_SKIA_SUPPORT_) || defined(_SKIA_SUPPORT_PATHS_)
+ const char kChecksum[] = "19f8574d6378ee36e349376d88b7a2c4";
+#elif BUILDFLAG(IS_APPLE)
+ const char kChecksum[] = "8a0fd8772dba6e1e952e49d159cc64b5";
+#else
+ const char kChecksum[] = "45a7694933c2ba3c5dc8f6cc18b79175";
+#endif
+ {
+ ASSERT_TRUE(OpenDocument("bug_1302455.pdf"));
+ FPDF_PAGE page = LoadPage(0);
+ ASSERT_TRUE(page);
+
+ EXPECT_EQ(FPDF_FORMFIELD_TEXTFIELD,
+ FPDFPage_HasFormFieldAtPoint(form_handle(), page, 110, 170));
+ FORM_OnMouseMove(form_handle(), page, 0, 110, 170);
+ FORM_OnLButtonDown(form_handle(), page, 0, 110, 170);
+ FORM_OnLButtonUp(form_handle(), page, 0, 110, 170);
+ FORM_OnChar(form_handle(), page, 'B', 0);
+
+ FORM_ForceToKillFocus(form_handle());
+ ScopedFPDFBitmap bitmap = RenderLoadedPage(page);
+ CompareBitmap(bitmap.get(), 300, 300, kChecksum);
+
+ EXPECT_TRUE(FPDF_SaveAsCopy(document(), this, 0));
+
+ UnloadPage(page);
+ }
+ VerifySavedDocument(300, 300, kChecksum);
+}
+
+TEST_F(FPDFFormFillEmbedderTest, Bug1302455EditBothForms) {
+#if defined(_SKIA_SUPPORT_) || defined(_SKIA_SUPPORT_PATHS_)
+ const char kChecksum[] = "edbc9b0e190118a9039fffc11e494081";
+#elif BUILDFLAG(IS_APPLE)
+ const char kChecksum[] = "1f422ee1c520ad74b1a993b64bd4dc4a";
+#else
+ const char kChecksum[] = "13984969b1e141079ab5f4aa80185463";
+#endif
+ {
+ ASSERT_TRUE(OpenDocument("bug_1302455.pdf"));
+ FPDF_PAGE page = LoadPage(0);
+ ASSERT_TRUE(page);
+
+ EXPECT_EQ(FPDF_FORMFIELD_TEXTFIELD,
+ FPDFPage_HasFormFieldAtPoint(form_handle(), page, 110, 110));
+ FORM_OnMouseMove(form_handle(), page, 0, 110, 110);
+ FORM_OnLButtonDown(form_handle(), page, 0, 110, 110);
+ FORM_OnLButtonUp(form_handle(), page, 0, 110, 110);
+ FORM_OnChar(form_handle(), page, 'A', 0);
+
+ EXPECT_EQ(FPDF_FORMFIELD_TEXTFIELD,
+ FPDFPage_HasFormFieldAtPoint(form_handle(), page, 110, 170));
+ FORM_OnMouseMove(form_handle(), page, 0, 110, 170);
+ FORM_OnLButtonDown(form_handle(), page, 0, 110, 170);
+ FORM_OnLButtonUp(form_handle(), page, 0, 110, 170);
+ FORM_OnChar(form_handle(), page, 'B', 0);
+
+ FORM_ForceToKillFocus(form_handle());
+ ScopedFPDFBitmap bitmap = RenderLoadedPage(page);
+ CompareBitmap(bitmap.get(), 300, 300, kChecksum);
+
+ EXPECT_TRUE(FPDF_SaveAsCopy(document(), this, 0));
+
+ UnloadPage(page);
+ }
+ VerifySavedDocument(300, 300, kChecksum);
+}
+
TEST_F(FPDFFormFillEmbedderTest, RemoveFormFieldHighlight) {
#if defined(_SKIA_SUPPORT_) || defined(_SKIA_SUPPORT_PATHS_)
const char kMd5NoHighlight[] = "013aa241c39c02505d9525550be04e48";
diff --git a/testing/resources/bug_1302455.in b/testing/resources/bug_1302455.in
new file mode 100644
index 0000000..1e18f12
--- /dev/null
+++ b/testing/resources/bug_1302455.in
@@ -0,0 +1,75 @@
+{{header}}
+{{object 1 0}} <<
+ /Type /Catalog
+ /Pages 2 0 R
+ /AcroForm <<
+ /DR 4 0 R
+ /Fields [6 0 R 7 0 R]
+ >>
+>>
+endobj
+{{object 2 0}} <<
+ /Type /Pages
+ /Kids [3 0 R]
+ /Count 1
+>>
+endobj
+{{object 3 0}} <<
+ /Type /Page
+ /Parent 2 0 R
+ /Annots [6 0 R 7 0 R]
+ /MediaBox [0 0 300 300]
+ /Resources 4 0 R
+>>
+endobj
+{{object 4 0}} <<
+ /Font <<
+ /F1 5 0 R
+ >>
+>>
+endobj
+{{object 5 0}} <<
+ /Type /Font
+ /Subtype /Type1
+ /BaseFont /Helvetica
+>>
+endobj
+{{object 6 0}} <<
+ /Type /Annot
+ /Subtype /Widget
+ /FT /Tx
+ /AP <<
+ /N 8 0 R
+ >>
+ /F 4
+ /Rect [100 100 200 130]
+ /T (Text Box 1)
+>>
+endobj
+{{object 7 0}} <<
+ /Type /Annot
+ /Subtype /Widget
+ /FT /Tx
+ /AP <<
+ /N 8 0 R
+ >>
+ /F 4
+ /Rect [100 160 200 190]
+ /T (Text Box 2)
+>>
+endobj
+{{object 8 0}} <<
+ /Type /XObject
+ /Subtype /Form
+ /BBox [0 0 100 30]
+ {{streamlen}}
+>>
+stream
+1 0 0 rg
+0 0 100 30 re f
+endstream
+endobj
+{{xref}}
+{{trailer}}
+{{startxref}}
+%%EOF
diff --git a/testing/resources/bug_1302455.pdf b/testing/resources/bug_1302455.pdf
new file mode 100644
index 0000000..a9782c4
--- /dev/null
+++ b/testing/resources/bug_1302455.pdf
@@ -0,0 +1,90 @@
+%PDF-1.7
+% ò¤ô
+1 0 obj <<
+ /Type /Catalog
+ /Pages 2 0 R
+ /AcroForm <<
+ /DR 4 0 R
+ /Fields [6 0 R 7 0 R]
+ >>
+>>
+endobj
+2 0 obj <<
+ /Type /Pages
+ /Kids [3 0 R]
+ /Count 1
+>>
+endobj
+3 0 obj <<
+ /Type /Page
+ /Parent 2 0 R
+ /Annots [6 0 R 7 0 R]
+ /MediaBox [0 0 300 300]
+ /Resources 4 0 R
+>>
+endobj
+4 0 obj <<
+ /Font <<
+ /F1 5 0 R
+ >>
+>>
+endobj
+5 0 obj <<
+ /Type /Font
+ /Subtype /Type1
+ /BaseFont /Helvetica
+>>
+endobj
+6 0 obj <<
+ /Type /Annot
+ /Subtype /Widget
+ /FT /Tx
+ /AP <<
+ /N 8 0 R
+ >>
+ /F 4
+ /Rect [100 100 200 130]
+ /T (Text Box 1)
+>>
+endobj
+7 0 obj <<
+ /Type /Annot
+ /Subtype /Widget
+ /FT /Tx
+ /AP <<
+ /N 8 0 R
+ >>
+ /F 4
+ /Rect [100 160 200 190]
+ /T (Text Box 2)
+>>
+endobj
+8 0 obj <<
+ /Type /XObject
+ /Subtype /Form
+ /BBox [0 0 100 30]
+ /Length 25
+>>
+stream
+1 0 0 rg
+0 0 100 30 re f
+endstream
+endobj
+xref
+0 9
+0000000000 65535 f
+0000000015 00000 n
+0000000128 00000 n
+0000000191 00000 n
+0000000311 00000 n
+0000000362 00000 n
+0000000438 00000 n
+0000000581 00000 n
+0000000724 00000 n
+trailer <<
+ /Root 1 0 R
+ /Size 9
+>>
+startxref
+855
+%%EOF
diff --git a/testing/resources/javascript/field_properties.in b/testing/resources/javascript/field_properties.in
index 1961f6a..0a84430d 100644
--- a/testing/resources/javascript/field_properties.in
+++ b/testing/resources/javascript/field_properties.in
@@ -116,7 +116,7 @@
function testRadioButtonPropertiesCase(field) {
try {
- testROProperty(field, "exportValues", "N");
+ testROProperty(field, "exportValues", "Yes");
testRIProperty(field, "radiosInUnison", false);
testRIProperty(field, "style", "check");
testROProperty(field, "type", "radiobutton");
@@ -129,7 +129,7 @@
function testCheckBoxPropertiesCase(field) {
try {
- testROProperty(field, "exportValues", "N");
+ testROProperty(field, "exportValues", "Yes");
testRIProperty(field, "style", "check");
testROProperty(field, "type", "checkbox");
testRIProperty(field, "value", "Off");
diff --git a/testing/resources/javascript/field_properties_expected.txt b/testing/resources/javascript/field_properties_expected.txt
index 8670196..b3d8195 100644
--- a/testing/resources/javascript/field_properties_expected.txt
+++ b/testing/resources/javascript/field_properties_expected.txt
@@ -96,7 +96,7 @@
Alert: PASS: type threw Field.type: Operation not supported.
Alert: PASS: buttonPosition = 7
Alert: PASS: buttonPosition = 7
-Alert: PASS: exportValues = N
+Alert: PASS: exportValues = Yes
Alert: PASS: exportValues threw Field.exportValues: Object no longer exists.
Alert: PASS: radiosInUnison = false
Alert: PASS: radiosInUnison = false
@@ -108,7 +108,7 @@
Alert: PASS: value = Off
Alert: PASS: valueAsString = Off
Alert: PASS: valueAsString threw Field.valueAsString: Operation not supported.
-Alert: PASS: exportValues = N
+Alert: PASS: exportValues = Yes
Alert: PASS: exportValues threw Field.exportValues: Object no longer exists.
Alert: PASS: style = check
Alert: PASS: style = check
