From e3a17f1438e454518a58fc0841bb615344acf865 Mon Sep 17 00:00:00 2001 From: Andy Stadler Date: Wed, 15 Dec 2010 11:36:02 -0800 Subject: [PATCH] Restrict download/display to "attachment" and "inline" * Check content-disposition and restrict to these two types * Add unit test * Reformatting collectParts (code style cleanup) Bug: 3242502 Change-Id: I5dcbdda5d4788502113771f4fd1b5fff834a402d --- .../email/mail/internet/MimeUtility.java | 72 +++++++---------- .../android/email/LegacyConversionsTests.java | 81 +++++++++++++++++-- 2 files changed, 104 insertions(+), 49 deletions(-) diff --git a/src/com/android/email/mail/internet/MimeUtility.java b/src/com/android/email/mail/internet/MimeUtility.java index 0b2f5a5e8..e3145f950 100644 --- a/src/com/android/email/mail/internet/MimeUtility.java +++ b/src/com/android/email/mail/internet/MimeUtility.java @@ -385,13 +385,14 @@ public class MimeUtility { } /** - * An unfortunately named method that makes decisions about a Part (usually a Message) - * as to which of it's children will be "viewable" and which will be attachments. - * The method recursively sorts the viewables and attachments into seperate - * lists for further processing. - * @param part - * @param viewables - * @param attachments + * Recursively scan a Part (usually a Message) and sort out which of its children will be + * "viewable" and which will be attachments. + * + * @param part The part to be broken down + * @param viewables This arraylist will be populated with all parts that appear to be + * the "message" (e.g. text/plain & text/html) + * @param attachments This arraylist will be populated with all parts that appear to be + * attachments (including inlines) * @throws MessagingException */ public static void collectParts(Part part, ArrayList viewables, @@ -403,51 +404,40 @@ public class MimeUtility { dispositionType = MimeUtility.getHeaderParameter(disposition, null); dispositionFilename = MimeUtility.getHeaderParameter(disposition, "filename"); } + boolean attachmentDisposition = "attachment".equalsIgnoreCase(dispositionType); + boolean inlineDisposition = "inline".equalsIgnoreCase(dispositionType); - /* - * A best guess that this part is intended to be an attachment and not inline. - */ - boolean attachment = ("attachment".equalsIgnoreCase(dispositionType)) - || (dispositionFilename != null) - && (!"inline".equalsIgnoreCase(dispositionType)); + // A guess that this part is intended to be an attachment + boolean attachment = attachmentDisposition + || (dispositionFilename != null && !inlineDisposition); + + // A guess that this part is intended to be an inline. + boolean inline = inlineDisposition && (dispositionFilename != null); + + // One or the other + boolean attachmentOrInline = attachment || inline; - /* - * If the part is Multipart but not alternative it's either mixed or - * something we don't know about, which means we treat it as mixed - * per the spec. We just process it's pieces recursively. - */ if (part.getBody() instanceof Multipart) { + // If the part is Multipart but not alternative it's either mixed or + // something we don't know about, which means we treat it as mixed + // per the spec. We just process its pieces recursively. Multipart mp = (Multipart)part.getBody(); for (int i = 0; i < mp.getCount(); i++) { collectParts(mp.getBodyPart(i), viewables, attachments); } - } - /* - * If the part is an embedded message we just continue to process - * it, pulling any viewables or attachments into the running list. - */ - else if (part.getBody() instanceof Message) { + } else if (part.getBody() instanceof Message) { + // If the part is an embedded message we just continue to process + // it, pulling any viewables or attachments into the running list. Message message = (Message)part.getBody(); collectParts(message, viewables, attachments); - } - /* - * If the part is HTML and it got this far it's part of a mixed (et - * al) and should be rendered inline. - */ - else if ((!attachment) && (part.getMimeType().equalsIgnoreCase("text/html"))) { + } else if ((!attachmentOrInline) && ("text/html".equalsIgnoreCase(part.getMimeType()))) { + // If the part is HTML and we got this far, it's a viewable part of a mixed viewables.add(part); - } - /* - * If the part is plain text and it got this far it's part of a - * mixed (et al) and should be rendered inline. - */ - else if ((!attachment) && (part.getMimeType().equalsIgnoreCase("text/plain"))) { + } else if ((!attachmentOrInline) && ("text/plain".equalsIgnoreCase(part.getMimeType()))) { + // If the part is text and we got this far, it's a viewable part of a mixed viewables.add(part); - } - /* - * Finally, if it's nothing else we will include it as an attachment. - */ - else { + } else if (attachmentOrInline) { + // Finally, if it's an attachment or an inline we will include it as an attachment. attachments.add(part); } } diff --git a/tests/src/com/android/email/LegacyConversionsTests.java b/tests/src/com/android/email/LegacyConversionsTests.java index fc0f9bd00..3ec8a41ab 100644 --- a/tests/src/com/android/email/LegacyConversionsTests.java +++ b/tests/src/com/android/email/LegacyConversionsTests.java @@ -242,10 +242,10 @@ public class LegacyConversionsTests extends ProviderTestCase2 { assertEquals(2, c.getCount()); while (c.moveToNext()) { Attachment attachment = Attachment.getContent(c, Attachment.class); - if ("101".equals(attachment.mLocation)) { + if ("100".equals(attachment.mLocation)) { checkAttachment("attachment1Part", attachments.get(0), attachment, localMessage.mAccountKey); - } else if ("102".equals(attachment.mLocation)) { + } else if ("101".equals(attachment.mLocation)) { checkAttachment("attachment2Part", attachments.get(1), attachment, localMessage.mAccountKey); } else { @@ -257,6 +257,57 @@ public class LegacyConversionsTests extends ProviderTestCase2 { } } + /** + * Test that only "attachment" or "inline" attachments are captured and added. + * @throws MessagingException + * @throws IOException + */ + public void testAttachmentDispositions() throws MessagingException, IOException { + // Prepare a local message to add the attachments to + final long accountId = 1; + final long mailboxId = 1; + + // Prepare the three attachments we want to test + BodyPart[] sourceAttachments = new BodyPart[3]; + BodyPart attachmentPart; + + // 1. Standard attachment + attachmentPart = MessageTestUtils.bodyPart("image/jpg", null); + attachmentPart.setHeader(MimeHeader.HEADER_CONTENT_TYPE, "image/jpg"); + attachmentPart.setHeader(MimeHeader.HEADER_CONTENT_TRANSFER_ENCODING, "base64"); + attachmentPart.setHeader(MimeHeader.HEADER_CONTENT_DISPOSITION, + "attachment;\n filename=\"file-1\";\n size=100"); + attachmentPart.setHeader(MimeHeader.HEADER_ANDROID_ATTACHMENT_STORE_DATA, "100"); + sourceAttachments[0] = attachmentPart; + + // 2. Inline attachment + attachmentPart = MessageTestUtils.bodyPart("image/gif", null); + attachmentPart.setHeader(MimeHeader.HEADER_CONTENT_TYPE, "image/gif"); + attachmentPart.setHeader(MimeHeader.HEADER_CONTENT_TRANSFER_ENCODING, "base64"); + attachmentPart.setHeader(MimeHeader.HEADER_CONTENT_DISPOSITION, + "inline;\n filename=\"file-2\";\n size=200"); + attachmentPart.setHeader(MimeHeader.HEADER_ANDROID_ATTACHMENT_STORE_DATA, "101"); + sourceAttachments[1] = attachmentPart; + + // 3. Neither (use VCALENDAR) + attachmentPart = MessageTestUtils.bodyPart("text/calendar", null); + attachmentPart.setHeader(MimeHeader.HEADER_CONTENT_TYPE, + "text/calendar; charset=UTF-8; method=REQUEST"); + attachmentPart.setHeader(MimeHeader.HEADER_CONTENT_TRANSFER_ENCODING, "7bit"); + attachmentPart.setHeader(MimeHeader.HEADER_ANDROID_ATTACHMENT_STORE_DATA, "102"); + sourceAttachments[2] = attachmentPart; + + // Prepare local message (destination) and legacy message w/attachments (source) + final EmailContent.Message localMessage = ProviderTestUtils.setupMessage( + "local-message", accountId, mailboxId, false, true, mProviderContext); + final Message legacyMessage = prepareLegacyMessageWithAttachments(sourceAttachments); + convertAndCheckcheckAddedAttachments(localMessage, legacyMessage); + + // Run the conversion and check for the converted attachments - this test asserts + // that there are two attachments numbered 100 & 101 (so will fail if it finds 102) + convertAndCheckcheckAddedAttachments(localMessage, legacyMessage); + } + /** * Test that attachments aren't re-added in the DB. This supports the "partial download" * nature of POP messages. @@ -350,9 +401,8 @@ public class LegacyConversionsTests extends ProviderTestCase2 { */ private Message prepareLegacyMessageWithAttachments(int numAttachments, boolean localData, boolean filenameInDisposition) throws MessagingException { - // First, build one or more attachment parts - MultipartBuilder mpBuilder = new MultipartBuilder("multipart/mixed"); - for (int i = 1; i <= numAttachments; ++i) { + BodyPart[] attachmentParts = new BodyPart[numAttachments]; + for (int i = 0; i < numAttachments; ++i) { // construct parameter parts for content-type:name or content-disposition:filename. String name = ""; String filename = ""; @@ -372,7 +422,7 @@ public class LegacyConversionsTests extends ProviderTestCase2 { bp.setHeader(MimeHeader.HEADER_CONTENT_TYPE, "image/jpg" + name); bp.setHeader(MimeHeader.HEADER_CONTENT_TRANSFER_ENCODING, "base64"); bp.setHeader(MimeHeader.HEADER_CONTENT_DISPOSITION, "attachment" + filename); - mpBuilder.addBodyPart(bp); + attachmentParts[i] = bp; } else { // generate an attachment that came from a server BodyPart attachmentPart = MessageTestUtils.bodyPart("image/jpg", null); @@ -381,13 +431,28 @@ public class LegacyConversionsTests extends ProviderTestCase2 { attachmentPart.setHeader(MimeHeader.HEADER_CONTENT_TYPE, "image/jpg" + name); attachmentPart.setHeader(MimeHeader.HEADER_CONTENT_TRANSFER_ENCODING, "base64"); attachmentPart.setHeader(MimeHeader.HEADER_CONTENT_DISPOSITION, - "attachment" + filename + ";\n size=" + i + "00"); + "attachment" + filename + ";\n size=" + (i+1) + "00"); attachmentPart.setHeader(MimeHeader.HEADER_ANDROID_ATTACHMENT_STORE_DATA, "10" + i); - mpBuilder.addBodyPart(attachmentPart); + attachmentParts[i] = attachmentPart; } } + return prepareLegacyMessageWithAttachments(attachmentParts); + } + + /** + * Prepare a legacy message with 1+ attachments + * @param attachments array containing one or more attachments + */ + private Message prepareLegacyMessageWithAttachments(BodyPart[] attachments) + throws MessagingException { + // Build the multipart that holds the attachments + MultipartBuilder mpBuilder = new MultipartBuilder("multipart/mixed"); + for (int i = 0; i < attachments.length; ++i) { + mpBuilder.addBodyPart(attachments[i]); + } + // Now build a message with them final Message legacyMessage = new MessageBuilder() .setBody(new MultipartBuilder("multipart/mixed")