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