Change experimental API FPDFAttachment_GetFile().
FPDFAttachment_GetFile() returns 0 when retrieving attachment data
fails or when the attachment size is 0-byte. Since PDF documents could
have 0-byte attachments embedded, we need this API to distinguish the
two situations.
This CL changes the return value for FPDFAttachment_GetFile() to
FPDF_BOOL type to indicate success/failure on retrieving attachment
file data, and adds an output parameter |out_buflen| to return the
attachment's size in bytes.
Bug: pdfium:1537
Change-Id: I390b33c9bc9a5ee223db9e101f2817b468b1b040
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/70474
Commit-Queue: Hui Yingst <nigi@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/fpdfsdk/fpdf_attachment.cpp b/fpdfsdk/fpdf_attachment.cpp
index da5e6e3..f4e2963 100644
--- a/fpdfsdk/fpdf_attachment.cpp
+++ b/fpdfsdk/fpdf_attachment.cpp
@@ -247,17 +247,23 @@
return true;
}
-FPDF_EXPORT unsigned long FPDF_CALLCONV
+FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV
FPDFAttachment_GetFile(FPDF_ATTACHMENT attachment,
void* buffer,
- unsigned long buflen) {
+ unsigned long buflen,
+ unsigned long* out_buflen) {
+ if (!out_buflen)
+ return false;
+
CPDF_Object* pFile = CPDFObjectFromFPDFAttachment(attachment);
if (!pFile)
- return 0;
+ return false;
CPDF_Stream* pFileStream = CPDF_FileSpec(pFile).GetFileStream();
if (!pFileStream)
- return 0;
+ return false;
- return DecodeStreamMaybeCopyAndReturnLength(pFileStream, buffer, buflen);
+ *out_buflen =
+ DecodeStreamMaybeCopyAndReturnLength(pFileStream, buffer, buflen);
+ return true;
}
diff --git a/fpdfsdk/fpdf_attachment_embeddertest.cpp b/fpdfsdk/fpdf_attachment_embeddertest.cpp
index 08f00dc..2f79ae5 100644
--- a/fpdfsdk/fpdf_attachment_embeddertest.cpp
+++ b/fpdfsdk/fpdf_attachment_embeddertest.cpp
@@ -37,11 +37,17 @@
EXPECT_EQ(12u, FPDFAttachment_GetName(attachment, buf.data(), length_bytes));
EXPECT_EQ(L"1.txt", GetPlatformWString(buf.data()));
+ // Check some unsuccessful cases of FPDFAttachment_GetFile.
+ EXPECT_FALSE(FPDFAttachment_GetFile(attachment, nullptr, 0, nullptr));
+ EXPECT_FALSE(FPDFAttachment_GetFile(nullptr, nullptr, 0, &length_bytes));
+
// Check that the content of the first attachment is correct.
- length_bytes = FPDFAttachment_GetFile(attachment, nullptr, 0);
+ ASSERT_TRUE(FPDFAttachment_GetFile(attachment, nullptr, 0, &length_bytes));
std::vector<char> content_buf(length_bytes);
- ASSERT_EQ(
- 4u, FPDFAttachment_GetFile(attachment, content_buf.data(), length_bytes));
+ unsigned long actual_length_bytes;
+ ASSERT_TRUE(FPDFAttachment_GetFile(attachment, content_buf.data(),
+ length_bytes, &actual_length_bytes));
+ ASSERT_EQ(4u, actual_length_bytes);
EXPECT_EQ(std::string("test"), std::string(content_buf.data(), 4));
// Check that a non-existent key does not exist.
@@ -68,11 +74,12 @@
ASSERT_TRUE(attachment);
// Retrieve the second attachment file.
- length_bytes = FPDFAttachment_GetFile(attachment, nullptr, 0);
+ ASSERT_TRUE(FPDFAttachment_GetFile(attachment, nullptr, 0, &length_bytes));
content_buf.clear();
content_buf.resize(length_bytes);
- ASSERT_EQ(5869u, FPDFAttachment_GetFile(attachment, content_buf.data(),
- length_bytes));
+ ASSERT_TRUE(FPDFAttachment_GetFile(attachment, content_buf.data(),
+ length_bytes, &actual_length_bytes));
+ ASSERT_EQ(5869u, actual_length_bytes);
// Check that the calculated checksum of the file data matches expectation.
const char kCheckSum[] = "72afcddedf554dda63c0c88e06f1ce18";
@@ -134,10 +141,12 @@
EXPECT_EQ(L"0.txt", GetPlatformWString(buf.data()));
// Verify the content of the new attachment (i.e. the first attachment).
- length_bytes = FPDFAttachment_GetFile(attachment, nullptr, 0);
+ ASSERT_TRUE(FPDFAttachment_GetFile(attachment, nullptr, 0, &length_bytes));
std::vector<char> content_buf(length_bytes);
- ASSERT_EQ(
- 6u, FPDFAttachment_GetFile(attachment, content_buf.data(), length_bytes));
+ unsigned long actual_length_bytes;
+ ASSERT_TRUE(FPDFAttachment_GetFile(attachment, content_buf.data(),
+ length_bytes, &actual_length_bytes));
+ ASSERT_EQ(6u, actual_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.
@@ -159,11 +168,12 @@
EXPECT_EQ(L"z.txt", GetPlatformWString(buf.data()));
// Verify the content of the new attachment (i.e. the fourth attachment).
- length_bytes = FPDFAttachment_GetFile(attachment, nullptr, 0);
+ ASSERT_TRUE(FPDFAttachment_GetFile(attachment, nullptr, 0, &length_bytes));
content_buf.clear();
content_buf.resize(length_bytes);
- ASSERT_EQ(
- 6u, FPDFAttachment_GetFile(attachment, content_buf.data(), length_bytes));
+ ASSERT_TRUE(FPDFAttachment_GetFile(attachment, content_buf.data(),
+ length_bytes, &actual_length_bytes));
+ ASSERT_EQ(6u, actual_length_bytes);
EXPECT_EQ(std::string(kContents2), std::string(content_buf.data(), 6));
}
@@ -203,10 +213,12 @@
EXPECT_EQ(L"5.txt", GetPlatformWString(buf.data()));
// Verify the content of the new attachment.
- length_bytes = FPDFAttachment_GetFile(attachment, nullptr, 0);
+ ASSERT_TRUE(FPDFAttachment_GetFile(attachment, nullptr, 0, &length_bytes));
std::vector<char> content_buf(length_bytes);
- ASSERT_EQ(12u, FPDFAttachment_GetFile(attachment, content_buf.data(),
- length_bytes));
+ unsigned long actual_length_bytes;
+ ASSERT_TRUE(FPDFAttachment_GetFile(attachment, content_buf.data(),
+ length_bytes, &actual_length_bytes));
+ ASSERT_EQ(12u, actual_length_bytes);
EXPECT_EQ(std::string(kContents), std::string(content_buf.data(), 12));
// Verify the creation date of the new attachment.
@@ -230,7 +242,8 @@
// 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));
+ ASSERT_TRUE(FPDFAttachment_GetFile(attachment, nullptr, 0, &length_bytes));
+ EXPECT_EQ(0u, length_bytes);
length_bytes =
FPDFAttachment_GetStringValue(attachment, kChecksumKey, nullptr, 0);
ASSERT_EQ(70u, length_bytes);
@@ -268,10 +281,12 @@
EXPECT_EQ(L"0.txt", GetPlatformWString(buf.data()));
// Verify the content of the new attachment (i.e. the first attachment).
- length_bytes = FPDFAttachment_GetFile(attachment, nullptr, 0);
+ ASSERT_TRUE(FPDFAttachment_GetFile(attachment, nullptr, 0, &length_bytes));
std::vector<char> content_buf(length_bytes);
- ASSERT_EQ(
- 6u, FPDFAttachment_GetFile(attachment, content_buf.data(), length_bytes));
+ unsigned long actual_length_bytes;
+ ASSERT_TRUE(FPDFAttachment_GetFile(attachment, content_buf.data(),
+ length_bytes, &actual_length_bytes));
+ ASSERT_EQ(6u, actual_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.
@@ -293,11 +308,12 @@
EXPECT_EQ(L"z.txt", GetPlatformWString(buf.data()));
// Verify the content of the new attachment (i.e. the second attachment).
- length_bytes = FPDFAttachment_GetFile(attachment, nullptr, 0);
+ ASSERT_TRUE(FPDFAttachment_GetFile(attachment, nullptr, 0, &length_bytes));
content_buf.clear();
content_buf.resize(length_bytes);
- ASSERT_EQ(
- 6u, FPDFAttachment_GetFile(attachment, content_buf.data(), length_bytes));
+ ASSERT_TRUE(FPDFAttachment_GetFile(attachment, content_buf.data(),
+ length_bytes, &actual_length_bytes));
+ ASSERT_EQ(6u, actual_length_bytes);
EXPECT_EQ(std::string(kContents2), std::string(content_buf.data(), 6));
}
diff --git a/public/fpdf_attachment.h b/public/fpdf_attachment.h
index afd4eb0..a097db1 100644
--- a/public/fpdf_attachment.h
+++ b/public/fpdf_attachment.h
@@ -149,19 +149,28 @@
unsigned long len);
// Experimental API.
-// Get the file data of |attachment|. |buffer| is only modified if |buflen| is
-// longer than the length of the file. On errors, |buffer| is unmodified and the
-// returned length is 0.
+// Get the file data of |attachment|.
+// When the attachment file data is readable, true is returned, and |out_buflen|
+// is updated to indicate the file data size. |buffer| is only modified if
+// |buflen| is non-null and long enough to contain the entire file data. Callers
+// must check both the return value and the input |buflen| is no less than the
+// returned |out_buflen| before using the data.
+//
+// Otherwise, when the attachment file data is unreadable or when |out_buflen|
+// is null, false is returned and |buffer| and |out_buflen| remain unmodified.
//
// attachment - handle to an attachment.
// buffer - buffer for holding the file data from |attachment|.
// buflen - length of the buffer in bytes.
+// out_buflen - pointer to the variable that will receive the minimum buffer
+// size to contain the file data of |attachment|.
//
-// Returns the length of the file.
-FPDF_EXPORT unsigned long FPDF_CALLCONV
+// Returns true on success, false otherwise.
+FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV
FPDFAttachment_GetFile(FPDF_ATTACHMENT attachment,
void* buffer,
- unsigned long buflen);
+ unsigned long buflen,
+ unsigned long* out_buflen);
#ifdef __cplusplus
} // extern "C"
diff --git a/samples/pdfium_test_write_helper.cc b/samples/pdfium_test_write_helper.cc
index 122d783..d44a298 100644
--- a/samples/pdfium_test_write_helper.cc
+++ b/samples/pdfium_test_write_helper.cc
@@ -635,20 +635,25 @@
}
// Retrieve the attachment.
- 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()) {
- fprintf(stderr, "Attachment \"%s\" is empty.\n", attachment_name.c_str());
+ if (!FPDFAttachment_GetFile(attachment, nullptr, 0, &length_bytes)) {
+ fprintf(stderr, "Failed to retrieve attachment \"%s\".\n",
+ attachment_name.c_str());
continue;
}
- // Write the attachment file.
+ std::vector<char> data_buf(length_bytes);
+ if (length_bytes) {
+ unsigned long actual_length_bytes;
+ if (!FPDFAttachment_GetFile(attachment, data_buf.data(), length_bytes,
+ &actual_length_bytes)) {
+ fprintf(stderr, "Failed to retrieve attachment \"%s\".\n",
+ attachment_name.c_str());
+ continue;
+ }
+ }
+
+ // Write the attachment file. Since a PDF document could have 0-byte files
+ // as attachments, we should allow saving the 0-byte attachments to files.
WriteBufferToFile(data_buf.data(), length_bytes, save_name, "attachment");
}
}