Handle edge case when inline image attachment part is missing a filename

Fixes #1329

First attempt at a fix: #1332

In this second attempt, I tried to scope down the issue to where the issue appears most probable: inline images.

We do not add a default filename for the nameless attachment, rather just ensure that the message part is considered an attachment if it is an inline image.

Author: Lauren Oliveri

CHANGELOG.rdoc

@@ -17,5 +17,7 @@ Compatibility:
 Features:
 * Message#inspect_structure and PartsList#inspect_structure pretty-print the hierarchy of message parts. (TylerRick)
 
+Bugs:
+* Handle case when inline image attachment parts do not contain a filename or Content-Location header. (lao9)
 
 Please check [2-7-stable](https://github.com/mikel/mail/blob/2-7-stable/CHANGELOG.rdoc) for previous changes.

lib/mail/message.rb

@@ -1911,7 +1911,7 @@ def decode_body
     # Returns true if this part is an attachment,
     # false otherwise.
     def attachment?
-      !!find_attachment
+      !!find_attachment_name || inline_image?
     end
 
     # Returns the attachment data if there is any
@@ -1921,7 +1921,7 @@ def attachment
 
     # Returns the filename of the attachment
     def filename
-      find_attachment
+      find_attachment_name
     end
 
     def all_parts
@@ -2117,7 +2117,7 @@ def init_with_string(string)
     end
 
     # Returns the filename of the attachment (if it exists) or returns nil
-    def find_attachment
+    def find_attachment_name
       content_type_name = header[:content_type].filename rescue nil
       content_disp_name = header[:content_disposition].filename rescue nil
       content_loc_name  = header[:content_location].location rescue nil
@@ -2135,6 +2135,14 @@ def find_attachment
       filename
     end
 
+    def inline_image?
+     image? && inline?
+    end
+
+    def image?
+      header[:content_type]&.main_type == "image"
+    end
+
     def do_delivery
       begin
         if perform_deliveries

spec/fixtures/emails/attachment_emails/attachment_missing_filename.eml

@@ -0,0 +1,30 @@
+Mime-Version: 1.0 (Apple Message framework v730)
+Content-Type: multipart/mixed; boundary=Apple-Mail-13-196941151
+Message-Id: <[email protected]>
+From: [email protected]
+Subject: testing
+Date: Mon, 6 Jun 2005 22:21:22 +0200
+To: [email protected]
+
+
+--Apple-Mail-13-196941151
+Content-Transfer-Encoding: quoted-printable
+Content-Type: text/plain;
+	charset=ISO-8859-1;
+	delsp=yes;
+	format=flowed
+
+This is the first part.
+
+--Apple-Mail-13-196941151
+Content-Type: image/jpeg
+Content-Transfer-Encoding: base64
+Content-ID: <qbFGyPQAS8>
+Content-Disposition: inline
+
+jamisSqGSIb3DQEHAqCAMIjamisxCzAJBgUrDgMCGgUAMIAGCSqGSjamisEHAQAAoIIFSjCCBUYw
+ggQujamisQICBD++ukQwDQYJKojamisNAQEFBQAwMTELMAkGA1UEBhMCRjamisAKBgNVBAoTA1RE
+QzEUMBIGjamisxMLVERDIE9DRVMgQ0jamisNMDQwMjI5MTE1OTAxWhcNMDYwMjamisIyOTAxWjCB
+gDELMAkGA1UEjamisEsxKTAnBgNVBAoTIEjamisuIG9yZ2FuaXNhdG9yaXNrIHRpbjamisRuaW5=
+
+--Apple-Mail-13-196941151--

spec/fixtures/emails/attachment_emails/attachment_with_filename.eml

@@ -0,0 +1,30 @@
+Mime-Version: 1.0 (Apple Message framework v730)
+Content-Type: multipart/mixed; boundary=Apple-Mail-13-196941151
+Message-Id: <[email protected]>
+From: [email protected]
+Subject: testing
+Date: Mon, 6 Jun 2005 22:21:22 +0200
+To: [email protected]
+
+
+--Apple-Mail-13-196941151
+Content-Transfer-Encoding: quoted-printable
+Content-Type: text/plain;
+	charset=ISO-8859-1;
+	delsp=yes;
+	format=flowed
+
+This is the first part.
+
+--Apple-Mail-13-196941151
+Content-Type: image/jpeg; filename="Photo25.jpg"
+Content-Transfer-Encoding: base64
+Content-ID: <qbFGyPQAS8>
+Content-Disposition: inline
+
+jamisSqGSIb3DQEHAqCAMIjamisxCzAJBgUrDgMCGgUAMIAGCSqGSjamisEHAQAAoIIFSjCCBUYw
+ggQujamisQICBD++ukQwDQYJKojamisNAQEFBQAwMTELMAkGA1UEBhMCRjamisAKBgNVBAoTA1RE
+QzEUMBIGjamisxMLVERDIE9DRVMgQ0jamisNMDQwMjI5MTE1OTAxWhcNMDYwMjamisIyOTAxWjCB
+gDELMAkGA1UEjamisEsxKTAnBgNVBAoTIEjamisuIG9yZ2FuaXNhdG9yaXNrIHRpbjamisRuaW5=
+
+--Apple-Mail-13-196941151--

spec/mail/attachments_list_spec.rb

@@ -239,6 +239,11 @@ def check_decoded(actual, expected)
       expect(mail.attachments[0].filename).to eq 'api.rb'
     end
 
+    it "should not need filename for inline attachments" do
+      mail = read_fixture('emails/attachment_emails/attachment_missing_filename.eml')
+      expect(mail.attachments.length).to eq 1
+    end
+
     it "should decode an attachment" do
       mail = read_fixture('emails/attachment_emails/attachment_pdf.eml')
       expect(mail.attachments[0].decoded.length).to eq 1026

spec/mail/message_spec.rb

@@ -1439,7 +1439,7 @@ def message_headers_should_match(message, other)
           part.body          = 'a' * 999
         end
         mail.encoded
-        
+
         expect(mail.parts.count).to eq(1)
         expect(mail.parts.last.content_transfer_encoding).to match(/7bit|8bit|binary/)
       end
@@ -1459,19 +1459,19 @@ def message_headers_should_match(message, other)
             end
           end
         end
-        
+
         it "should not add an empty charset header" do
           @mail.charset = nil
-          
+
           expect(@mail.multipart?).to eq true
           expect(@mail.parts.count).to eq 2
           expect(@mail.encoded.scan(/charset=UTF-8/).count).to eq 2
         end
-        
+
         it "should remove the charset header" do
           @mail.charset = 'iso-8859-1'
           @mail.charset = nil
-          
+
           expect(@mail.encoded.scan(/charset=UTF-8/).count).to eq 2
           expect(@mail.encoded.scan(/charset=iso-8859-1/).count).to eq 0
         end
@@ -1495,6 +1495,41 @@ def message_headers_should_match(message, other)
         end
       end
 
+      describe "attachment?" do
+        context "with content location" do
+          subject do
+            read_fixture('emails', 'attachment_emails', 'attachment_content_location.eml')
+          end
+
+          it "returns true using the content location filename" do
+            expect(subject.parts[0].attachment?).to eq(false)
+            expect(subject.parts[1].attachment?).to eq(true)
+          end
+        end
+
+        context "with content type filename" do
+          subject do
+            read_fixture('emails', 'attachment_emails', 'attachment_with_filename.eml')
+          end
+
+          it "returns true using the content type filename" do
+            expect(subject.parts[0].attachment?).to eq(false)
+            expect(subject.parts[1].attachment?).to eq(true)
+          end
+        end
+
+        context "without content location or filename" do
+          subject do
+            read_fixture('emails', 'attachment_emails', 'attachment_missing_filename.eml')
+          end
+
+          it "returns true for the inline attachment" do
+            expect(subject.parts[0].attachment?).to eq(false)
+            expect(subject.parts[1].attachment?).to eq(true)
+          end
+        end
+      end
+
       describe "content-transfer-encoding" do
 
         it "should use 7bit for only US-ASCII chars" do