From c3099d1c694251a654edc6cb72df8adb5e2268ab Mon Sep 17 00:00:00 2001 From: Henrique Nakashima Date: Tue, 18 Sep 2018 18:08:15 +0000 Subject: Change signature of FPDFPageObjMark_Get(Name|ParamKey). These methods used to return the size of the buffer to contain the value to be returned. Now they will return an FPDF_BOOL to make it consistent with the other mark APIs and more intuitive to differentiate a success from a failure. The size will be returned through an out param. This CL also adds more focused testing for these API methods and similar ones. Change-Id: I6f9837f99d955aaba2c49a259ed7805a286e091d Reviewed-on: https://pdfium-review.googlesource.com/42411 Commit-Queue: Henrique Nakashima Reviewed-by: Lei Zhang --- fpdfsdk/fpdf_edit_embeddertest.cpp | 215 ++++++++++++++++++++++++++++++++++--- fpdfsdk/fpdf_editpage.cpp | 30 ++++-- 2 files changed, 220 insertions(+), 25 deletions(-) (limited to 'fpdfsdk') diff --git a/fpdfsdk/fpdf_edit_embeddertest.cpp b/fpdfsdk/fpdf_edit_embeddertest.cpp index 3bacb0b2eb..4250091fce 100644 --- a/fpdfsdk/fpdf_edit_embeddertest.cpp +++ b/fpdfsdk/fpdf_edit_embeddertest.cpp @@ -541,7 +541,11 @@ void CheckMarkCounts(FPDF_PAGE page, FPDF_PAGEOBJECTMARK mark = FPDFPageObj_GetMark(page_object, j); char buffer[256]; - ASSERT_GT(FPDFPageObjMark_GetName(mark, buffer, sizeof(buffer)), 0u); + unsigned long name_len = 999u; + ASSERT_TRUE( + FPDFPageObjMark_GetName(mark, buffer, sizeof(buffer), &name_len)); + EXPECT_GT(name_len, 0u); + EXPECT_NE(999u, name_len); std::wstring name = GetPlatformWString(reinterpret_cast(buffer)); if (name == L"Prime") { @@ -551,9 +555,9 @@ void CheckMarkCounts(FPDF_PAGE page, int expected_square = start_from + i; EXPECT_EQ(1, FPDFPageObjMark_CountParams(mark)); - unsigned long get_param_key_return = - FPDFPageObjMark_GetParamKey(mark, 0, buffer, sizeof(buffer)); - ASSERT_GT(get_param_key_return, 0u); + unsigned long get_param_key_return = 999u; + ASSERT_TRUE(FPDFPageObjMark_GetParamKey(mark, 0, buffer, sizeof(buffer), + &get_param_key_return)); EXPECT_EQ((6u + 1u) * 2u, get_param_key_return); std::wstring key = GetPlatformWString(reinterpret_cast(buffer)); @@ -571,9 +575,9 @@ void CheckMarkCounts(FPDF_PAGE page, bounds_count++; EXPECT_EQ(1, FPDFPageObjMark_CountParams(mark)); - unsigned long get_param_key_return = - FPDFPageObjMark_GetParamKey(mark, 0, buffer, sizeof(buffer)); - ASSERT_GT(get_param_key_return, 0u); + unsigned long get_param_key_return = 999u; + ASSERT_TRUE(FPDFPageObjMark_GetParamKey(mark, 0, buffer, sizeof(buffer), + &get_param_key_return)); EXPECT_EQ((8u + 1u) * 2u, get_param_key_return); std::wstring key = GetPlatformWString(reinterpret_cast(buffer)); @@ -660,7 +664,11 @@ TEST_F(FPDFEditEmbeddertest, RemoveMarkedObjectsPrime) { FPDF_PAGEOBJECTMARK mark = FPDFPageObj_GetMark(page_object, j); char buffer[256]; - ASSERT_GT(FPDFPageObjMark_GetName(mark, buffer, sizeof(buffer)), 0u); + unsigned long name_len = 999u; + ASSERT_TRUE( + FPDFPageObjMark_GetName(mark, buffer, sizeof(buffer), &name_len)); + EXPECT_GT(name_len, 0u); + EXPECT_NE(999u, name_len); std::wstring name = GetPlatformWString(reinterpret_cast(buffer)); if (name == L"Prime") { @@ -726,7 +734,11 @@ TEST_F(FPDFEditEmbeddertest, RemoveMarks) { FPDF_PAGEOBJECTMARK mark = FPDFPageObj_GetMark(page_object, j); char buffer[256]; - ASSERT_GT(FPDFPageObjMark_GetName(mark, buffer, sizeof(buffer)), 0u); + unsigned long name_len = 999u; + ASSERT_TRUE( + FPDFPageObjMark_GetName(mark, buffer, sizeof(buffer), &name_len)); + EXPECT_GT(name_len, 0u); + EXPECT_NE(999u, name_len); std::wstring name = GetPlatformWString(reinterpret_cast(buffer)); if (name == L"Prime") { @@ -775,7 +787,11 @@ TEST_F(FPDFEditEmbeddertest, RemoveMarkParam) { FPDF_PAGEOBJECTMARK mark = FPDFPageObj_GetMark(page_object, j); char buffer[256]; - ASSERT_GT(FPDFPageObjMark_GetName(mark, buffer, sizeof(buffer)), 0u); + unsigned long name_len = 999u; + ASSERT_TRUE( + FPDFPageObjMark_GetName(mark, buffer, sizeof(buffer), &name_len)); + EXPECT_GT(name_len, 0u); + EXPECT_NE(999u, name_len); std::wstring name = GetPlatformWString(reinterpret_cast(buffer)); if (name == L"Square") { @@ -812,7 +828,11 @@ TEST_F(FPDFEditEmbeddertest, RemoveMarkParam) { FPDF_PAGEOBJECTMARK mark = FPDFPageObj_GetMark(page_object, j); char buffer[256]; - ASSERT_GT(FPDFPageObjMark_GetName(mark, buffer, sizeof(buffer)), 0u); + unsigned long name_len = 999u; + ASSERT_TRUE( + FPDFPageObjMark_GetName(mark, buffer, sizeof(buffer), &name_len)); + EXPECT_GT(name_len, 0u); + EXPECT_NE(999u, name_len); std::wstring name = GetPlatformWString(reinterpret_cast(buffer)); if (name == L"Square") { @@ -2497,7 +2517,9 @@ TEST_F(FPDFEditEmbeddertest, SetMarkParam) { FPDF_PAGEOBJECTMARK mark = FPDFPageObj_GetMark(page_object, 1); ASSERT_TRUE(mark); char buffer[256]; - ASSERT_GT(FPDFPageObjMark_GetName(mark, buffer, sizeof(buffer)), 0u); + unsigned long name_len = 999u; + ASSERT_TRUE(FPDFPageObjMark_GetName(mark, buffer, sizeof(buffer), &name_len)); + EXPECT_EQ((6u + 1u) * 2u, name_len); ASSERT_EQ(L"Bounds", GetPlatformWString(reinterpret_cast(buffer))); unsigned long out_buffer_len; @@ -2573,7 +2595,9 @@ 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, sizeof(buffer)), 0u); + unsigned long name_len = 999u; + ASSERT_TRUE(FPDFPageObjMark_GetName(mark, buffer, sizeof(buffer), &name_len)); + EXPECT_EQ((14u + 1u) * 2, name_len); std::wstring name = GetPlatformWString(reinterpret_cast(buffer)); EXPECT_EQ(L"Test Mark Name", name); @@ -2603,10 +2627,11 @@ TEST_F(FPDFEditEmbeddertest, AddMarkedText) { EXPECT_EQ(FPDF_OBJECT_STRING, FPDFPageObjMark_GetParamValueType(mark, "StringKey")); - unsigned long out_buffer_len; + unsigned long out_buffer_len = 999u; EXPECT_TRUE(FPDFPageObjMark_GetParamStringValue( mark, "StringKey", buffer, sizeof(buffer), &out_buffer_len)); EXPECT_GT(out_buffer_len, 0u); + EXPECT_NE(999u, out_buffer_len); name = GetPlatformWString(reinterpret_cast(buffer)); EXPECT_EQ(L"StringValue", name); @@ -2648,7 +2673,10 @@ 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, sizeof(buffer)), 0u); + + name_len = 999u; + ASSERT_TRUE(FPDFPageObjMark_GetName(mark, buffer, sizeof(buffer), &name_len)); + EXPECT_EQ((14u + 1u) * 2, name_len); name = GetPlatformWString(reinterpret_cast(buffer)); EXPECT_EQ(L"Test Mark Name", name); @@ -2656,6 +2684,163 @@ TEST_F(FPDFEditEmbeddertest, AddMarkedText) { CloseSavedDocument(); } +TEST_F(FPDFEditEmbeddertest, MarkGetName) { + EXPECT_TRUE(OpenDocument("text_in_page_marked.pdf")); + FPDF_PAGE page = LoadPage(0); + ASSERT_TRUE(page); + FPDF_PAGEOBJECT page_object = FPDFPage_GetObject(page, 18); + FPDF_PAGEOBJECTMARK mark = FPDFPageObj_GetMark(page_object, 1); + ASSERT_TRUE(mark); + + char buffer[256]; + unsigned long out_len; + + // Show the positive cases of FPDFPageObjMark_GetName. + out_len = 999u; + EXPECT_TRUE(FPDFPageObjMark_GetName(mark, nullptr, 0, &out_len)); + EXPECT_EQ((6u + 1u) * 2u, out_len); + + out_len = 999u; + EXPECT_TRUE(FPDFPageObjMark_GetName(mark, buffer, sizeof(buffer), &out_len)); + EXPECT_EQ(L"Bounds", + GetPlatformWString(reinterpret_cast(buffer))); + EXPECT_EQ((6u + 1u) * 2u, out_len); + + // Show the negative cases of FPDFPageObjMark_GetName. + out_len = 999u; + EXPECT_FALSE( + FPDFPageObjMark_GetName(nullptr, buffer, sizeof(buffer), &out_len)); + EXPECT_EQ(999u, out_len); + + EXPECT_FALSE(FPDFPageObjMark_GetName(mark, buffer, sizeof(buffer), nullptr)); + + UnloadPage(page); +} + +TEST_F(FPDFEditEmbeddertest, MarkGetParamKey) { + EXPECT_TRUE(OpenDocument("text_in_page_marked.pdf")); + FPDF_PAGE page = LoadPage(0); + ASSERT_TRUE(page); + FPDF_PAGEOBJECT page_object = FPDFPage_GetObject(page, 18); + FPDF_PAGEOBJECTMARK mark = FPDFPageObj_GetMark(page_object, 1); + ASSERT_TRUE(mark); + + char buffer[256]; + unsigned long out_len; + + // Show the positive cases of FPDFPageObjMark_GetParamKey. + out_len = 999u; + EXPECT_TRUE(FPDFPageObjMark_GetParamKey(mark, 0, nullptr, 0, &out_len)); + EXPECT_EQ((8u + 1u) * 2u, out_len); + + out_len = 999u; + EXPECT_TRUE( + FPDFPageObjMark_GetParamKey(mark, 0, buffer, sizeof(buffer), &out_len)); + EXPECT_EQ(L"Position", + GetPlatformWString(reinterpret_cast(buffer))); + EXPECT_EQ((8u + 1u) * 2u, out_len); + + // Show the negative cases of FPDFPageObjMark_GetParamKey. + out_len = 999u; + EXPECT_FALSE(FPDFPageObjMark_GetParamKey(nullptr, 0, buffer, sizeof(buffer), + &out_len)); + EXPECT_EQ(999u, out_len); + + out_len = 999u; + EXPECT_FALSE( + FPDFPageObjMark_GetParamKey(mark, 1, buffer, sizeof(buffer), &out_len)); + EXPECT_EQ(999u, out_len); + + EXPECT_FALSE( + FPDFPageObjMark_GetParamKey(mark, 0, buffer, sizeof(buffer), nullptr)); + + UnloadPage(page); +} + +TEST_F(FPDFEditEmbeddertest, MarkGetIntParam) { + EXPECT_TRUE(OpenDocument("text_in_page_marked.pdf")); + FPDF_PAGE page = LoadPage(0); + ASSERT_TRUE(page); + FPDF_PAGEOBJECT page_object = FPDFPage_GetObject(page, 8); + FPDF_PAGEOBJECTMARK mark = FPDFPageObj_GetMark(page_object, 0); + ASSERT_TRUE(mark); + + int out_value; + + // Show the positive cases of FPDFPageObjMark_GetParamIntValue. + out_value = 999; + EXPECT_TRUE(FPDFPageObjMark_GetParamIntValue(mark, "Factor", &out_value)); + EXPECT_EQ(3, out_value); + + // Show the negative cases of FPDFPageObjMark_GetParamIntValue. + out_value = 999; + EXPECT_FALSE(FPDFPageObjMark_GetParamIntValue(nullptr, "Factor", &out_value)); + EXPECT_EQ(999, out_value); + + out_value = 999; + EXPECT_FALSE(FPDFPageObjMark_GetParamIntValue(mark, "ParamThatDoesNotExist", + &out_value)); + EXPECT_EQ(999, out_value); + + EXPECT_FALSE(FPDFPageObjMark_GetParamIntValue(mark, "Factor", nullptr)); + + page_object = FPDFPage_GetObject(page, 18); + mark = FPDFPageObj_GetMark(page_object, 1); + out_value = 999; + EXPECT_FALSE(FPDFPageObjMark_GetParamIntValue(mark, "Position", &out_value)); + EXPECT_EQ(999, out_value); + + UnloadPage(page); +} + +TEST_F(FPDFEditEmbeddertest, MarkGetStringParam) { + EXPECT_TRUE(OpenDocument("text_in_page_marked.pdf")); + FPDF_PAGE page = LoadPage(0); + ASSERT_TRUE(page); + FPDF_PAGEOBJECT page_object = FPDFPage_GetObject(page, 18); + FPDF_PAGEOBJECTMARK mark = FPDFPageObj_GetMark(page_object, 1); + ASSERT_TRUE(mark); + + char buffer[256]; + unsigned long out_len; + + // Show the positive cases of FPDFPageObjMark_GetParamStringValue. + out_len = 999u; + EXPECT_TRUE(FPDFPageObjMark_GetParamStringValue(mark, "Position", nullptr, 0, + &out_len)); + EXPECT_EQ((4u + 1u) * 2u, out_len); + + out_len = 999u; + EXPECT_TRUE(FPDFPageObjMark_GetParamStringValue(mark, "Position", buffer, + sizeof(buffer), &out_len)); + EXPECT_EQ(L"Last", + GetPlatformWString(reinterpret_cast(buffer))); + EXPECT_EQ((4u + 1u) * 2u, out_len); + + // Show the negative cases of FPDFPageObjMark_GetParamStringValue. + out_len = 999u; + EXPECT_FALSE(FPDFPageObjMark_GetParamStringValue(nullptr, "Position", buffer, + sizeof(buffer), &out_len)); + EXPECT_EQ(999u, out_len); + + out_len = 999u; + EXPECT_FALSE(FPDFPageObjMark_GetParamStringValue( + mark, "ParamThatDoesNotExist", buffer, sizeof(buffer), &out_len)); + EXPECT_EQ(999u, out_len); + + EXPECT_FALSE(FPDFPageObjMark_GetParamStringValue(mark, "Position", buffer, + sizeof(buffer), nullptr)); + + page_object = FPDFPage_GetObject(page, 8); + mark = FPDFPageObj_GetMark(page_object, 0); + out_len = 999u; + EXPECT_FALSE(FPDFPageObjMark_GetParamStringValue(mark, "Factor", buffer, + sizeof(buffer), &out_len)); + EXPECT_EQ(999u, out_len); + + UnloadPage(page); +} + TEST_F(FPDFEditEmbeddertest, ExtractImageBitmap) { ASSERT_TRUE(OpenDocument("embedded_images.pdf")); FPDF_PAGE page = LoadPage(0); diff --git a/fpdfsdk/fpdf_editpage.cpp b/fpdfsdk/fpdf_editpage.cpp index 30049342ca..c71f7e27be 100644 --- a/fpdfsdk/fpdf_editpage.cpp +++ b/fpdfsdk/fpdf_editpage.cpp @@ -345,19 +345,21 @@ FPDFPageObj_RemoveMark(FPDF_PAGEOBJECT page_object, FPDF_PAGEOBJECTMARK mark) { return result; } -FPDF_EXPORT unsigned long FPDF_CALLCONV +FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV FPDFPageObjMark_GetName(FPDF_PAGEOBJECTMARK mark, void* buffer, - unsigned long buflen) { - if (!mark) - return 0; + unsigned long buflen, + unsigned long* out_buflen) { + if (!mark || !out_buflen) + return false; const CPDF_ContentMarkItem* pMarkItem = CPDFContentMarkItemFromFPDFPageObjectMark(mark); - return Utf16EncodeMaybeCopyAndReturnLength( + *out_buflen = Utf16EncodeMaybeCopyAndReturnLength( WideString::FromUTF8(pMarkItem->GetName().AsStringView()), buffer, buflen); + return true; } FPDF_EXPORT int FPDF_CALLCONV @@ -372,24 +374,29 @@ FPDFPageObjMark_CountParams(FPDF_PAGEOBJECTMARK mark) { return pParams ? pParams->GetCount() : 0; } -FPDF_EXPORT unsigned long FPDF_CALLCONV +FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV FPDFPageObjMark_GetParamKey(FPDF_PAGEOBJECTMARK mark, unsigned long index, void* buffer, - unsigned long buflen) { + unsigned long buflen, + unsigned long* out_buflen) { + if (!out_buflen) + return false; + const CPDF_Dictionary* pParams = GetMarkParamDict(mark); if (!pParams) - return 0; + return false; for (auto& it : *pParams) { if (index == 0) { - return Utf16EncodeMaybeCopyAndReturnLength( + *out_buflen = Utf16EncodeMaybeCopyAndReturnLength( WideString::FromUTF8(it.first.AsStringView()), buffer, buflen); + return true; } --index; } - return 0; + return false; } FPDF_EXPORT FPDF_OBJECT_TYPE FPDF_CALLCONV @@ -407,6 +414,9 @@ FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV FPDFPageObjMark_GetParamIntValue(FPDF_PAGEOBJECTMARK mark, FPDF_BYTESTRING key, int* out_value) { + if (!out_value) + return false; + const CPDF_Dictionary* pParams = GetMarkParamDict(mark); if (!pParams) return false; -- cgit v1.2.3