diff options
author | Henrique Nakashima <hnakashima@chromium.org> | 2018-09-18 18:08:15 +0000 |
---|---|---|
committer | Chromium commit bot <commit-bot@chromium.org> | 2018-09-18 18:08:15 +0000 |
commit | c3099d1c694251a654edc6cb72df8adb5e2268ab (patch) | |
tree | 527e4cfa4a2efdaeef4ed6d5f5d93ec953ef8e83 | |
parent | 7ea611fb82308b4b8f9dc150a9d79334435c53b6 (diff) | |
download | pdfium-c3099d1c694251a654edc6cb72df8adb5e2268ab.tar.xz |
Change signature of FPDFPageObjMark_Get(Name|ParamKey).chromium/3556
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 <hnakashima@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
-rw-r--r-- | fpdfsdk/fpdf_edit_embeddertest.cpp | 215 | ||||
-rw-r--r-- | fpdfsdk/fpdf_editpage.cpp | 30 | ||||
-rw-r--r-- | public/fpdf_edit.h | 66 |
3 files changed, 260 insertions, 51 deletions
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<unsigned short*>(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<unsigned short*>(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<unsigned short*>(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<unsigned short*>(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<unsigned short*>(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<unsigned short*>(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<unsigned short*>(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<unsigned short*>(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<unsigned short*>(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<unsigned short*>(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<unsigned short*>(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<unsigned short*>(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<unsigned short*>(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<unsigned short*>(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; diff --git a/public/fpdf_edit.h b/public/fpdf_edit.h index dcf6201fe8..180d50247e 100644 --- a/public/fpdf_edit.h +++ b/public/fpdf_edit.h @@ -332,18 +332,23 @@ FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV FPDFPageObj_RemoveMark(FPDF_PAGEOBJECT page_object, FPDF_PAGEOBJECTMARK mark); // Experimental API. -// Get name of a content mark. |buffer| is only modified if |buflen| is longer -// than the length of the name. +// Get the name of a content mark. // -// mark - handle to a content mark. -// buffer - buffer for holding the returned name in UTF16-LE. -// buflen - length of the buffer. +// mark - handle to a content mark. +// buffer - buffer for holding the returned name in UTF16-LE. This is only +// modified if |buflen| is longer than the length of the name. +// Optional, pass null to just retrieve the size of the buffer +// needed. +// buflen - length of the buffer. +// out_buflen - pointer to variable that will receive the minimum buffer size +// to contain the name. Not filled if FALSE is returned. // -// Returns the length of the name. -FPDF_EXPORT unsigned long FPDF_CALLCONV +// Returns TRUE if the operation succeeded, FALSE if it failed. +FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV FPDFPageObjMark_GetName(FPDF_PAGEOBJECTMARK mark, void* buffer, - unsigned long buflen); + unsigned long buflen, + unsigned long* out_buflen); // Experimental API. // Get the number of key/value pair parameters in |mark|. @@ -356,20 +361,25 @@ FPDF_EXPORT int FPDF_CALLCONV FPDFPageObjMark_CountParams(FPDF_PAGEOBJECTMARK mark); // Experimental API. -// Get the key of a property in a content mark. |buffer| is only modified if -// |buflen| is longer than the length of the key. +// Get the key of a property in a content mark. // -// mark - handle to a content mark. -// index - index of the property. -// buffer - buffer for holding the returned key in UTF16-LE. -// buflen - length of the buffer. +// mark - handle to a content mark. +// index - index of the property. +// buffer - buffer for holding the returned key in UTF16-LE. This is only +// modified if |buflen| is longer than the length of the key. +// Optional, pass null to just retrieve the size of the buffer +// needed. +// buflen - length of the buffer. +// out_buflen - pointer to variable that will receive the minimum buffer size +// to contain the key. Not filled if FALSE is returned. // -// Returns the length of the key. -FPDF_EXPORT unsigned long FPDF_CALLCONV +// Returns TRUE if the operation was successful, FALSE otherwise. +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); // Experimental API. // Get the type of the value of a property in a content mark by key. @@ -400,14 +410,17 @@ FPDFPageObjMark_GetParamIntValue(FPDF_PAGEOBJECTMARK mark, // Experimental API. // Get the value of a string property in a content mark by key. -// |buffer| is only modified if |buflen| is longer than the length of the value. // // mark - handle to a content mark. // key - string key of the property. -// buffer - buffer for holding the returned value in UTF16-LE. +// buffer - buffer for holding the returned value in UTF16-LE. This is +// only modified if |buflen| is longer than the length of the +// value. +// Optional, pass null to just retrieve the size of the buffer +// needed. // buflen - length of the buffer. -// out_buflen - pointer to variable that will receive the length of the value. -// Not filled if false is returned. +// out_buflen - pointer to variable that will receive the minimum buffer size +// to contain the value. Not filled if FALSE is returned. // // Returns TRUE if the key maps to a string/blob value, FALSE otherwise. FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV @@ -419,15 +432,16 @@ FPDFPageObjMark_GetParamStringValue(FPDF_PAGEOBJECTMARK mark, // Experimental API. // Get the value of a blob property in a content mark by key. -// |buffer| is only modified if |buflen| is longer than or equal to the length -// of the value. // // mark - handle to a content mark. // key - string key of the property. -// buffer - buffer for holding the returned value. +// buffer - buffer for holding the returned value. This is only modified +// if |buflen| is at least as long as the length of the value. +// Optional, pass null to just retrieve the size of the buffer +// needed. // buflen - length of the buffer. -// out_buflen - pointer to variable that will receive the length of the value. -// Not filled if false is returned. +// out_buflen - pointer to variable that will receive the minimum buffer size +// to contain the value. Not filled if FALSE is returned. // // Returns TRUE if the key maps to a string/blob value, FALSE otherwise. FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV |