Add a flag |is_readable| in DocumentAttachmentInfo structure.

Since FPDFAttachment_GetFile() returns false when it fails to retrieve
data from an invalid attachment, this CL adds an |is_readable| flag in
DocumentAttachmentInfo to indicate whether an attachment's data can be
retrieved successfully. If |is_readable| gets set to false, the
attachment's |size_bytes| will remain at its default value of 0.

This CL adds test files embedded_attachments_invalid_data.in and
embedded_attachments_invalid_data.pdf, which contain invalid
attachments. The CL also adds unit tests to check whether |is_readable|
flags are set correctly when loading DocumentAttachmentInfo from both
valid and invalid attachments.

Bug: 177188
Change-Id: Idaef5820d760a837048f96f810f892660216f4d3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2240909
Commit-Queue: Hui Yingst <nigi@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Daniel Hosseinian <dhoss@chromium.org>
Cr-Commit-Position: refs/heads/master@{#787251}

pdf/document_attachment_info.h

@@ -29,6 +29,10 @@ struct DocumentAttachmentInfo {
   // Last modified date of the attachment. It stores the arbitrary string saved
   // in field "ModDate".
   base::string16 modified_date;
+
+  // The flag that indicates whether the attachment can be retrieved
+  // successfully.
+  bool is_readable = false;
 };
 
 }  // namespace chrome_pdf

pdf/pdfium/pdfium_engine.cc

@@ -362,14 +362,6 @@ base::string16 GetAttachmentAttribute(FPDF_ATTACHMENT attachment,
       /*check_expected_size=*/true);
 }
 
