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"); } }