Avoid using void* in experimental APIs in fpdf_edit.h Instead of taking buffers of type void*, specify the proper type. e.g. FPDF_WCHAR*, or const unsigned char*. Then adjust tests to match and avoid having to do casts. Also: - Improve documentation in fpdf_edit.h. - Remove an UNSAFE_TODO() along the way. Change-Id: Ifd1efafd2944385497ac87379914b419bd271766 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/124950 Reviewed-by: Tom Sepez <tsepez@chromium.org> Reviewed-by: Tom Sepez <tsepez@google.com> Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/fpdfsdk/fpdf_edit_embeddertest.cpp b/fpdfsdk/fpdf_edit_embeddertest.cpp index 57680a1..8d3512d 100644 --- a/fpdfsdk/fpdf_edit_embeddertest.cpp +++ b/fpdfsdk/fpdf_edit_embeddertest.cpp
@@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include <stdint.h> + #include <limits> #include <memory> #include <ostream> @@ -23,6 +25,7 @@ #include "core/fxcrt/fx_codepage.h" #include "core/fxcrt/fx_memcpy_wrappers.h" #include "core/fxcrt/fx_system.h" +#include "core/fxcrt/span_util.h" #include "core/fxge/cfx_defaultrenderdevice.h" #include "core/fxge/fx_font.h" #include "fpdfsdk/cpdfsdk_helpers.h" @@ -1318,14 +1321,13 @@ for (int j = 0; j < mark_count; ++j) { FPDF_PAGEOBJECTMARK mark = FPDFPageObj_GetMark(page_object, j); - char buffer[256]; + FPDF_WCHAR buffer[128]; 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)); + std::wstring name = GetPlatformWString(buffer); if (name == L"Prime") { prime_count++; } else if (name == L"Square") { @@ -1337,8 +1339,7 @@ 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)); + std::wstring key = GetPlatformWString(buffer); EXPECT_EQ(L"Factor", key); EXPECT_EQ(FPDF_OBJECT_NUMBER, @@ -1357,8 +1358,7 @@ 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)); + std::wstring key = GetPlatformWString(buffer); EXPECT_EQ(L"Position", key); EXPECT_EQ(FPDF_OBJECT_STRING, @@ -1367,8 +1367,7 @@ EXPECT_TRUE(FPDFPageObjMark_GetParamStringValue( mark, "Position", buffer, sizeof(buffer), &length)); ASSERT_GT(length, 0u); - std::wstring value = - GetPlatformWString(reinterpret_cast<unsigned short*>(buffer)); + std::wstring value = GetPlatformWString(buffer); // "Position" can be "First", "Last", or "End". if (i == 0) { @@ -1470,14 +1469,13 @@ for (int j = 0; j < mark_count; ++j) { FPDF_PAGEOBJECTMARK mark = FPDFPageObj_GetMark(page_object, j); - char buffer[256]; + FPDF_WCHAR buffer[128]; 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)); + std::wstring name = GetPlatformWString(buffer); if (name == L"Prime") { primes.push_back(page_object); } @@ -1572,14 +1570,13 @@ for (int j = mark_count - 1; j >= 0; --j) { FPDF_PAGEOBJECTMARK mark = FPDFPageObj_GetMark(page_object, j); - char buffer[256]; + FPDF_WCHAR buffer[128]; 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)); + std::wstring name = GetPlatformWString(buffer); if (name == L"Prime") { // Remove mark. EXPECT_TRUE(FPDFPageObj_RemoveMark(page_object, mark)); @@ -1625,14 +1622,13 @@ for (int j = 0; j < mark_count; ++j) { FPDF_PAGEOBJECTMARK mark = FPDFPageObj_GetMark(page_object, j); - char buffer[256]; + FPDF_WCHAR buffer[128]; 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)); + std::wstring name = GetPlatformWString(buffer); if (name == L"Square") { // Show the mark has a "Factor" parameter. int out_value; @@ -1666,14 +1662,13 @@ for (int j = 0; j < mark_count; ++j) { FPDF_PAGEOBJECTMARK mark = FPDFPageObj_GetMark(page_object, j); - char buffer[256]; + FPDF_WCHAR buffer[128]; 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)); + std::wstring name = GetPlatformWString(buffer); if (name == L"Square") { // Verify the "Factor" parameter is still gone. int out_value; @@ -3957,17 +3952,15 @@ FPDF_PAGEOBJECT page_object = FPDFPage_GetObject(page.get(), 18); FPDF_PAGEOBJECTMARK mark = FPDFPageObj_GetMark(page_object, 1); ASSERT_TRUE(mark); - char buffer[256]; + FPDF_WCHAR buffer[128]; 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))); + ASSERT_EQ(L"Bounds", GetPlatformWString(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<unsigned short*>(buffer))); + ASSERT_EQ(L"Last", GetPlatformWString(buffer)); // Set is to "End". EXPECT_TRUE(FPDFPageObjMark_SetStringParam(document(), page_object, mark, @@ -3984,8 +3977,7 @@ // 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<unsigned short*>(buffer))); + EXPECT_EQ(L"End", GetPlatformWString(buffer)); // Save the file EXPECT_TRUE(FPDFPage_GenerateContent(page.get())); @@ -4001,8 +3993,7 @@ 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<unsigned short*>(buffer))); + EXPECT_EQ(L"End", GetPlatformWString(buffer)); CloseSavedPage(saved_page); CloseSavedDocument(); @@ -4035,29 +4026,27 @@ EXPECT_TRUE(mark); EXPECT_EQ(1, FPDFPageObj_CountMarks(text_object)); EXPECT_EQ(mark, FPDFPageObj_GetMark(text_object, 0)); - char buffer[256]; + FPDF_WCHAR buffer[128]; 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)); + std::wstring name = GetPlatformWString(buffer); EXPECT_EQ(L"Test Mark Name", name); // Add parameters: // - int "IntKey" : 42 // - string "StringKey": "StringValue" // - blob "BlobKey": "\x01\x02\x03\0BlobValue1\0\0\0BlobValue2\0" - constexpr size_t kBlobLen = 28; - char block_value[kBlobLen]; - UNSAFE_TODO(FXSYS_memcpy( - block_value, "\x01\x02\x03\0BlobValue1\0\0\0BlobValue2\0", kBlobLen)); + // + // Note that the trailing NUL is in `kBlobData` implicitly. + constexpr uint8_t kBlobData[] = "\x01\x02\x03\0BlobValue1\0\0\0BlobValue2"; EXPECT_EQ(0, FPDFPageObjMark_CountParams(mark)); 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", block_value, kBlobLen)); + EXPECT_TRUE(FPDFPageObjMark_SetBlobParam( + document(), text_object, mark, "BlobKey", kBlobData, sizeof(kBlobData))); EXPECT_EQ(3, FPDFPageObjMark_CountParams(mark)); // Check the two parameters can be retrieved. @@ -4074,16 +4063,19 @@ 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)); + name = GetPlatformWString(buffer); EXPECT_EQ(L"StringValue", name); EXPECT_EQ(FPDF_OBJECT_STRING, FPDFPageObjMark_GetParamValueType(mark, "BlobKey")); out_buffer_len = 0; + constexpr size_t kBlobLen = 28; + unsigned char blob_buffer[kBlobLen]; EXPECT_TRUE(FPDFPageObjMark_GetParamBlobValue( - mark, "BlobKey", buffer, sizeof(buffer), &out_buffer_len)); + mark, "BlobKey", blob_buffer, sizeof(blob_buffer), &out_buffer_len)); EXPECT_EQ(kBlobLen, out_buffer_len); - EXPECT_EQ(0, memcmp(block_value, buffer, kBlobLen)); + EXPECT_TRUE(fxcrt::span_equals(pdfium::make_span(kBlobData), + pdfium::make_span(blob_buffer))); // Render and check the bitmap is the expected one. { @@ -4113,7 +4105,7 @@ 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)); + name = GetPlatformWString(buffer); EXPECT_EQ(L"Test Mark Name", name); CloseSavedPage(saved_page); @@ -4128,7 +4120,7 @@ FPDF_PAGEOBJECTMARK mark = FPDFPageObj_GetMark(page_object, 1); ASSERT_TRUE(mark); - char buffer[256]; + FPDF_WCHAR buffer[128]; unsigned long out_len; // Show the positive cases of FPDFPageObjMark_GetName. @@ -4138,8 +4130,7 @@ 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(L"Bounds", GetPlatformWString(buffer)); EXPECT_EQ((6u + 1u) * 2u, out_len); // Show the negative cases of FPDFPageObjMark_GetName. @@ -4159,7 +4150,7 @@ FPDF_PAGEOBJECTMARK mark = FPDFPageObj_GetMark(page_object, 1); ASSERT_TRUE(mark); - char buffer[256]; + FPDF_WCHAR buffer[128]; unsigned long out_len; // Show the positive cases of FPDFPageObjMark_GetParamKey. @@ -4170,8 +4161,7 @@ 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(L"Position", GetPlatformWString(buffer)); EXPECT_EQ((8u + 1u) * 2u, out_len); // Show the negative cases of FPDFPageObjMark_GetParamKey. @@ -4231,7 +4221,7 @@ FPDF_PAGEOBJECTMARK mark = FPDFPageObj_GetMark(page_object, 1); ASSERT_TRUE(mark); - char buffer[256]; + FPDF_WCHAR buffer[128]; unsigned long out_len; // Show the positive cases of FPDFPageObjMark_GetParamStringValue. @@ -4243,8 +4233,7 @@ 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(L"Last", GetPlatformWString(buffer)); EXPECT_EQ((4u + 1u) * 2u, out_len); // Show the negative cases of FPDFPageObjMark_GetParamStringValue.
diff --git a/fpdfsdk/fpdf_editpage.cpp b/fpdfsdk/fpdf_editpage.cpp index 0309278..d41b12a 100644 --- a/fpdfsdk/fpdf_editpage.cpp +++ b/fpdfsdk/fpdf_editpage.cpp
@@ -369,7 +369,7 @@ FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV FPDFPageObjMark_GetName(FPDF_PAGEOBJECTMARK mark, - void* buffer, + FPDF_WCHAR* buffer, unsigned long buflen, unsigned long* out_buflen) { const CPDF_ContentMarkItem* pMarkItem = @@ -398,7 +398,7 @@ FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV FPDFPageObjMark_GetParamKey(FPDF_PAGEOBJECTMARK mark, unsigned long index, - void* buffer, + FPDF_WCHAR* buffer, unsigned long buflen, unsigned long* out_buflen) { if (!out_buflen) @@ -456,7 +456,7 @@ FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV FPDFPageObjMark_GetParamStringValue(FPDF_PAGEOBJECTMARK mark, FPDF_BYTESTRING key, - void* buffer, + FPDF_WCHAR* buffer, unsigned long buflen, unsigned long* out_buflen) { if (!out_buflen) @@ -480,7 +480,7 @@ FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV FPDFPageObjMark_GetParamBlobValue(FPDF_PAGEOBJECTMARK mark, FPDF_BYTESTRING key, - void* buffer, + unsigned char* buffer, unsigned long buflen, unsigned long* out_buflen) { if (!out_buflen) @@ -579,7 +579,7 @@ FPDF_PAGEOBJECT page_object, FPDF_PAGEOBJECTMARK mark, FPDF_BYTESTRING key, - void* value, + const unsigned char* value, unsigned long value_len) { if (!value && value_len > 0) { return false; @@ -598,9 +598,7 @@ // SAFETY: required from caller. pParams->SetNewFor<CPDF_String>( - key, - UNSAFE_BUFFERS( - pdfium::make_span(static_cast<const uint8_t*>(value), value_len)), + key, UNSAFE_BUFFERS(pdfium::make_span(value, value_len)), CPDF_String::DataType::kIsHex); pPageObj->SetDirty(true); return true;
diff --git a/public/fpdf_edit.h b/public/fpdf_edit.h index f4e08c7..6ab72cb 100644 --- a/public/fpdf_edit.h +++ b/public/fpdf_edit.h
@@ -424,17 +424,18 @@ // // mark - handle to a content mark. // buffer - buffer for holding the returned name in UTF-16LE. This is only -// modified if |buflen| is longer than the length of the name. +// modified if |buflen| is large enough to store the name. // Optional, pass null to just retrieve the size of the buffer // needed. -// buflen - length of the buffer. +// buflen - length of the buffer in bytes. // out_buflen - pointer to variable that will receive the minimum buffer size -// to contain the name. Not filled if FALSE is returned. +// in bytes to contain the name. This is a required parameter. +// Not filled if FALSE is returned. // // Returns TRUE if the operation succeeded, FALSE if it failed. FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV FPDFPageObjMark_GetName(FPDF_PAGEOBJECTMARK mark, - void* buffer, + FPDF_WCHAR* buffer, unsigned long buflen, unsigned long* out_buflen); @@ -454,18 +455,19 @@ // mark - handle to a content mark. // index - index of the property. // buffer - buffer for holding the returned key in UTF-16LE. This is only -// modified if |buflen| is longer than the length of the key. +// modified if |buflen| is large enough to store the key. // Optional, pass null to just retrieve the size of the buffer // needed. -// buflen - length of the buffer. +// buflen - length of the buffer in bytes. // out_buflen - pointer to variable that will receive the minimum buffer size -// to contain the key. Not filled if FALSE is returned. +// in bytes to contain the name. This is a required parameter. +// Not filled if FALSE is returned. // // 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, + FPDF_WCHAR* buffer, unsigned long buflen, unsigned long* out_buflen); @@ -502,19 +504,19 @@ // mark - handle to a content mark. // key - string key of the property. // buffer - buffer for holding the returned value in UTF-16LE. This is -// only modified if |buflen| is longer than the length of the -// value. +// only modified if |buflen| is large enough to store the value. // Optional, pass null to just retrieve the size of the buffer // needed. -// buflen - length of the buffer. +// buflen - length of the buffer in bytes. // out_buflen - pointer to variable that will receive the minimum buffer size -// to contain the value. Not filled if FALSE is returned. +// in bytes to contain the name. This is a required parameter. +// 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 FPDFPageObjMark_GetParamStringValue(FPDF_PAGEOBJECTMARK mark, FPDF_BYTESTRING key, - void* buffer, + FPDF_WCHAR* buffer, unsigned long buflen, unsigned long* out_buflen); @@ -524,18 +526,19 @@ // mark - handle to a content mark. // key - string key of the property. // buffer - buffer for holding the returned value. This is only modified -// if |buflen| is at least as long as the length of the value. +// if |buflen| is large enough to store the value. // Optional, pass null to just retrieve the size of the buffer // needed. -// buflen - length of the buffer. +// buflen - length of the buffer in bytes. // out_buflen - pointer to variable that will receive the minimum buffer size -// to contain the value. Not filled if FALSE is returned. +// in bytes to contain the name. This is a required parameter. +// 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 FPDFPageObjMark_GetParamBlobValue(FPDF_PAGEOBJECTMARK mark, FPDF_BYTESTRING key, - void* buffer, + unsigned char* buffer, unsigned long buflen, unsigned long* out_buflen); @@ -595,7 +598,7 @@ FPDF_PAGEOBJECT page_object, FPDF_PAGEOBJECTMARK mark, FPDF_BYTESTRING key, - void* value, + const unsigned char* value, unsigned long value_len); // Experimental API.