-unsigned long GetAttachmentFileLengthInBytes(FPDF_ATTACHMENT attachment) {
-  unsigned long actual_length_bytes;
-  bool is_attachment_readable = FPDFAttachment_GetFile(
-      attachment, /*buffer=*/nullptr, /*buflen=*/0, &actual_length_bytes);
-  DCHECK(is_attachment_readable);
-  return actual_length_bytes;
-}
-
 base::string16 GetAttachmentName(FPDF_ATTACHMENT attachment) {
   return CallPDFiumWideStringBufferApi(
       base::BindRepeating(&FPDFAttachment_GetName, attachment),
@@ -3871,12 +3863,17 @@ void PDFiumEngine::LoadDocumentAttachmentInfoList() {
     DCHECK(attachment);
 
     doc_attachment_info_list_[i].name = GetAttachmentName(attachment);
-    doc_attachment_info_list_[i].size_bytes =
-        GetAttachmentFileLengthInBytes(attachment);
     doc_attachment_info_list_[i].creation_date =
         GetAttachmentAttribute(attachment, "CreationDate");
     doc_attachment_info_list_[i].modified_date =
         GetAttachmentAttribute(attachment, "ModDate");
+
+    unsigned long actual_length_bytes;
+    doc_attachment_info_list_[i].is_readable =
+        FPDFAttachment_GetFile(attachment, /*buffer=*/nullptr,
+                               /*buflen=*/0, &actual_length_bytes);
+    if (doc_attachment_info_list_[i].is_readable)
+      doc_attachment_info_list_[i].size_bytes = actual_length_bytes;
   }
 }
 

pdf/pdfium/pdfium_engine_unittest.cc

@@ -200,6 +200,7 @@ TEST_F(PDFiumEngineTest, GetDocumentAttachmentInfo) {
   {
     const DocumentAttachmentInfo& attachment = attachments[0];
     EXPECT_EQ("1.txt", base::UTF16ToUTF8(attachment.name));
+    EXPECT_TRUE(attachment.is_readable);
     EXPECT_EQ(4u, attachment.size_bytes);
     EXPECT_EQ("D:20170712214438-07'00'",
               base::UTF16ToUTF8(attachment.creation_date));
@@ -209,6 +210,7 @@ TEST_F(PDFiumEngineTest, GetDocumentAttachmentInfo) {
   {
     const DocumentAttachmentInfo& attachment = attachments[1];
     EXPECT_EQ("attached.pdf", base::UTF16ToUTF8(attachment.name));
+    EXPECT_TRUE(attachment.is_readable);
     EXPECT_EQ(5869u, attachment.size_bytes);
     EXPECT_EQ("D:20170712214443-07'00'",
               base::UTF16ToUTF8(attachment.creation_date));
@@ -219,12 +221,33 @@ TEST_F(PDFiumEngineTest, GetDocumentAttachmentInfo) {
     // Test attachments with no creation date or last modified date.
     const DocumentAttachmentInfo& attachment = attachments[2];
     EXPECT_EQ("附錄.txt", base::UTF16ToUTF8(attachment.name));
+    EXPECT_TRUE(attachment.is_readable);
     EXPECT_EQ(5u, attachment.size_bytes);
     EXPECT_THAT(attachment.creation_date, IsEmpty());
     EXPECT_THAT(attachment.modified_date, IsEmpty());
   }
 }
 
+TEST_F(PDFiumEngineTest, DocumentWithInvalidAttachment) {
+  NiceMock<MockTestClient> client;
+  std::unique_ptr<PDFiumEngine> engine = InitializeEngine(
+      &client, FILE_PATH_LITERAL("embedded_attachments_invalid_data.pdf"));
+  ASSERT_TRUE(engine);
+
+  const std::vector<DocumentAttachmentInfo>& attachments =
+      engine->GetDocumentAttachmentInfoList();
+  ASSERT_EQ(1u, attachments.size());
+
+  // Test on an attachment which FPDFAttachment_GetFile() fails to retrieve data
+  // from.
+  const DocumentAttachmentInfo& attachment = attachments[0];
+  EXPECT_EQ("1.txt", base::UTF16ToUTF8(attachment.name));
+  EXPECT_FALSE(attachment.is_readable);
+  EXPECT_EQ(0u, attachment.size_bytes);
+  EXPECT_THAT(attachment.creation_date, IsEmpty());
+  EXPECT_THAT(attachment.modified_date, IsEmpty());
+}
+
 TEST_F(PDFiumEngineTest, NoDocumentAttachmentInfo) {
   NiceMock<MockTestClient> client;
   std::unique_ptr<PDFiumEngine> engine =

pdf/test/data/embedded_attachments_invalid_data.in

@@ -0,0 +1,41 @@
+{{header}}
+{{object 1 0}} <<
+  /Type /Catalog
+  /Names 4 0 R
+  /Pages 2 0 R
+>>
+endobj
+{{object 2 0}} <<
+  /Type /Pages
+  /Count 1
+  /Kids [3 0 R]
+>>
+endobj
+{{object 3 0}} <<
+  /Type /Page
+  /Parent 2 0 R
+  /MediaBox [0 0 612 792]
+  /Resources <<
+    /ProcSet [/PDF]
+  >>
+>>
+endobj
+{{object 4 0}} <<
+  /EmbeddedFiles 5 0 R
+>>
+endobj
+{{object 5 0}} <<
+  /Names [(1.txt) 6 0 R]
+>>
+endobj
+{{object 6 0}} <<
+  /Type /Filespec
+  /Desc ()
+  /F (1.txt)
+  /UF (1.txt)
+>>
+endobj
+{{xref}}
+{{trailer}}
+{{startxref}}
+%%EOF

pdf/test/data/embedded_attachments_invalid_data.pdf

@@ -0,0 +1,54 @@
+%PDF-1.7
+%<25><><EFBFBD><EFBFBD>
+1 0 obj <<
+  /Type /Catalog
+  /Names 4 0 R
+  /Pages 2 0 R
+>>
+endobj
+2 0 obj <<
+  /Type /Pages
+  /Count 1
+  /Kids [3 0 R]
+>>
+endobj
+3 0 obj <<
+  /Type /Page
+  /Parent 2 0 R
+  /MediaBox [0 0 612 792]
+  /Resources <<
+    /ProcSet [/PDF]
+  >>
+>>
+endobj
+4 0 obj <<
+  /EmbeddedFiles 5 0 R
+>>
+endobj
+5 0 obj <<
+  /Names [(1.txt) 6 0 R]
+>>
+endobj
+6 0 obj <<
+  /Type /Filespec
+  /Desc ()
+  /F (1.txt)
+  /UF (1.txt)
+>>
+endobj
+xref
+0 7
+0000000000 65535 f 
+0000000015 00000 n 
+0000000083 00000 n 
+0000000146 00000 n 
+0000000264 00000 n 
+0000000308 00000 n 
+0000000354 00000 n 
+trailer <<
+  /Root 1 0 R
+  /Size 7
+>>
+startxref
+431
+%%EOF