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_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()));
}