Give buffers in experimental fpdf_attachment.h APIs clearer types.
Change the buffer parameters' types from void* to FPDF_WCHAR*. Then:
- Clarify in the documentation that the buffer length is in bytes.
- Change callers to use containers of FPDF_WCHAR instead of char.
- Change callers to avoid casting.
- Use GetFPDFWideStringBuffer() to help create std::vector<FPDF_WCHAR>
without having to byte length to FPDF_WCHAR count conversions.
The affected APIs are:
- FPDFAttachment_GetName()
- FPDFAttachment_GetStringValue()
Change-Id: Ia077bcb7b04e49feeae1a2c04ed8fb58c8f9e2d0
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/52890
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
diff --git a/fpdfsdk/fpdf_attachment.cpp b/fpdfsdk/fpdf_attachment.cpp
index 259ed65..937b504 100644
--- a/fpdfsdk/fpdf_attachment.cpp
+++ b/fpdfsdk/fpdf_attachment.cpp
@@ -131,7 +131,7 @@
FPDF_EXPORT unsigned long FPDF_CALLCONV
FPDFAttachment_GetName(FPDF_ATTACHMENT attachment,
- void* buffer,
+ FPDF_WCHAR* buffer,
unsigned long buflen) {
CPDF_Object* pFile = CPDFObjectFromFPDFAttachment(attachment);
if (!pFile)
@@ -186,7 +186,7 @@
FPDF_EXPORT unsigned long FPDF_CALLCONV
FPDFAttachment_GetStringValue(FPDF_ATTACHMENT attachment,
FPDF_BYTESTRING key,
- void* buffer,
+ FPDF_WCHAR* buffer,
unsigned long buflen) {
CPDF_Object* pFile = CPDFObjectFromFPDFAttachment(attachment);
if (!pFile)
diff --git a/fpdfsdk/fpdf_attachment_embeddertest.cpp b/fpdfsdk/fpdf_attachment_embeddertest.cpp
index 59508ee..7ce6003 100644
--- a/fpdfsdk/fpdf_attachment_embeddertest.cpp
+++ b/fpdfsdk/fpdf_attachment_embeddertest.cpp
@@ -27,19 +27,18 @@
ASSERT_TRUE(attachment);
// Check that the name of the first attachment is correct.
- unsigned long len = FPDFAttachment_GetName(attachment, nullptr, 0);
- std::vector<char> buf(len);
- EXPECT_EQ(12u, FPDFAttachment_GetName(attachment, buf.data(), len));
- EXPECT_STREQ(L"1.txt",
- GetPlatformWString(reinterpret_cast<unsigned short*>(buf.data()))
- .c_str());
+ unsigned long length_bytes = FPDFAttachment_GetName(attachment, nullptr, 0);
+ ASSERT_EQ(12u, length_bytes);
+ std::vector<FPDF_WCHAR> buf = GetFPDFWideStringBuffer(length_bytes);
+ EXPECT_EQ(12u, FPDFAttachment_GetName(attachment, buf.data(), length_bytes));
+ EXPECT_EQ(L"1.txt", GetPlatformWString(buf.data()));
// Check that the content of the first attachment is correct.
- len = FPDFAttachment_GetFile(attachment, nullptr, 0);
- buf.clear();
- buf.resize(len);
- ASSERT_EQ(4u, FPDFAttachment_GetFile(attachment, buf.data(), len));
- EXPECT_EQ(std::string("test"), std::string(buf.data(), 4));
+ length_bytes = FPDFAttachment_GetFile(attachment, nullptr, 0);
+ std::vector<char> content_buf(length_bytes);
+ ASSERT_EQ(
+ 4u, FPDFAttachment_GetFile(attachment, content_buf.data(), length_bytes));
+ EXPECT_EQ(std::string("test"), std::string(content_buf.data(), 4));
// Check that a non-existent key does not exist.
EXPECT_FALSE(FPDFAttachment_HasKey(attachment, "none"));
@@ -52,40 +51,40 @@
FPDFAttachment_GetStringValue(attachment, kSizeKey, nullptr, 0));
// Check that the creation date of the first attachment is correct.
- len = FPDFAttachment_GetStringValue(attachment, kDateKey, nullptr, 0);
- buf.clear();
- buf.resize(len);
+ length_bytes =
+ FPDFAttachment_GetStringValue(attachment, kDateKey, nullptr, 0);
+ ASSERT_EQ(48u, length_bytes);
+ buf = GetFPDFWideStringBuffer(length_bytes);
EXPECT_EQ(48u, FPDFAttachment_GetStringValue(attachment, kDateKey, buf.data(),
- len));
- EXPECT_STREQ(L"D:20170712214438-07'00'",
- GetPlatformWString(reinterpret_cast<unsigned short*>(buf.data()))
- .c_str());
+ length_bytes));
+ EXPECT_EQ(L"D:20170712214438-07'00'", GetPlatformWString(buf.data()));
// Retrieve the second attachment.
attachment = FPDFDoc_GetAttachment(document(), 1);
ASSERT_TRUE(attachment);
// Retrieve the second attachment file.
- len = FPDFAttachment_GetFile(attachment, nullptr, 0);
- buf.clear();
- buf.resize(len);
- EXPECT_EQ(5869u, FPDFAttachment_GetFile(attachment, buf.data(), len));
+ length_bytes = FPDFAttachment_GetFile(attachment, nullptr, 0);
+ content_buf.clear();
+ content_buf.resize(length_bytes);
+ ASSERT_EQ(5869u, FPDFAttachment_GetFile(attachment, content_buf.data(),
+ length_bytes));
// Check that the calculated checksum of the file data matches expectation.
const char kCheckSum[] = "72afcddedf554dda63c0c88e06f1ce18";
const wchar_t kCheckSumW[] = L"<72AFCDDEDF554DDA63C0C88E06F1CE18>";
- const std::string generated_checksum =
- GenerateMD5Base16(reinterpret_cast<uint8_t*>(buf.data()), len);
+ const std::string generated_checksum = GenerateMD5Base16(
+ reinterpret_cast<uint8_t*>(content_buf.data()), length_bytes);
EXPECT_EQ(kCheckSum, generated_checksum);
// Check that the stored checksum matches expectation.
- len = FPDFAttachment_GetStringValue(attachment, kChecksumKey, nullptr, 0);
- buf.clear();
- buf.resize(len);
+ length_bytes =
+ FPDFAttachment_GetStringValue(attachment, kChecksumKey, nullptr, 0);
+ ASSERT_EQ(70u, length_bytes);
+ buf = GetFPDFWideStringBuffer(length_bytes);
EXPECT_EQ(70u, FPDFAttachment_GetStringValue(attachment, kChecksumKey,
- buf.data(), len));
- EXPECT_EQ(kCheckSumW,
- GetPlatformWString(reinterpret_cast<unsigned short*>(buf.data())));
+ buf.data(), length_bytes));
+ EXPECT_EQ(kCheckSumW, GetPlatformWString(buf.data()));
}
TEST_F(FPDFAttachmentEmbedderTest, AddAttachments) {
@@ -112,19 +111,18 @@
// Verify the name of the new attachment (i.e. the first attachment).
attachment = FPDFDoc_GetAttachment(document(), 0);
ASSERT_TRUE(attachment);
- unsigned long len = FPDFAttachment_GetName(attachment, nullptr, 0);
- std::vector<char> buf(len);
- EXPECT_EQ(12u, FPDFAttachment_GetName(attachment, buf.data(), len));
- EXPECT_STREQ(L"0.txt",
- GetPlatformWString(reinterpret_cast<unsigned short*>(buf.data()))
- .c_str());
+ unsigned long length_bytes = FPDFAttachment_GetName(attachment, nullptr, 0);
+ ASSERT_EQ(12u, length_bytes);
+ std::vector<FPDF_WCHAR> buf = GetFPDFWideStringBuffer(length_bytes);
+ EXPECT_EQ(12u, FPDFAttachment_GetName(attachment, buf.data(), length_bytes));
+ EXPECT_EQ(L"0.txt", GetPlatformWString(buf.data()));
// Verify the content of the new attachment (i.e. the first attachment).
- len = FPDFAttachment_GetFile(attachment, nullptr, 0);
- buf.clear();
- buf.resize(len);
- ASSERT_EQ(6u, FPDFAttachment_GetFile(attachment, buf.data(), len));
- EXPECT_EQ(std::string(kContents1), std::string(buf.data(), 6));
+ length_bytes = FPDFAttachment_GetFile(attachment, nullptr, 0);
+ std::vector<char> content_buf(length_bytes);
+ ASSERT_EQ(
+ 6u, FPDFAttachment_GetFile(attachment, content_buf.data(), length_bytes));
+ EXPECT_EQ(std::string(kContents1), std::string(content_buf.data(), 6));
// Add an attachment to the end of the embedded file list and set its file.
file_name = GetFPDFWideString(L"z.txt");
@@ -137,20 +135,19 @@
// Verify the name of the new attachment (i.e. the fourth attachment).
attachment = FPDFDoc_GetAttachment(document(), 3);
ASSERT_TRUE(attachment);
- len = FPDFAttachment_GetName(attachment, nullptr, 0);
- buf.clear();
- buf.resize(len);
- EXPECT_EQ(12u, FPDFAttachment_GetName(attachment, buf.data(), len));
- EXPECT_STREQ(L"z.txt",
- GetPlatformWString(reinterpret_cast<unsigned short*>(buf.data()))
- .c_str());
+ length_bytes = FPDFAttachment_GetName(attachment, nullptr, 0);
+ ASSERT_EQ(12u, length_bytes);
+ buf = GetFPDFWideStringBuffer(length_bytes);
+ EXPECT_EQ(12u, FPDFAttachment_GetName(attachment, buf.data(), length_bytes));
+ EXPECT_EQ(L"z.txt", GetPlatformWString(buf.data()));
// Verify the content of the new attachment (i.e. the fourth attachment).
- len = FPDFAttachment_GetFile(attachment, nullptr, 0);
- buf.clear();
- buf.resize(len);
- ASSERT_EQ(6u, FPDFAttachment_GetFile(attachment, buf.data(), len));
- EXPECT_EQ(std::string(kContents2), std::string(buf.data(), 6));
+ length_bytes = FPDFAttachment_GetFile(attachment, nullptr, 0);
+ content_buf.clear();
+ content_buf.resize(length_bytes);
+ ASSERT_EQ(
+ 6u, FPDFAttachment_GetFile(attachment, content_buf.data(), length_bytes));
+ EXPECT_EQ(std::string(kContents2), std::string(content_buf.data(), 6));
}
TEST_F(FPDFAttachmentEmbedderTest, AddAttachmentsWithParams) {
@@ -181,51 +178,49 @@
// Verify the name of the new attachment (i.e. the second attachment).
attachment = FPDFDoc_GetAttachment(document(), 1);
ASSERT_TRUE(attachment);
- unsigned long len = FPDFAttachment_GetName(attachment, nullptr, 0);
- std::vector<char> buf(len);
- EXPECT_EQ(12u, FPDFAttachment_GetName(attachment, buf.data(), len));
- EXPECT_STREQ(L"5.txt",
- GetPlatformWString(reinterpret_cast<unsigned short*>(buf.data()))
- .c_str());
+ unsigned long length_bytes = FPDFAttachment_GetName(attachment, nullptr, 0);
+ ASSERT_EQ(12u, length_bytes);
+ std::vector<FPDF_WCHAR> buf = GetFPDFWideStringBuffer(length_bytes);
+ EXPECT_EQ(12u, FPDFAttachment_GetName(attachment, buf.data(), length_bytes));
+ EXPECT_EQ(L"5.txt", GetPlatformWString(buf.data()));
// Verify the content of the new attachment.
- len = FPDFAttachment_GetFile(attachment, nullptr, 0);
- buf.clear();
- buf.resize(len);
- ASSERT_EQ(12u, FPDFAttachment_GetFile(attachment, buf.data(), len));
- EXPECT_EQ(std::string(kContents), std::string(buf.data(), 12));
+ length_bytes = FPDFAttachment_GetFile(attachment, nullptr, 0);
+ std::vector<char> content_buf(length_bytes);
+ ASSERT_EQ(12u, FPDFAttachment_GetFile(attachment, content_buf.data(),
+ length_bytes));
+ EXPECT_EQ(std::string(kContents), std::string(content_buf.data(), 12));
// Verify the creation date of the new attachment.
- len = FPDFAttachment_GetStringValue(attachment, kDateKey, nullptr, 0);
- buf.clear();
- buf.resize(len);
+ length_bytes =
+ FPDFAttachment_GetStringValue(attachment, kDateKey, nullptr, 0);
+ ASSERT_EQ(48u, length_bytes);
+ buf = GetFPDFWideStringBuffer(length_bytes);
EXPECT_EQ(48u, FPDFAttachment_GetStringValue(attachment, kDateKey, buf.data(),
- len));
- EXPECT_STREQ(kDateW,
- GetPlatformWString(reinterpret_cast<unsigned short*>(buf.data()))
- .c_str());
+ length_bytes));
+ EXPECT_EQ(kDateW, GetPlatformWString(buf.data()));
// Verify the checksum of the new attachment.
- len = FPDFAttachment_GetStringValue(attachment, kChecksumKey, nullptr, 0);
- buf.clear();
- buf.resize(len);
+ length_bytes =
+ FPDFAttachment_GetStringValue(attachment, kChecksumKey, nullptr, 0);
+ ASSERT_EQ(70u, length_bytes);
+ buf = GetFPDFWideStringBuffer(length_bytes);
EXPECT_EQ(70u, FPDFAttachment_GetStringValue(attachment, kChecksumKey,
- buf.data(), len));
- EXPECT_STREQ(kCheckSumW,
- GetPlatformWString(reinterpret_cast<unsigned short*>(buf.data()))
- .c_str());
+ buf.data(), length_bytes));
+ EXPECT_EQ(kCheckSumW, GetPlatformWString(buf.data()));
// Overwrite the existing file with empty content, and check that the checksum
// gets updated to the correct value.
EXPECT_TRUE(FPDFAttachment_SetFile(attachment, document(), nullptr, 0));
EXPECT_EQ(0u, FPDFAttachment_GetFile(attachment, nullptr, 0));
- len = FPDFAttachment_GetStringValue(attachment, kChecksumKey, nullptr, 0);
- buf.clear();
- buf.resize(len);
+ length_bytes =
+ FPDFAttachment_GetStringValue(attachment, kChecksumKey, nullptr, 0);
+ ASSERT_EQ(70u, length_bytes);
+ buf = GetFPDFWideStringBuffer(length_bytes);
EXPECT_EQ(70u, FPDFAttachment_GetStringValue(attachment, kChecksumKey,
- buf.data(), len));
+ buf.data(), length_bytes));
EXPECT_EQ(L"<D41D8CD98F00B204E9800998ECF8427E>",
- GetPlatformWString(reinterpret_cast<unsigned short*>(buf.data())));
+ GetPlatformWString(buf.data()));
}
TEST_F(FPDFAttachmentEmbedderTest, DeleteAttachment) {
@@ -235,12 +230,11 @@
// Verify the name of the first attachment.
FPDF_ATTACHMENT attachment = FPDFDoc_GetAttachment(document(), 0);
- unsigned long len = FPDFAttachment_GetName(attachment, nullptr, 0);
- std::vector<char> buf(len);
- EXPECT_EQ(12u, FPDFAttachment_GetName(attachment, buf.data(), len));
- EXPECT_STREQ(L"1.txt",
- GetPlatformWString(reinterpret_cast<unsigned short*>(buf.data()))
- .c_str());
+ unsigned long length_bytes = FPDFAttachment_GetName(attachment, nullptr, 0);
+ ASSERT_EQ(12u, length_bytes);
+ std::vector<FPDF_WCHAR> buf = GetFPDFWideStringBuffer(length_bytes);
+ EXPECT_EQ(12u, FPDFAttachment_GetName(attachment, buf.data(), length_bytes));
+ EXPECT_EQ(L"1.txt", GetPlatformWString(buf.data()));
// Delete the first attachment.
EXPECT_TRUE(FPDFDoc_DeleteAttachment(document(), 0));
@@ -248,11 +242,9 @@
// Verify the name of the new first attachment.
attachment = FPDFDoc_GetAttachment(document(), 0);
- len = FPDFAttachment_GetName(attachment, nullptr, 0);
- buf.clear();
- buf.resize(len);
- EXPECT_EQ(26u, FPDFAttachment_GetName(attachment, buf.data(), len));
- EXPECT_STREQ(L"attached.pdf",
- GetPlatformWString(reinterpret_cast<unsigned short*>(buf.data()))
- .c_str());
+ length_bytes = FPDFAttachment_GetName(attachment, nullptr, 0);
+ ASSERT_EQ(26u, length_bytes);
+ buf = GetFPDFWideStringBuffer(length_bytes);
+ EXPECT_EQ(26u, FPDFAttachment_GetName(attachment, buf.data(), length_bytes));
+ EXPECT_EQ(L"attached.pdf", GetPlatformWString(buf.data()));
}
diff --git a/public/fpdf_attachment.h b/public/fpdf_attachment.h
index 7b55974..1c424f6 100644
--- a/public/fpdf_attachment.h
+++ b/public/fpdf_attachment.h
@@ -65,12 +65,12 @@
//
// attachment - handle to an attachment.
// buffer - buffer for holding the file name, encoded in UTF16-LE.
-// buflen - length of the buffer.
+// buflen - length of the buffer in bytes.
//
-// Returns the length of the file name.
+// Returns the length of the file name in bytes.
FPDF_EXPORT unsigned long FPDF_CALLCONV
FPDFAttachment_GetName(FPDF_ATTACHMENT attachment,
- void* buffer,
+ FPDF_WCHAR* buffer,
unsigned long buflen);
// Experimental API.
@@ -122,13 +122,13 @@
// attachment - handle to an attachment.
// key - the key to the requested string value, encoded in UTF-8.
// buffer - buffer for holding the string value encoded in UTF16-LE.
-// buflen - length of the buffer.
+// buflen - length of the buffer in bytes.
//
-// Returns the length of the dictionary value string.
+// Returns the length of the dictionary value string in bytes.
FPDF_EXPORT unsigned long FPDF_CALLCONV
FPDFAttachment_GetStringValue(FPDF_ATTACHMENT attachment,
FPDF_BYTESTRING key,
- void* buffer,
+ FPDF_WCHAR* buffer,
unsigned long buflen);
// Experimental API.
diff --git a/samples/pdfium_test_write_helper.cc b/samples/pdfium_test_write_helper.cc
index 5b3fe2d..23164a8 100644
--- a/samples/pdfium_test_write_helper.cc
+++ b/samples/pdfium_test_write_helper.cc
@@ -511,15 +511,13 @@
// Retrieve the attachment file name.
std::string attachment_name;
- unsigned long len = FPDFAttachment_GetName(attachment, nullptr, 0);
- if (len) {
- std::vector<char> buf(len);
- unsigned long actual_len =
- FPDFAttachment_GetName(attachment, buf.data(), len);
- if (actual_len == len) {
- attachment_name =
- GetPlatformString(reinterpret_cast<unsigned short*>(buf.data()));
- }
+ unsigned long length_bytes = FPDFAttachment_GetName(attachment, nullptr, 0);
+ if (length_bytes) {
+ std::vector<FPDF_WCHAR> buf = GetFPDFWideStringBuffer(length_bytes);
+ unsigned long actual_length_bytes =
+ FPDFAttachment_GetName(attachment, buf.data(), length_bytes);
+ if (actual_length_bytes == length_bytes)
+ attachment_name = GetPlatformString(buf.data());
}
if (attachment_name.empty()) {
fprintf(stderr, "Attachment #%d has an empty file name.\n", i + 1);
@@ -538,12 +536,12 @@
}
// Retrieve the attachment.
- len = FPDFAttachment_GetFile(attachment, nullptr, 0);
- std::vector<char> data_buf(len);
- if (len) {
- unsigned long actual_len =
- FPDFAttachment_GetFile(attachment, data_buf.data(), len);
- if (actual_len != len)
+ length_bytes = FPDFAttachment_GetFile(attachment, nullptr, 0);
+ std::vector<char> data_buf(length_bytes);
+ if (length_bytes) {
+ unsigned long actual_length_bytes =
+ FPDFAttachment_GetFile(attachment, data_buf.data(), length_bytes);
+ if (actual_length_bytes != length_bytes)
data_buf.clear();
}
if (data_buf.empty()) {
@@ -558,8 +556,8 @@
continue;
}
- size_t written_len = fwrite(data_buf.data(), 1, len, fp);
- if (written_len == len) {
+ size_t written_len = fwrite(data_buf.data(), 1, length_bytes, fp);
+ if (written_len == length_bytes) {
fprintf(stderr, "Saved attachment \"%s\" as: %s.\n",
attachment_name.c_str(), save_name);
} else {
