From a34067721d6630975c356a621a12753bbdab1cdb Mon Sep 17 00:00:00 2001 From: Henrique Nakashima Date: Fri, 13 Jul 2018 19:10:53 +0000 Subject: Fix mark parameters not saved if nothing changed. Bug: pdfium:1037 Change-Id: Ia2cd0d6ef99495dda3289988123489e3a2ad6e82 Reviewed-on: https://pdfium-review.googlesource.com/37750 Reviewed-by: Lei Zhang Commit-Queue: Henrique Nakashima --- core/fpdfapi/page/cpdf_contentmark.cpp | 16 +++++ core/fpdfapi/page/cpdf_contentmark.h | 2 + fpdfsdk/fpdf_edit_embeddertest.cpp | 112 ++++++++++++++++++++++++++------- fpdfsdk/fpdf_editpage.cpp | 25 ++++++++ public/fpdf_edit.h | 32 ++++++---- 5 files changed, 152 insertions(+), 35 deletions(-) diff --git a/core/fpdfapi/page/cpdf_contentmark.cpp b/core/fpdfapi/page/cpdf_contentmark.cpp index 1ff567d9e6..4932ee6231 100644 --- a/core/fpdfapi/page/cpdf_contentmark.cpp +++ b/core/fpdfapi/page/cpdf_contentmark.cpp @@ -30,6 +30,13 @@ size_t CPDF_ContentMark::CountItems() const { return m_pMarkData->CountItems(); } +bool CPDF_ContentMark::ContainsItem(const CPDF_ContentMarkItem* pItem) const { + if (!m_pMarkData) + return false; + + return m_pMarkData->ContainsItem(pItem); +} + CPDF_ContentMarkItem* CPDF_ContentMark::GetItem(size_t i) { ASSERT(i < CountItems()); return m_pMarkData->GetItem(i); @@ -105,6 +112,15 @@ size_t CPDF_ContentMark::MarkData::CountItems() const { return m_Marks.size(); } +bool CPDF_ContentMark::MarkData::ContainsItem( + const CPDF_ContentMarkItem* pItem) const { + for (const auto pMark : m_Marks) { + if (pMark.Get() == pItem) + return true; + } + return false; +} + CPDF_ContentMarkItem* CPDF_ContentMark::MarkData::GetItem(size_t index) { return m_Marks[index].Get(); } diff --git a/core/fpdfapi/page/cpdf_contentmark.h b/core/fpdfapi/page/cpdf_contentmark.h index 8bbae52418..8dc98370e6 100644 --- a/core/fpdfapi/page/cpdf_contentmark.h +++ b/core/fpdfapi/page/cpdf_contentmark.h @@ -24,6 +24,7 @@ class CPDF_ContentMark { std::unique_ptr Clone(); int GetMarkedContentID() const; size_t CountItems() const; + bool ContainsItem(const CPDF_ContentMarkItem* pItem) const; // The returned pointer is never null. CPDF_ContentMarkItem* GetItem(size_t i); @@ -45,6 +46,7 @@ class CPDF_ContentMark { ~MarkData() override; size_t CountItems() const; + bool ContainsItem(const CPDF_ContentMarkItem* pItem) const; CPDF_ContentMarkItem* GetItem(size_t index); const CPDF_ContentMarkItem* GetItem(size_t index) const; diff --git a/fpdfsdk/fpdf_edit_embeddertest.cpp b/fpdfsdk/fpdf_edit_embeddertest.cpp index 1ba227f8c3..ba790de654 100644 --- a/fpdfsdk/fpdf_edit_embeddertest.cpp +++ b/fpdfsdk/fpdf_edit_embeddertest.cpp @@ -541,7 +541,7 @@ void CheckMarkCounts(FPDF_PAGE page, FPDF_PAGEOBJECTMARK mark = FPDFPageObj_GetMark(page_object, j); char buffer[256]; - ASSERT_GT(FPDFPageObjMark_GetName(mark, buffer, 256), 0u); + ASSERT_GT(FPDFPageObjMark_GetName(mark, buffer, sizeof(buffer)), 0u); std::wstring name = GetPlatformWString(reinterpret_cast(buffer)); if (name == L"Prime") { @@ -552,7 +552,7 @@ void CheckMarkCounts(FPDF_PAGE page, EXPECT_EQ(1, FPDFPageObjMark_CountParams(mark)); unsigned long get_param_key_return = - FPDFPageObjMark_GetParamKey(mark, 0, buffer, 256); + FPDFPageObjMark_GetParamKey(mark, 0, buffer, sizeof(buffer)); ASSERT_GT(get_param_key_return, 0u); EXPECT_EQ((6u + 1u) * 2u, get_param_key_return); std::wstring key = @@ -572,7 +572,7 @@ void CheckMarkCounts(FPDF_PAGE page, EXPECT_EQ(1, FPDFPageObjMark_CountParams(mark)); unsigned long get_param_key_return = - FPDFPageObjMark_GetParamKey(mark, 0, buffer, 256); + FPDFPageObjMark_GetParamKey(mark, 0, buffer, sizeof(buffer)); ASSERT_GT(get_param_key_return, 0u); EXPECT_EQ((8u + 1u) * 2u, get_param_key_return); std::wstring key = @@ -582,19 +582,24 @@ void CheckMarkCounts(FPDF_PAGE page, EXPECT_EQ(FPDF_OBJECT_STRING, FPDFPageObjMark_GetParamValueType(mark, "Position")); unsigned long length; - EXPECT_TRUE(FPDFPageObjMark_GetParamStringValue(mark, "Position", - buffer, 256, &length)); + EXPECT_TRUE(FPDFPageObjMark_GetParamStringValue( + mark, "Position", buffer, sizeof(buffer), &length)); ASSERT_GT(length, 0u); std::wstring value = GetPlatformWString(reinterpret_cast(buffer)); - // "Position" can be "First" or "Last". + // "Position" can be "First", "Last", or "End". if (i == 0) { EXPECT_EQ((5u + 1u) * 2u, length); EXPECT_EQ(L"First", value); } else if (i == object_count - 1) { - EXPECT_EQ((4u + 1u) * 2u, length); - EXPECT_EQ(L"Last", value); + if (length == (4u + 1u) * 2u) { + EXPECT_EQ(L"Last", value); + } else if (length == (3u + 1u) * 2u) { + EXPECT_EQ(L"End", value); + } else { + FAIL(); + } } else { FAIL(); } @@ -655,7 +660,7 @@ TEST_F(FPDFEditEmbeddertest, RemoveMarkedObjectsPrime) { FPDF_PAGEOBJECTMARK mark = FPDFPageObj_GetMark(page_object, j); char buffer[256]; - ASSERT_GT(FPDFPageObjMark_GetName(mark, buffer, 256), 0u); + ASSERT_GT(FPDFPageObjMark_GetName(mark, buffer, sizeof(buffer)), 0u); std::wstring name = GetPlatformWString(reinterpret_cast(buffer)); if (name == L"Prime") { @@ -2157,8 +2162,8 @@ TEST_F(FPDFEditEmbeddertest, AddMark) { FPDF_PAGEOBJECT page_object = FPDFPage_GetObject(page, 0); FPDF_PAGEOBJECTMARK mark = FPDFPageObj_AddMark(page_object, "Bounds"); EXPECT_TRUE(mark); - EXPECT_TRUE( - FPDFPageObjMark_SetStringParam(document(), mark, "Position", "First")); + EXPECT_TRUE(FPDFPageObjMark_SetStringParam(document(), page_object, mark, + "Position", "First")); CheckMarkCounts(page, 1, kExpectedObjectCount, 8, 4, 9, 2); @@ -2177,6 +2182,68 @@ TEST_F(FPDFEditEmbeddertest, AddMark) { CloseSavedDocument(); } +TEST_F(FPDFEditEmbeddertest, SetMarkParam) { + // Load document with some text. + EXPECT_TRUE(OpenDocument("text_in_page_marked.pdf")); + FPDF_PAGE page = LoadPage(0); + ASSERT_TRUE(page); + + constexpr int kExpectedObjectCount = 19; + CheckMarkCounts(page, 1, kExpectedObjectCount, 8, 4, 9, 1); + + // Check the "Bounds" mark's "Position" param is "Last". + FPDF_PAGEOBJECT page_object = FPDFPage_GetObject(page, 18); + FPDF_PAGEOBJECTMARK mark = FPDFPageObj_GetMark(page_object, 1); + ASSERT_TRUE(mark); + char buffer[256]; + ASSERT_GT(FPDFPageObjMark_GetName(mark, buffer, sizeof(buffer)), 0u); + ASSERT_EQ(L"Bounds", + GetPlatformWString(reinterpret_cast(buffer))); + unsigned long out_buffer_len; + ASSERT_TRUE(FPDFPageObjMark_GetParamStringValue( + mark, "Position", buffer, sizeof(buffer), &out_buffer_len)); + ASSERT_EQ(L"Last", + GetPlatformWString(reinterpret_cast(buffer))); + + // Set is to "End". + EXPECT_TRUE(FPDFPageObjMark_SetStringParam(document(), page_object, mark, + "Position", "End")); + + // Verify the object passed must correspond to the mark passed. + FPDF_PAGEOBJECT another_page_object = FPDFPage_GetObject(page, 17); + EXPECT_FALSE(FPDFPageObjMark_SetStringParam(document(), another_page_object, + mark, "Position", "End")); + + // Verify nothing else changed. + CheckMarkCounts(page, 1, kExpectedObjectCount, 8, 4, 9, 1); + + // Verify "Position" now maps to "End". + EXPECT_TRUE(FPDFPageObjMark_GetParamStringValue( + mark, "Position", buffer, sizeof(buffer), &out_buffer_len)); + EXPECT_EQ(L"End", + GetPlatformWString(reinterpret_cast(buffer))); + + // Save the file + EXPECT_TRUE(FPDFPage_GenerateContent(page)); + EXPECT_TRUE(FPDF_SaveAsCopy(document(), this, 0)); + UnloadPage(page); + + // Re-open the file and cerify "Position" still maps to "End". + OpenSavedDocument(); + FPDF_PAGE saved_page = LoadSavedPage(0); + + CheckMarkCounts(saved_page, 1, kExpectedObjectCount, 8, 4, 9, 1); + page_object = FPDFPage_GetObject(saved_page, 18); + mark = FPDFPageObj_GetMark(page_object, 1); + EXPECT_TRUE(FPDFPageObjMark_GetParamStringValue( + mark, "Position", buffer, sizeof(buffer), &out_buffer_len)); + EXPECT_EQ(L"End", + GetPlatformWString(reinterpret_cast(buffer))); + + CloseSavedPage(saved_page); + CloseSavedDocument(); +} + TEST_F(FPDFEditEmbeddertest, AddMarkedText) { // Start with a blank page. FPDF_PAGE page = FPDFPage_New(CreateNewDocument(), 0, 612, 792); @@ -2206,7 +2273,7 @@ TEST_F(FPDFEditEmbeddertest, AddMarkedText) { EXPECT_EQ(1, FPDFPageObj_CountMarks(text_object)); EXPECT_EQ(mark, FPDFPageObj_GetMark(text_object, 0)); char buffer[256]; - EXPECT_GT(FPDFPageObjMark_GetName(mark, buffer, 256), 0u); + EXPECT_GT(FPDFPageObjMark_GetName(mark, buffer, sizeof(buffer)), 0u); std::wstring name = GetPlatformWString(reinterpret_cast(buffer)); EXPECT_EQ(L"TestMarkName", name); @@ -2219,11 +2286,12 @@ TEST_F(FPDFEditEmbeddertest, AddMarkedText) { char kBlobValue[kBlobLen]; memcpy(kBlobValue, "\x01\x02\x03\0BlobValue1\0\0\0BlobValue2\0", kBlobLen); EXPECT_EQ(0, FPDFPageObjMark_CountParams(mark)); - EXPECT_TRUE(FPDFPageObjMark_SetIntParam(document(), mark, "IntKey", 42)); - EXPECT_TRUE(FPDFPageObjMark_SetStringParam(document(), mark, "StringKey", - "StringValue")); - EXPECT_TRUE(FPDFPageObjMark_SetBlobParam(document(), mark, "BlobKey", - kBlobValue, kBlobLen)); + EXPECT_TRUE( + FPDFPageObjMark_SetIntParam(document(), text_object, mark, "IntKey", 42)); + EXPECT_TRUE(FPDFPageObjMark_SetStringParam(document(), text_object, mark, + "StringKey", "StringValue")); + EXPECT_TRUE(FPDFPageObjMark_SetBlobParam(document(), text_object, mark, + "BlobKey", kBlobValue, kBlobLen)); EXPECT_EQ(3, FPDFPageObjMark_CountParams(mark)); // Check the two parameters can be retrieved. @@ -2236,8 +2304,8 @@ TEST_F(FPDFEditEmbeddertest, AddMarkedText) { EXPECT_EQ(FPDF_OBJECT_STRING, FPDFPageObjMark_GetParamValueType(mark, "StringKey")); unsigned long out_buffer_len; - EXPECT_TRUE(FPDFPageObjMark_GetParamStringValue(mark, "StringKey", buffer, - 256, &out_buffer_len)); + EXPECT_TRUE(FPDFPageObjMark_GetParamStringValue( + mark, "StringKey", buffer, sizeof(buffer), &out_buffer_len)); EXPECT_GT(out_buffer_len, 0u); name = GetPlatformWString(reinterpret_cast(buffer)); EXPECT_EQ(L"StringValue", name); @@ -2245,8 +2313,8 @@ TEST_F(FPDFEditEmbeddertest, AddMarkedText) { EXPECT_EQ(FPDF_OBJECT_STRING, FPDFPageObjMark_GetParamValueType(mark, "BlobKey")); out_buffer_len = 0; - EXPECT_TRUE(FPDFPageObjMark_GetParamBlobValue(mark, "BlobKey", buffer, 256, - &out_buffer_len)); + EXPECT_TRUE(FPDFPageObjMark_GetParamBlobValue( + mark, "BlobKey", buffer, sizeof(buffer), &out_buffer_len)); EXPECT_EQ(kBlobLen, out_buffer_len); EXPECT_EQ(0, memcmp(kBlobValue, buffer, kBlobLen)); @@ -2278,7 +2346,7 @@ TEST_F(FPDFEditEmbeddertest, AddMarkedText) { EXPECT_EQ(1, FPDFPageObj_CountMarks(text_object)); mark = FPDFPageObj_GetMark(text_object, 0); EXPECT_TRUE(mark); - EXPECT_GT(FPDFPageObjMark_GetName(mark, buffer, 256), 0u); + EXPECT_GT(FPDFPageObjMark_GetName(mark, buffer, sizeof(buffer)), 0u); name = GetPlatformWString(reinterpret_cast(buffer)); EXPECT_EQ(L"TestMarkName", name); diff --git a/fpdfsdk/fpdf_editpage.cpp b/fpdfsdk/fpdf_editpage.cpp index dbf9124030..3abdcdf0fc 100644 --- a/fpdfsdk/fpdf_editpage.cpp +++ b/fpdfsdk/fpdf_editpage.cpp @@ -133,6 +133,13 @@ CPDF_Dictionary* GetOrCreateMarkParamsDict(FPDF_DOCUMENT document, return pParams; } +bool PageObjectContainsMark(CPDF_PageObject* pPageObj, + FPDF_PAGEOBJECTMARK mark) { + const CPDF_ContentMarkItem* pMarkItem = + CPDFContentMarkItemFromFPDFPageObjectMark(mark); + return pMarkItem && pPageObj->m_ContentMark.ContainsItem(pMarkItem); +} + unsigned int GetUnsignedAlpha(float alpha) { return static_cast(alpha * 255.f + 0.5f); } @@ -484,36 +491,53 @@ FPDFPageObj_HasTransparency(FPDF_PAGEOBJECT pageObject) { FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV FPDFPageObjMark_SetIntParam(FPDF_DOCUMENT document, + FPDF_PAGEOBJECT page_object, FPDF_PAGEOBJECTMARK mark, FPDF_BYTESTRING key, int value) { + CPDF_PageObject* pPageObj = CPDFPageObjectFromFPDFPageObject(page_object); + if (!pPageObj || !PageObjectContainsMark(pPageObj, mark)) + return false; + CPDF_Dictionary* pParams = GetOrCreateMarkParamsDict(document, mark); if (!pParams) return false; pParams->SetNewFor(key, value); + pPageObj->SetDirty(true); return true; } FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV FPDFPageObjMark_SetStringParam(FPDF_DOCUMENT document, + FPDF_PAGEOBJECT page_object, FPDF_PAGEOBJECTMARK mark, FPDF_BYTESTRING key, FPDF_BYTESTRING value) { + CPDF_PageObject* pPageObj = CPDFPageObjectFromFPDFPageObject(page_object); + if (!pPageObj || !PageObjectContainsMark(pPageObj, mark)) + return false; + CPDF_Dictionary* pParams = GetOrCreateMarkParamsDict(document, mark); if (!pParams) return false; pParams->SetNewFor(key, value, false); + pPageObj->SetDirty(true); return true; } FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV FPDFPageObjMark_SetBlobParam(FPDF_DOCUMENT document, + FPDF_PAGEOBJECT page_object, FPDF_PAGEOBJECTMARK mark, FPDF_BYTESTRING key, void* value, unsigned long value_len) { + CPDF_PageObject* pPageObj = CPDFPageObjectFromFPDFPageObject(page_object); + if (!pPageObj || !PageObjectContainsMark(pPageObj, mark)) + return false; + CPDF_Dictionary* pParams = GetOrCreateMarkParamsDict(document, mark); if (!pParams) return false; @@ -523,6 +547,7 @@ FPDFPageObjMark_SetBlobParam(FPDF_DOCUMENT document, pParams->SetNewFor( key, ByteString(static_cast(value), value_len), true); + pPageObj->SetDirty(true); return true; } diff --git a/public/fpdf_edit.h b/public/fpdf_edit.h index 677bce80b1..e71b4edb94 100644 --- a/public/fpdf_edit.h +++ b/public/fpdf_edit.h @@ -440,14 +440,16 @@ FPDFPageObjMark_GetParamBlobValue(FPDF_PAGEOBJECTMARK mark, // with key |key| exists, its value is set to |value|. Otherwise, it is added as // a new parameter. // -// document - handle to the document. -// mark - handle to a content mark. -// key - string key of the property. -// value - int value to set. +// document - handle to the document. +// page_object - handle to the page object with the mark. +// mark - handle to a content mark. +// key - string key of the property. +// value - int value to set. // // Returns TRUE if the operation succeeded, FALSE otherwise. FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV FPDFPageObjMark_SetIntParam(FPDF_DOCUMENT document, + FPDF_PAGEOBJECT page_object, FPDF_PAGEOBJECTMARK mark, FPDF_BYTESTRING key, int value); @@ -457,14 +459,16 @@ FPDFPageObjMark_SetIntParam(FPDF_DOCUMENT document, // with key |key| exists, its value is set to |value|. Otherwise, it is added as // a new parameter. // -// document - handle to the document. -// mark - handle to a content mark. -// key - string key of the property. -// value - string value to set. +// document - handle to the document. +// page_object - handle to the page object with the mark. +// mark - handle to a content mark. +// key - string key of the property. +// value - string value to set. // // Returns TRUE if the operation succeeded, FALSE otherwise. FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV FPDFPageObjMark_SetStringParam(FPDF_DOCUMENT document, + FPDF_PAGEOBJECT page_object, FPDF_PAGEOBJECTMARK mark, FPDF_BYTESTRING key, FPDF_BYTESTRING value); @@ -474,15 +478,17 @@ FPDFPageObjMark_SetStringParam(FPDF_DOCUMENT document, // with key |key| exists, its value is set to |value|. Otherwise, it is added as // a new parameter. // -// document - handle to the document. -// mark - handle to a content mark. -// key - string key of the property. -// value - pointer to blob value to set. -// value_len - size in bytes of |value|. +// document - handle to the document. +// page_object - handle to the page object with the mark. +// mark - handle to a content mark. +// key - string key of the property. +// value - pointer to blob value to set. +// value_len - size in bytes of |value|. // // Returns TRUE if the operation succeeded, FALSE otherwise. FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV FPDFPageObjMark_SetBlobParam(FPDF_DOCUMENT document, + FPDF_PAGEOBJECT page_object, FPDF_PAGEOBJECTMARK mark, FPDF_BYTESTRING key, void* value, -- cgit v1.2.3