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 <hnakashima@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/fpdfsdk/fpdf_edit_embeddertest.cpp b/fpdfsdk/fpdf_edit_embeddertest.cpp
index 3bacb0b..4250091 100644
--- a/fpdfsdk/fpdf_edit_embeddertest.cpp
+++ b/fpdfsdk/fpdf_edit_embeddertest.cpp
@@ -541,7 +541,11 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 3004934..c71f7e2 100644
--- a/fpdfsdk/fpdf_editpage.cpp
+++ b/fpdfsdk/fpdf_editpage.cpp
@@ -345,19 +345,21 @@
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 @@
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 @@
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 dcf6201..180d502 100644
--- a/public/fpdf_edit.h
+++ b/public/fpdf_edit.h
@@ -332,18 +332,23 @@
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 @@
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 @@
// 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 @@
// 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
