From 36a3a8bb20698c7db80c179cd94ec539cbaeb160 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 23 Jun 2016 19:06:30 +0200 Subject: [PATCH 01/19] rename DecryptedStreamParser and DecryptedTempFileBody --- ...StreamParser.java => MimePartStreamParser.java} | 5 ++--- ...TempFileBody.java => ProvidedTempFileBody.java} | 4 ++-- .../extractors/AttachmentInfoExtractor.java | 14 ++++++-------- .../com/fsck/k9/ui/crypto/MessageCryptoHelper.java | 4 ++-- 4 files changed, 12 insertions(+), 15 deletions(-) rename k9mail/src/main/java/com/fsck/k9/mailstore/{DecryptStreamParser.java => MimePartStreamParser.java} (97%) rename k9mail/src/main/java/com/fsck/k9/mailstore/{DecryptedTempFileBody.java => ProvidedTempFileBody.java} (94%) diff --git a/k9mail/src/main/java/com/fsck/k9/mailstore/DecryptStreamParser.java b/k9mail/src/main/java/com/fsck/k9/mailstore/MimePartStreamParser.java similarity index 97% rename from k9mail/src/main/java/com/fsck/k9/mailstore/DecryptStreamParser.java rename to k9mail/src/main/java/com/fsck/k9/mailstore/MimePartStreamParser.java index ee50cd0bf..92315350c 100644 --- a/k9mail/src/main/java/com/fsck/k9/mailstore/DecryptStreamParser.java +++ b/k9mail/src/main/java/com/fsck/k9/mailstore/MimePartStreamParser.java @@ -34,8 +34,7 @@ import org.apache.james.mime4j.stream.Field; import org.apache.james.mime4j.stream.MimeConfig; import org.apache.james.mime4j.util.MimeUtil; -// TODO rename this class? this class doesn't really bear any 'decrypted' semantics anymore... -public class DecryptStreamParser { +public class MimePartStreamParser { private static final String DECRYPTED_CACHE_DIRECTORY = "decrypted"; @@ -83,7 +82,7 @@ public class DecryptStreamParser { private static Body createBody(InputStream inputStream, String transferEncoding, File decryptedTempDirectory) throws IOException { - DecryptedTempFileBody body = new DecryptedTempFileBody(decryptedTempDirectory, transferEncoding); + ProvidedTempFileBody body = new ProvidedTempFileBody(decryptedTempDirectory, transferEncoding); OutputStream outputStream = body.getOutputStream(); try { InputStream decodingInputStream; diff --git a/k9mail/src/main/java/com/fsck/k9/mailstore/DecryptedTempFileBody.java b/k9mail/src/main/java/com/fsck/k9/mailstore/ProvidedTempFileBody.java similarity index 94% rename from k9mail/src/main/java/com/fsck/k9/mailstore/DecryptedTempFileBody.java rename to k9mail/src/main/java/com/fsck/k9/mailstore/ProvidedTempFileBody.java index b96030dfe..d8365242e 100644 --- a/k9mail/src/main/java/com/fsck/k9/mailstore/DecryptedTempFileBody.java +++ b/k9mail/src/main/java/com/fsck/k9/mailstore/ProvidedTempFileBody.java @@ -17,7 +17,7 @@ import com.fsck.k9.mail.MessagingException; import com.fsck.k9.mail.internet.SizeAware; import org.apache.commons.io.output.DeferredFileOutputStream; -public class DecryptedTempFileBody extends BinaryAttachmentBody implements SizeAware { +public class ProvidedTempFileBody extends BinaryAttachmentBody implements SizeAware { public static final int MEMORY_BACKED_THRESHOLD = 1024 * 8; @@ -28,7 +28,7 @@ public class DecryptedTempFileBody extends BinaryAttachmentBody implements SizeA private byte[] data; - public DecryptedTempFileBody(File tempDirectory, String transferEncoding) { + public ProvidedTempFileBody(File tempDirectory, String transferEncoding) { this.tempDirectory = tempDirectory; try { setEncoding(transferEncoding); diff --git a/k9mail/src/main/java/com/fsck/k9/message/extractors/AttachmentInfoExtractor.java b/k9mail/src/main/java/com/fsck/k9/message/extractors/AttachmentInfoExtractor.java index d510a55c1..cc4cc22e8 100644 --- a/k9mail/src/main/java/com/fsck/k9/message/extractors/AttachmentInfoExtractor.java +++ b/k9mail/src/main/java/com/fsck/k9/message/extractors/AttachmentInfoExtractor.java @@ -15,7 +15,7 @@ import com.fsck.k9.mail.Part; import com.fsck.k9.mail.internet.MimeHeader; import com.fsck.k9.mail.internet.MimeUtility; import com.fsck.k9.mailstore.AttachmentViewInfo; -import com.fsck.k9.mailstore.DecryptedTempFileBody; +import com.fsck.k9.mailstore.ProvidedTempFileBody; import com.fsck.k9.mailstore.LocalPart; import com.fsck.k9.provider.AttachmentProvider; import com.fsck.k9.provider.K9FileProvider; @@ -44,18 +44,16 @@ public class AttachmentInfoExtractor { uri = AttachmentProvider.getAttachmentUri(accountUuid, messagePartId); } else { Body body = part.getBody(); - if (body instanceof DecryptedTempFileBody) { - DecryptedTempFileBody decryptedTempFileBody = (DecryptedTempFileBody) body; + if (body instanceof ProvidedTempFileBody) { + ProvidedTempFileBody providedTempFileBody = (ProvidedTempFileBody) body; try { - size = decryptedTempFileBody.getSize(); - - File file = decryptedTempFileBody.getFile(); + File file = providedTempFileBody.getFile(); uri = K9FileProvider.getUriForFile(context, file, part.getMimeType()); + size = providedTempFileBody.getSize(); + return extractAttachmentInfo(part, uri, size); } catch (IOException e) { throw new MessagingException("Error preparing decrypted data as attachment", e); } - - return extractAttachmentInfo(part, uri, size); } else { throw new UnsupportedOperationException(); } diff --git a/k9mail/src/main/java/com/fsck/k9/ui/crypto/MessageCryptoHelper.java b/k9mail/src/main/java/com/fsck/k9/ui/crypto/MessageCryptoHelper.java index 7193ad933..623e4a95c 100644 --- a/k9mail/src/main/java/com/fsck/k9/ui/crypto/MessageCryptoHelper.java +++ b/k9mail/src/main/java/com/fsck/k9/ui/crypto/MessageCryptoHelper.java @@ -34,7 +34,7 @@ import com.fsck.k9.mail.internet.SizeAware; import com.fsck.k9.mail.internet.TextBody; import com.fsck.k9.mailstore.CryptoResultAnnotation; import com.fsck.k9.mailstore.CryptoResultAnnotation.CryptoError; -import com.fsck.k9.mailstore.DecryptStreamParser; +import com.fsck.k9.mailstore.MimePartStreamParser; import com.fsck.k9.mailstore.LocalMessage; import com.fsck.k9.mailstore.MessageHelper; import org.apache.commons.io.IOUtils; @@ -425,7 +425,7 @@ public class MessageCryptoHelper { @WorkerThread public MimeBodyPart processData(InputStream is) throws IOException { try { - return DecryptStreamParser.parse(context, is); + return MimePartStreamParser.parse(context, is); } catch (MessagingException e) { Log.e(K9.LOG_TAG, "Something went wrong while parsing the decrypted MIME part", e); //TODO: pass error to main thread and display error message to user From e102a1d474772856049d769fa3a6a0efa1a65354 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 23 Jun 2016 19:22:08 +0200 Subject: [PATCH 02/19] introduce FileProviderInterface, specialize K9FileProvider for decrypted --- k9mail/src/main/AndroidManifest.xml | 6 +- .../fsck/k9/mailstore/AttachmentResolver.java | 10 +-- .../com/fsck/k9/mailstore/LocalFolder.java | 4 +- .../mailstore/MessageViewInfoExtractor.java | 5 +- .../k9/mailstore/MimePartStreamParser.java | 66 ++++------------ .../k9/mailstore/ProvidedTempFileBody.java | 26 ++++--- .../extractors/AttachmentInfoExtractor.java | 26 +++---- .../k9/provider/DecryptedFileProvider.java | 76 +++++++++++++++++++ .../com/fsck/k9/provider/K9FileProvider.java | 25 ------ .../FileProviderDeferredFileOutputStream.java | 63 +++++++++++++++ .../k9/service/FileProviderInterface.java | 15 ++++ .../k9/ui/compose/QuotedMessagePresenter.java | 5 +- .../k9/ui/crypto/MessageCryptoHelper.java | 6 +- .../res/xml/allowed_file_provider_paths.xml | 3 - .../res/xml/decrypted_file_provider_paths.xml | 3 + 15 files changed, 217 insertions(+), 122 deletions(-) create mode 100644 k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java delete mode 100644 k9mail/src/main/java/com/fsck/k9/provider/K9FileProvider.java create mode 100644 k9mail/src/main/java/com/fsck/k9/service/FileProviderDeferredFileOutputStream.java create mode 100644 k9mail/src/main/java/com/fsck/k9/service/FileProviderInterface.java delete mode 100644 k9mail/src/main/res/xml/allowed_file_provider_paths.xml create mode 100644 k9mail/src/main/res/xml/decrypted_file_provider_paths.xml diff --git a/k9mail/src/main/AndroidManifest.xml b/k9mail/src/main/AndroidManifest.xml index 31c4f1f3f..eb5c6a786 100644 --- a/k9mail/src/main/AndroidManifest.xml +++ b/k9mail/src/main/AndroidManifest.xml @@ -415,14 +415,14 @@ android:exported="false"/> + android:resource="@xml/decrypted_file_provider_paths" /> diff --git a/k9mail/src/main/java/com/fsck/k9/mailstore/AttachmentResolver.java b/k9mail/src/main/java/com/fsck/k9/mailstore/AttachmentResolver.java index eb2faa321..07f084165 100644 --- a/k9mail/src/main/java/com/fsck/k9/mailstore/AttachmentResolver.java +++ b/k9mail/src/main/java/com/fsck/k9/mailstore/AttachmentResolver.java @@ -6,7 +6,6 @@ import java.util.HashMap; import java.util.Map; import java.util.Stack; -import android.content.Context; import android.net.Uri; import android.support.annotation.Nullable; import android.support.annotation.WorkerThread; @@ -41,13 +40,12 @@ public class AttachmentResolver { } @WorkerThread - public static AttachmentResolver createFromPart(Context context, Part part) { - Map contentIdToAttachmentUriMap = buildCidToAttachmentUriMap(context, part); - + public static AttachmentResolver createFromPart(Part part) { + Map contentIdToAttachmentUriMap = buildCidToAttachmentUriMap(part); return new AttachmentResolver(contentIdToAttachmentUriMap); } - private static Map buildCidToAttachmentUriMap(Context context, Part rootPart) { + private static Map buildCidToAttachmentUriMap(Part rootPart) { HashMap result = new HashMap<>(); Stack partsToCheck = new Stack<>(); @@ -66,7 +64,7 @@ public class AttachmentResolver { try { String contentId = part.getContentId(); if (contentId != null) { - AttachmentViewInfo attachmentInfo = AttachmentInfoExtractor.extractAttachmentInfo(context, part); + AttachmentViewInfo attachmentInfo = AttachmentInfoExtractor.extractAttachmentInfo(part); result.put(contentId, attachmentInfo.uri); } } catch (MessagingException e) { diff --git a/k9mail/src/main/java/com/fsck/k9/mailstore/LocalFolder.java b/k9mail/src/main/java/com/fsck/k9/mailstore/LocalFolder.java index 62e70634f..974cb9dcd 100644 --- a/k9mail/src/main/java/com/fsck/k9/mailstore/LocalFolder.java +++ b/k9mail/src/main/java/com/fsck/k9/mailstore/LocalFolder.java @@ -1401,7 +1401,7 @@ public class LocalFolder extends Folder implements Serializable { } private void missingPartToContentValues(ContentValues cv, Part part) throws MessagingException { - AttachmentViewInfo attachment = AttachmentInfoExtractor.extractAttachmentInfo(part); + AttachmentViewInfo attachment = AttachmentInfoExtractor.extractAttachmentInfoForDatabase(part); cv.put("display_name", attachment.displayName); cv.put("data_location", DataLocation.MISSING); cv.put("decoded_body_size", attachment.size); @@ -1417,7 +1417,7 @@ public class LocalFolder extends Folder implements Serializable { private File leafPartToContentValues(ContentValues cv, Part part, Body body) throws MessagingException, IOException { - AttachmentViewInfo attachment = AttachmentInfoExtractor.extractAttachmentInfo(part); + AttachmentViewInfo attachment = AttachmentInfoExtractor.extractAttachmentInfoForDatabase(part); cv.put("display_name", attachment.displayName); String encoding = getTransferEncoding(part); diff --git a/k9mail/src/main/java/com/fsck/k9/mailstore/MessageViewInfoExtractor.java b/k9mail/src/main/java/com/fsck/k9/mailstore/MessageViewInfoExtractor.java index 0a9f17a8a..aee469e60 100644 --- a/k9mail/src/main/java/com/fsck/k9/mailstore/MessageViewInfoExtractor.java +++ b/k9mail/src/main/java/com/fsck/k9/mailstore/MessageViewInfoExtractor.java @@ -70,8 +70,7 @@ public class MessageViewInfoExtractor { extraViewableText = extraViewable.text; } - AttachmentResolver attachmentResolver = - AttachmentResolver.createFromPart(context, rootPart); + AttachmentResolver attachmentResolver = AttachmentResolver.createFromPart(rootPart); return MessageViewInfo.createWithExtractedContent(message, rootPart, viewable.html, attachmentInfos, cryptoResultAnnotation, extraViewableText, extraAttachmentInfos, attachmentResolver); @@ -86,7 +85,7 @@ public class MessageViewInfoExtractor { MessageExtractor.findViewablesAndAttachments(part, viewableParts, attachments); } - attachmentInfos.addAll(AttachmentInfoExtractor.extractAttachmentInfos(context, attachments)); + attachmentInfos.addAll(AttachmentInfoExtractor.extractAttachmentInfos(attachments)); return MessageViewInfoExtractor.extractTextFromViewables(context, viewableParts); } diff --git a/k9mail/src/main/java/com/fsck/k9/mailstore/MimePartStreamParser.java b/k9mail/src/main/java/com/fsck/k9/mailstore/MimePartStreamParser.java index 92315350c..71fadd402 100644 --- a/k9mail/src/main/java/com/fsck/k9/mailstore/MimePartStreamParser.java +++ b/k9mail/src/main/java/com/fsck/k9/mailstore/MimePartStreamParser.java @@ -1,17 +1,11 @@ package com.fsck.k9.mailstore; -import java.io.BufferedInputStream; -import java.io.File; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.util.Stack; -import android.content.Context; -import android.util.Log; - -import com.fsck.k9.K9; import com.fsck.k9.mail.Body; import com.fsck.k9.mail.BodyPart; import com.fsck.k9.mail.Message; @@ -22,6 +16,7 @@ import com.fsck.k9.mail.internet.MimeBodyPart; import com.fsck.k9.mail.internet.MimeMessage; import com.fsck.k9.mail.internet.MimeMultipart; import com.fsck.k9.mail.internet.MimeUtility; +import com.fsck.k9.service.FileProviderInterface; import org.apache.commons.io.IOUtils; import org.apache.james.mime4j.MimeException; import org.apache.james.mime4j.codec.Base64InputStream; @@ -36,19 +31,9 @@ import org.apache.james.mime4j.util.MimeUtil; public class MimePartStreamParser { - private static final String DECRYPTED_CACHE_DIRECTORY = "decrypted"; - - public static MimeBodyPart parse(Context context, InputStream inputStream) throws MessagingException, IOException { - inputStream = new BufferedInputStream(inputStream, 4096); - - boolean hasInputData = waitForInputData(inputStream); - if (!hasInputData) { - return null; - } - - File decryptedTempDirectory = getDecryptedTempDirectory(context); - - MimeBodyPart decryptedRootPart = new MimeBodyPart(); + public static MimeBodyPart parse(FileProviderInterface fileProviderInterface, InputStream inputStream) + throws MessagingException, IOException { + MimeBodyPart parsedRootPart = new MimeBodyPart(); MimeConfig parserConfig = new MimeConfig(); parserConfig.setMaxHeaderLen(-1); @@ -56,7 +41,7 @@ public class MimePartStreamParser { parserConfig.setMaxHeaderCount(-1); MimeStreamParser parser = new MimeStreamParser(parserConfig); - parser.setContentHandler(new PartBuilder(decryptedTempDirectory, decryptedRootPart)); + parser.setContentHandler(new PartBuilder(fileProviderInterface, parsedRootPart)); parser.setRecurse(); try { @@ -65,24 +50,12 @@ public class MimePartStreamParser { throw new MessagingException("Failed to parse decrypted content", e); } - return decryptedRootPart; + return parsedRootPart; } - private static boolean waitForInputData(InputStream inputStream) { - try { - inputStream.mark(1); - int ret = inputStream.read(); - inputStream.reset(); - return ret != -1; - } catch (IOException e) { - Log.d(K9.LOG_TAG, "got no input from pipe", e); - return false; - } - } - - private static Body createBody(InputStream inputStream, String transferEncoding, File decryptedTempDirectory) - throws IOException { - ProvidedTempFileBody body = new ProvidedTempFileBody(decryptedTempDirectory, transferEncoding); + private static Body createBody(InputStream inputStream, String transferEncoding, + FileProviderInterface fileProviderInterface) throws IOException { + ProvidedTempFileBody body = new ProvidedTempFileBody(fileProviderInterface, transferEncoding); OutputStream outputStream = body.getOutputStream(); try { InputStream decodingInputStream; @@ -112,26 +85,15 @@ public class MimePartStreamParser { return body; } - private static File getDecryptedTempDirectory(Context context) { - File directory = new File(context.getCacheDir(), DECRYPTED_CACHE_DIRECTORY); - if (!directory.exists()) { - if (!directory.mkdir()) { - Log.e(K9.LOG_TAG, "Error creating directory: " + directory.getAbsolutePath()); - } - } - - return directory; - } - private static class PartBuilder implements ContentHandler { - private final File decryptedTempDirectory; + private final FileProviderInterface fileProviderInterface; private final MimeBodyPart decryptedRootPart; - private final Stack stack = new Stack(); + private final Stack stack = new Stack<>(); - public PartBuilder(File decryptedTempDirectory, MimeBodyPart decryptedRootPart) + public PartBuilder(FileProviderInterface fileProviderInterface, MimeBodyPart decryptedRootPart) throws MessagingException { - this.decryptedTempDirectory = decryptedTempDirectory; + this.fileProviderInterface = fileProviderInterface; this.decryptedRootPart = decryptedRootPart; } @@ -233,7 +195,7 @@ public class MimePartStreamParser { Part part = (Part) stack.peek(); String transferEncoding = bd.getTransferEncoding(); - Body body = createBody(inputStream, transferEncoding, decryptedTempDirectory); + Body body = createBody(inputStream, transferEncoding, fileProviderInterface); part.setBody(body); } diff --git a/k9mail/src/main/java/com/fsck/k9/mailstore/ProvidedTempFileBody.java b/k9mail/src/main/java/com/fsck/k9/mailstore/ProvidedTempFileBody.java index d8365242e..a36b3504d 100644 --- a/k9mail/src/main/java/com/fsck/k9/mailstore/ProvidedTempFileBody.java +++ b/k9mail/src/main/java/com/fsck/k9/mailstore/ProvidedTempFileBody.java @@ -9,27 +9,33 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import android.net.Uri; import android.support.annotation.Nullable; import android.util.Log; import com.fsck.k9.K9; import com.fsck.k9.mail.MessagingException; import com.fsck.k9.mail.internet.SizeAware; -import org.apache.commons.io.output.DeferredFileOutputStream; +import com.fsck.k9.service.FileProviderDeferredFileOutputStream; +import com.fsck.k9.service.FileProviderInterface; + +/** This is a body where the body can be accessed through a FileProvider. + * @see FileProviderInterface + */ public class ProvidedTempFileBody extends BinaryAttachmentBody implements SizeAware { public static final int MEMORY_BACKED_THRESHOLD = 1024 * 8; - private final File tempDirectory; - @Nullable - private File file; + private final FileProviderInterface fileProviderInterface; + @Nullable private byte[] data; + private File file; - public ProvidedTempFileBody(File tempDirectory, String transferEncoding) { - this.tempDirectory = tempDirectory; + public ProvidedTempFileBody(FileProviderInterface fileProviderInterface, String transferEncoding) { + this.fileProviderInterface = fileProviderInterface; try { setEncoding(transferEncoding); } catch (MessagingException e) { @@ -38,7 +44,7 @@ public class ProvidedTempFileBody extends BinaryAttachmentBody implements SizeAw } public OutputStream getOutputStream() throws IOException { - return new DeferredFileOutputStream(MEMORY_BACKED_THRESHOLD, "decrypted", null, tempDirectory) { + return new FileProviderDeferredFileOutputStream(MEMORY_BACKED_THRESHOLD, fileProviderInterface) { @Override public void close() throws IOException { super.close(); @@ -82,11 +88,11 @@ public class ProvidedTempFileBody extends BinaryAttachmentBody implements SizeAw throw new IllegalStateException("Data must be fully written before it can be read!"); } - public File getFile() throws IOException { + public Uri getProviderUri(String mimeType) throws IOException { if (file == null) { writeMemoryToFile(); } - return file; + return fileProviderInterface.getUriForProvidedFile(file, mimeType); } private void writeMemoryToFile() throws IOException { @@ -99,8 +105,6 @@ public class ProvidedTempFileBody extends BinaryAttachmentBody implements SizeAw Log.d(K9.LOG_TAG, "Writing body to file for attachment access"); - file = File.createTempFile("decrypted", null, tempDirectory); - FileOutputStream fos = new FileOutputStream(file); fos.write(data); fos.close(); diff --git a/k9mail/src/main/java/com/fsck/k9/message/extractors/AttachmentInfoExtractor.java b/k9mail/src/main/java/com/fsck/k9/message/extractors/AttachmentInfoExtractor.java index cc4cc22e8..4bea11f5b 100644 --- a/k9mail/src/main/java/com/fsck/k9/message/extractors/AttachmentInfoExtractor.java +++ b/k9mail/src/main/java/com/fsck/k9/message/extractors/AttachmentInfoExtractor.java @@ -6,34 +6,34 @@ import java.io.IOException; import java.util.ArrayList; import java.util.List; -import android.content.Context; import android.net.Uri; +import android.util.Log; +import com.fsck.k9.K9; import com.fsck.k9.mail.Body; import com.fsck.k9.mail.MessagingException; import com.fsck.k9.mail.Part; import com.fsck.k9.mail.internet.MimeHeader; import com.fsck.k9.mail.internet.MimeUtility; import com.fsck.k9.mailstore.AttachmentViewInfo; -import com.fsck.k9.mailstore.ProvidedTempFileBody; import com.fsck.k9.mailstore.LocalPart; +import com.fsck.k9.mailstore.ProvidedTempFileBody; import com.fsck.k9.provider.AttachmentProvider; -import com.fsck.k9.provider.K9FileProvider; public class AttachmentInfoExtractor { - public static List extractAttachmentInfos(Context context, List attachmentParts) + public static List extractAttachmentInfos(List attachmentParts) throws MessagingException { List attachments = new ArrayList<>(); for (Part part : attachmentParts) { - attachments.add(extractAttachmentInfo(context, part)); + attachments.add(extractAttachmentInfo(part)); } return attachments; } - public static AttachmentViewInfo extractAttachmentInfo(Context context, Part part) throws MessagingException { + public static AttachmentViewInfo extractAttachmentInfo(Part part) throws MessagingException { Uri uri; long size; if (part instanceof LocalPart) { @@ -45,15 +45,15 @@ public class AttachmentInfoExtractor { } else { Body body = part.getBody(); if (body instanceof ProvidedTempFileBody) { - ProvidedTempFileBody providedTempFileBody = (ProvidedTempFileBody) body; + ProvidedTempFileBody decryptedTempFileBody = (ProvidedTempFileBody) body; + size = decryptedTempFileBody.getSize(); try { - File file = providedTempFileBody.getFile(); - uri = K9FileProvider.getUriForFile(context, file, part.getMimeType()); - size = providedTempFileBody.getSize(); - return extractAttachmentInfo(part, uri, size); + uri = decryptedTempFileBody.getProviderUri(part.getMimeType()); } catch (IOException e) { - throw new MessagingException("Error preparing decrypted data as attachment", e); + Log.e(K9.LOG_TAG, "Decrypted temp file (no longer?) exists!", e); + uri = null; } + return extractAttachmentInfo(part, uri, size); } else { throw new UnsupportedOperationException(); } @@ -62,7 +62,7 @@ public class AttachmentInfoExtractor { return extractAttachmentInfo(part, uri, size); } - public static AttachmentViewInfo extractAttachmentInfo(Part part) throws MessagingException { + public static AttachmentViewInfo extractAttachmentInfoForDatabase(Part part) throws MessagingException { return extractAttachmentInfo(part, Uri.EMPTY, AttachmentViewInfo.UNKNOWN_SIZE); } diff --git a/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java b/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java new file mode 100644 index 000000000..15ee88329 --- /dev/null +++ b/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java @@ -0,0 +1,76 @@ +package com.fsck.k9.provider; + + +import java.io.File; +import java.io.IOException; +import java.util.Date; + +import android.content.Context; +import android.net.Uri; +import android.support.v4.content.FileProvider; +import android.util.Log; + +import com.fsck.k9.BuildConfig; +import com.fsck.k9.K9; +import com.fsck.k9.service.FileProviderInterface; + + +public class DecryptedFileProvider extends FileProvider { + private static final String AUTHORITY = BuildConfig.APPLICATION_ID + ".decryptedfileprovider"; + private static final String DECRYPTED_CACHE_DIRECTORY = "decrypted"; + + + public static final long FILE_DELETE_THRESHOLD_MILLISECONDS = 15 * 60 * 1000; + + + @Override + public String getType(Uri uri) { + return uri.getQueryParameter("mime_type"); + } + + public static FileProviderInterface getFileProviderInterface(final Context context) { + return new FileProviderInterface() { + @Override + public File createProvidedFile() throws IOException { + File decryptedTempDirectory = getDecryptedTempDirectory(context); + return File.createTempFile("decrypted-", null, decryptedTempDirectory); + } + + @Override + public Uri getUriForProvidedFile(File file, String mimeType) throws IOException { + Uri uri = FileProvider.getUriForFile(context, AUTHORITY, file); + return uri.buildUpon().appendQueryParameter("mime_type", mimeType).build(); + } + }; + } + + public static boolean deleteOldTemporaryFiles(Context context) { + File tempDirectory = getDecryptedTempDirectory(context); + boolean allFilesDeleted = true; + for (File tempFile : tempDirectory.listFiles()) { + if (tempFile.lastModified() < new Date().getTime() - FILE_DELETE_THRESHOLD_MILLISECONDS) { + boolean fileDeleted = tempFile.delete(); + if (!fileDeleted) { + Log.e(K9.LOG_TAG, "Failed to delete temporary file"); + // TODO really do this? might cause our service to stay up indefinitely if a file can't be deleted + allFilesDeleted = false; + } + } else { + allFilesDeleted = false; + } + } + + return allFilesDeleted; + } + + private static File getDecryptedTempDirectory(Context context) { + File directory = new File(context.getCacheDir(), DECRYPTED_CACHE_DIRECTORY); + if (!directory.exists()) { + if (!directory.mkdir()) { + Log.e(K9.LOG_TAG, "Error creating directory: " + directory.getAbsolutePath()); + } + } + + return directory; + } +} diff --git a/k9mail/src/main/java/com/fsck/k9/provider/K9FileProvider.java b/k9mail/src/main/java/com/fsck/k9/provider/K9FileProvider.java deleted file mode 100644 index 3d032b19d..000000000 --- a/k9mail/src/main/java/com/fsck/k9/provider/K9FileProvider.java +++ /dev/null @@ -1,25 +0,0 @@ -package com.fsck.k9.provider; - - -import java.io.File; - -import android.content.Context; -import android.net.Uri; -import android.support.v4.content.FileProvider; - -import com.fsck.k9.BuildConfig; - - -public class K9FileProvider extends FileProvider { - private static final String AUTHORITY = BuildConfig.APPLICATION_ID + ".fileprovider"; - - public static Uri getUriForFile(Context context, File file, String mimeType) { - Uri uri = FileProvider.getUriForFile(context, AUTHORITY, file); - return uri.buildUpon().appendQueryParameter("mime_type", mimeType).build(); - } - - @Override - public String getType(Uri uri) { - return uri.getQueryParameter("mime_type"); - } -} diff --git a/k9mail/src/main/java/com/fsck/k9/service/FileProviderDeferredFileOutputStream.java b/k9mail/src/main/java/com/fsck/k9/service/FileProviderDeferredFileOutputStream.java new file mode 100644 index 000000000..1701b1b72 --- /dev/null +++ b/k9mail/src/main/java/com/fsck/k9/service/FileProviderDeferredFileOutputStream.java @@ -0,0 +1,63 @@ +package com.fsck.k9.service; + + +import java.io.ByteArrayOutputStream; +import java.io.File; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.OutputStream; + +import org.apache.commons.io.output.DeferredFileOutputStream; +import org.apache.commons.io.output.ThresholdingOutputStream; + + +/** + * This OutputStream is modelled after apache commons' {@link DeferredFileOutputStream}, + * but uses a {@link FileProviderInterface} instead of directly generating temporary files + * itself. + */ +public class FileProviderDeferredFileOutputStream extends ThresholdingOutputStream { + + + private ByteArrayOutputStream memoryOutputStream; + private OutputStream currentOutputStream; + private File outputFile; + private final FileProviderInterface fileProviderInterface; + + + public FileProviderDeferredFileOutputStream(int threshold, FileProviderInterface fileProviderInterface) { + super(threshold); + memoryOutputStream = new ByteArrayOutputStream(); + currentOutputStream = memoryOutputStream; + this.fileProviderInterface = fileProviderInterface; + } + + + @Override + protected OutputStream getStream() throws IOException { + return currentOutputStream; + } + + + @Override + protected void thresholdReached() throws IOException { + outputFile = fileProviderInterface.createProvidedFile(); + FileOutputStream fos = new FileOutputStream(outputFile); + memoryOutputStream.writeTo(fos); + currentOutputStream = fos; + memoryOutputStream = null; + } + + public byte[] getData() { + if (memoryOutputStream != null) + { + return memoryOutputStream.toByteArray(); + } + return null; + } + + public File getFile() { + return outputFile; + } + +} diff --git a/k9mail/src/main/java/com/fsck/k9/service/FileProviderInterface.java b/k9mail/src/main/java/com/fsck/k9/service/FileProviderInterface.java new file mode 100644 index 000000000..02b5faece --- /dev/null +++ b/k9mail/src/main/java/com/fsck/k9/service/FileProviderInterface.java @@ -0,0 +1,15 @@ +package com.fsck.k9.service; + + +import java.io.File; +import java.io.IOException; + +import android.net.Uri; + + +public interface FileProviderInterface { + + File createProvidedFile() throws IOException; + Uri getUriForProvidedFile(File file, String mimeType) throws IOException; + +} diff --git a/k9mail/src/main/java/com/fsck/k9/ui/compose/QuotedMessagePresenter.java b/k9mail/src/main/java/com/fsck/k9/ui/compose/QuotedMessagePresenter.java index 3464d549e..aa715d588 100644 --- a/k9mail/src/main/java/com/fsck/k9/ui/compose/QuotedMessagePresenter.java +++ b/k9mail/src/main/java/com/fsck/k9/ui/compose/QuotedMessagePresenter.java @@ -132,8 +132,7 @@ public class QuotedMessagePresenter { resources, sourceMessage, content, quoteStyle); // Load the message with the reply header. TODO replace with MessageViewInfo data - view.setQuotedHtml(quotedHtmlContent.getQuotedContent(), AttachmentResolver - .createFromPart(messageCompose, sourceMessage)); + view.setQuotedHtml(quotedHtmlContent.getQuotedContent(), AttachmentResolver.createFromPart(sourceMessage)); // TODO: Also strip the signature from the text/plain part view.setQuotedText(QuotedMessageHelper.quoteOriginalTextMessage(resources, sourceMessage, @@ -298,7 +297,7 @@ public class QuotedMessagePresenter { } // TODO replace with MessageViewInfo data view.setQuotedHtml(quotedHtmlContent.getQuotedContent(), - AttachmentResolver.createFromPart(messageCompose, message)); + AttachmentResolver.createFromPart(message)); } } if (bodyPlainOffset != null && bodyPlainLength != null) { diff --git a/k9mail/src/main/java/com/fsck/k9/ui/crypto/MessageCryptoHelper.java b/k9mail/src/main/java/com/fsck/k9/ui/crypto/MessageCryptoHelper.java index 623e4a95c..ffac415eb 100644 --- a/k9mail/src/main/java/com/fsck/k9/ui/crypto/MessageCryptoHelper.java +++ b/k9mail/src/main/java/com/fsck/k9/ui/crypto/MessageCryptoHelper.java @@ -38,6 +38,8 @@ import com.fsck.k9.mailstore.MimePartStreamParser; import com.fsck.k9.mailstore.LocalMessage; import com.fsck.k9.mailstore.MessageHelper; import org.apache.commons.io.IOUtils; +import com.fsck.k9.provider.DecryptedFileProvider; +import com.fsck.k9.service.FileProviderInterface; import org.openintents.openpgp.IOpenPgpService2; import org.openintents.openpgp.OpenPgpDecryptionResult; import org.openintents.openpgp.OpenPgpError; @@ -425,7 +427,9 @@ public class MessageCryptoHelper { @WorkerThread public MimeBodyPart processData(InputStream is) throws IOException { try { - return MimePartStreamParser.parse(context, is); + FileProviderInterface fileProviderInterface = + DecryptedFileProvider.getFileProviderInterface(context); + return MimePartStreamParser.parse(fileProviderInterface, is); } catch (MessagingException e) { Log.e(K9.LOG_TAG, "Something went wrong while parsing the decrypted MIME part", e); //TODO: pass error to main thread and display error message to user diff --git a/k9mail/src/main/res/xml/allowed_file_provider_paths.xml b/k9mail/src/main/res/xml/allowed_file_provider_paths.xml deleted file mode 100644 index 84682749b..000000000 --- a/k9mail/src/main/res/xml/allowed_file_provider_paths.xml +++ /dev/null @@ -1,3 +0,0 @@ - - - diff --git a/k9mail/src/main/res/xml/decrypted_file_provider_paths.xml b/k9mail/src/main/res/xml/decrypted_file_provider_paths.xml new file mode 100644 index 000000000..25ef4ea94 --- /dev/null +++ b/k9mail/src/main/res/xml/decrypted_file_provider_paths.xml @@ -0,0 +1,3 @@ + + + From 1eca7943465300da6205fa624a1fc2925d01d9cd Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 23 Jun 2016 19:24:43 +0200 Subject: [PATCH 03/19] add DecryptedFileProviderCleanupReceiver to clean up temp files on screen off --- .../k9/provider/DecryptedFileProvider.java | 47 +++++++++++++++++-- 1 file changed, 42 insertions(+), 5 deletions(-) diff --git a/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java b/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java index 15ee88329..a60d61438 100644 --- a/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java +++ b/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java @@ -5,8 +5,12 @@ import java.io.File; import java.io.IOException; import java.util.Date; +import android.content.BroadcastReceiver; import android.content.Context; +import android.content.Intent; +import android.content.IntentFilter; import android.net.Uri; +import android.support.annotation.MainThread; import android.support.v4.content.FileProvider; import android.util.Log; @@ -18,9 +22,10 @@ import com.fsck.k9.service.FileProviderInterface; public class DecryptedFileProvider extends FileProvider { private static final String AUTHORITY = BuildConfig.APPLICATION_ID + ".decryptedfileprovider"; private static final String DECRYPTED_CACHE_DIRECTORY = "decrypted"; + private static final long FILE_DELETE_THRESHOLD_MILLISECONDS = 3 * 60 * 1000; - public static final long FILE_DELETE_THRESHOLD_MILLISECONDS = 15 * 60 * 1000; + private static boolean receiverRegistered = false; @Override @@ -28,17 +33,20 @@ public class DecryptedFileProvider extends FileProvider { return uri.getQueryParameter("mime_type"); } - public static FileProviderInterface getFileProviderInterface(final Context context) { + public static FileProviderInterface getFileProviderInterface(Context context) { + final Context applicationContext = context.getApplicationContext(); + return new FileProviderInterface() { @Override public File createProvidedFile() throws IOException { - File decryptedTempDirectory = getDecryptedTempDirectory(context); + registerFileCleanupReceiver(applicationContext); + File decryptedTempDirectory = getDecryptedTempDirectory(applicationContext); return File.createTempFile("decrypted-", null, decryptedTempDirectory); } @Override public Uri getUriForProvidedFile(File file, String mimeType) throws IOException { - Uri uri = FileProvider.getUriForFile(context, AUTHORITY, file); + Uri uri = FileProvider.getUriForFile(applicationContext, AUTHORITY, file); return uri.buildUpon().appendQueryParameter("mime_type", mimeType).build(); } }; @@ -47,8 +55,10 @@ public class DecryptedFileProvider extends FileProvider { public static boolean deleteOldTemporaryFiles(Context context) { File tempDirectory = getDecryptedTempDirectory(context); boolean allFilesDeleted = true; + long deletionThreshold = new Date().getTime() - FILE_DELETE_THRESHOLD_MILLISECONDS; for (File tempFile : tempDirectory.listFiles()) { - if (tempFile.lastModified() < new Date().getTime() - FILE_DELETE_THRESHOLD_MILLISECONDS) { + long lastModified = tempFile.lastModified(); + if (lastModified < deletionThreshold) { boolean fileDeleted = tempFile.delete(); if (!fileDeleted) { Log.e(K9.LOG_TAG, "Failed to delete temporary file"); @@ -73,4 +83,31 @@ public class DecryptedFileProvider extends FileProvider { return directory; } + + @MainThread // no need to synchronize for receiverRegistered + private static void registerFileCleanupReceiver(Context context) { + if (receiverRegistered) { + return; + } + receiverRegistered = true; + IntentFilter intentFilter = new IntentFilter(); + intentFilter.addAction(Intent.ACTION_SCREEN_OFF); + context.registerReceiver(new DecryptedFileProviderCleanupReceiver(), intentFilter); + } + + private static class DecryptedFileProviderCleanupReceiver extends BroadcastReceiver { + @Override + @MainThread + public void onReceive(Context context, Intent intent) { + if (!Intent.ACTION_SCREEN_OFF.equals(intent.getAction())) { + throw new IllegalArgumentException("onReceive called with action that isn't screen off!"); + } + + boolean allFilesDeleted = deleteOldTemporaryFiles(context); + if (allFilesDeleted) { + context.unregisterReceiver(this); + receiverRegistered = false; + } + } + } } From be3543c78dc490bd1da801cdb43f832e26ae3a03 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 23 Jun 2016 23:21:17 +0200 Subject: [PATCH 04/19] also clean up decrypted temp files on low memory --- .../k9/provider/DecryptedFileProvider.java | 31 ++++++++++++++----- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java b/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java index a60d61438..417b39686 100644 --- a/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java +++ b/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java @@ -25,7 +25,7 @@ public class DecryptedFileProvider extends FileProvider { private static final long FILE_DELETE_THRESHOLD_MILLISECONDS = 3 * 60 * 1000; - private static boolean receiverRegistered = false; + private static DecryptedFileProviderCleanupReceiver receiverRegistered = null; @Override @@ -84,15 +84,32 @@ public class DecryptedFileProvider extends FileProvider { return directory; } - @MainThread // no need to synchronize for receiverRegistered - private static void registerFileCleanupReceiver(Context context) { - if (receiverRegistered) { + @Override + public void onTrimMemory(int level) { + if (level < TRIM_MEMORY_COMPLETE) { return; } - receiverRegistered = true; + Context context = getContext(); + if (context == null) { + return; + } + + deleteOldTemporaryFiles(context); + if (receiverRegistered != null) { + context.unregisterReceiver(receiverRegistered); + receiverRegistered = null; + } + } + + @MainThread // no need to synchronize for receiverRegistered + private static void registerFileCleanupReceiver(Context context) { + if (receiverRegistered != null) { + return; + } + receiverRegistered = new DecryptedFileProviderCleanupReceiver(); IntentFilter intentFilter = new IntentFilter(); intentFilter.addAction(Intent.ACTION_SCREEN_OFF); - context.registerReceiver(new DecryptedFileProviderCleanupReceiver(), intentFilter); + context.registerReceiver(receiverRegistered, intentFilter); } private static class DecryptedFileProviderCleanupReceiver extends BroadcastReceiver { @@ -106,7 +123,7 @@ public class DecryptedFileProvider extends FileProvider { boolean allFilesDeleted = deleteOldTemporaryFiles(context); if (allFilesDeleted) { context.unregisterReceiver(this); - receiverRegistered = false; + receiverRegistered = null; } } } From bb9f857d86683e6fef27c6b1d9b22c9f72f6c530 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 23 Jun 2016 22:54:33 +0200 Subject: [PATCH 05/19] add Attachment* tests, and some annotations --- .../main/java/com/fsck/k9/mail/Message.java | 2 + .../src/main/java/com/fsck/k9/mail/Part.java | 4 + .../fsck/k9/mail/internet/MimeBodyPart.java | 3 + .../com/fsck/k9/mail/internet/MimeHeader.java | 5 + .../fsck/k9/mail/internet/MimeMessage.java | 3 + .../fsck/k9/mail/internet/MimeUtility.java | 4 +- .../fsck/k9/mailstore/AttachmentResolver.java | 10 +- .../com/fsck/k9/mailstore/LocalFolder.java | 5 +- .../com/fsck/k9/mailstore/LocalMessage.java | 5 +- .../mailstore/MessageViewInfoExtractor.java | 10 +- .../extractors/AttachmentInfoExtractor.java | 24 ++- .../k9/mailstore/AttachmentResolverTest.java | 105 ++++++++++ .../AttachmentInfoExtractorTest.java | 181 ++++++++++++++++++ 13 files changed, 343 insertions(+), 18 deletions(-) create mode 100644 k9mail/src/test/java/com/fsck/k9/mailstore/AttachmentResolverTest.java create mode 100644 k9mail/src/test/java/com/fsck/k9/message/extractors/AttachmentInfoExtractorTest.java diff --git a/k9mail-library/src/main/java/com/fsck/k9/mail/Message.java b/k9mail-library/src/main/java/com/fsck/k9/mail/Message.java index 1b51b8a1e..6def28737 100644 --- a/k9mail-library/src/main/java/com/fsck/k9/mail/Message.java +++ b/k9mail-library/src/main/java/com/fsck/k9/mail/Message.java @@ -7,6 +7,7 @@ import java.util.Date; import java.util.EnumSet; import java.util.Set; +import android.support.annotation.NonNull; import android.util.Log; import com.fsck.k9.mail.filter.CountingOutputStream; @@ -129,6 +130,7 @@ public abstract class Message implements Part, CompositeBody { @Override public abstract void setHeader(String name, String value) throws MessagingException; + @NonNull @Override public abstract String[] getHeader(String name) throws MessagingException; diff --git a/k9mail-library/src/main/java/com/fsck/k9/mail/Part.java b/k9mail-library/src/main/java/com/fsck/k9/mail/Part.java index ebfa85750..9197e0d3d 100644 --- a/k9mail-library/src/main/java/com/fsck/k9/mail/Part.java +++ b/k9mail-library/src/main/java/com/fsck/k9/mail/Part.java @@ -5,6 +5,9 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import android.support.annotation.NonNull; + + public interface Part { void addHeader(String name, String value) throws MessagingException; @@ -25,6 +28,7 @@ public interface Part { /** * Returns an array of headers of the given name. The array may be empty. */ + @NonNull String[] getHeader(String name) throws MessagingException; boolean isMimeType(String mimeType); diff --git a/k9mail-library/src/main/java/com/fsck/k9/mail/internet/MimeBodyPart.java b/k9mail-library/src/main/java/com/fsck/k9/mail/internet/MimeBodyPart.java index 813587825..1235da1d0 100644 --- a/k9mail-library/src/main/java/com/fsck/k9/mail/internet/MimeBodyPart.java +++ b/k9mail-library/src/main/java/com/fsck/k9/mail/internet/MimeBodyPart.java @@ -11,6 +11,8 @@ import java.io.IOException; import java.io.OutputStream; import java.io.OutputStreamWriter; +import android.support.annotation.NonNull; + import org.apache.james.mime4j.util.MimeUtil; /** @@ -61,6 +63,7 @@ public class MimeBodyPart extends BodyPart { mHeader.setHeader(name, value); } + @NonNull @Override public String[] getHeader(String name) throws MessagingException { return mHeader.getHeader(name); diff --git a/k9mail-library/src/main/java/com/fsck/k9/mail/internet/MimeHeader.java b/k9mail-library/src/main/java/com/fsck/k9/mail/internet/MimeHeader.java index c04718b3a..eb902d9bb 100644 --- a/k9mail-library/src/main/java/com/fsck/k9/mail/internet/MimeHeader.java +++ b/k9mail-library/src/main/java/com/fsck/k9/mail/internet/MimeHeader.java @@ -8,6 +8,9 @@ import java.io.OutputStreamWriter; import java.nio.charset.Charset; import java.util.*; +import android.support.annotation.NonNull; + + public class MimeHeader implements Cloneable { public static final String HEADER_CONTENT_TYPE = "Content-Type"; public static final String HEADER_CONTENT_TRANSFER_ENCODING = "Content-Transfer-Encoding"; @@ -47,6 +50,7 @@ public class MimeHeader implements Cloneable { addHeader(name, value); } + @NonNull public Set getHeaderNames() { Set names = new LinkedHashSet(); for (Field field : mFields) { @@ -55,6 +59,7 @@ public class MimeHeader implements Cloneable { return names; } + @NonNull public String[] getHeader(String name) { List values = new ArrayList(); for (Field field : mFields) { diff --git a/k9mail-library/src/main/java/com/fsck/k9/mail/internet/MimeMessage.java b/k9mail-library/src/main/java/com/fsck/k9/mail/internet/MimeMessage.java index ff6c263fa..0bef61bb0 100644 --- a/k9mail-library/src/main/java/com/fsck/k9/mail/internet/MimeMessage.java +++ b/k9mail-library/src/main/java/com/fsck/k9/mail/internet/MimeMessage.java @@ -15,6 +15,8 @@ import java.util.Locale; import java.util.Set; import java.util.UUID; +import android.support.annotation.NonNull; + import org.apache.commons.io.IOUtils; import org.apache.james.mime4j.MimeException; import org.apache.james.mime4j.dom.field.DateTimeField; @@ -422,6 +424,7 @@ public class MimeMessage extends Message { mHeader.setHeader(name, value); } + @NonNull @Override public String[] getHeader(String name) throws MessagingException { return mHeader.getHeader(name); diff --git a/k9mail-library/src/main/java/com/fsck/k9/mail/internet/MimeUtility.java b/k9mail-library/src/main/java/com/fsck/k9/mail/internet/MimeUtility.java index 5314810de..f74623d7f 100644 --- a/k9mail-library/src/main/java/com/fsck/k9/mail/internet/MimeUtility.java +++ b/k9mail-library/src/main/java/com/fsck/k9/mail/internet/MimeUtility.java @@ -20,6 +20,8 @@ import java.io.OutputStream; import java.util.Locale; import java.util.regex.Pattern; +import android.support.annotation.NonNull; + public class MimeUtility { public static final String DEFAULT_ATTACHMENT_MIME_TYPE = "application/octet-stream"; @@ -1082,7 +1084,7 @@ public class MimeUtility { return DEFAULT_ATTACHMENT_MIME_TYPE; } - public static String getExtensionByMimeType(String mimeType) { + public static String getExtensionByMimeType(@NonNull String mimeType) { String lowerCaseMimeType = mimeType.toLowerCase(Locale.US); for (String[] contentTypeMapEntry : MIME_TYPE_BY_EXTENSION_MAP) { if (contentTypeMapEntry[1].equals(lowerCaseMimeType)) { diff --git a/k9mail/src/main/java/com/fsck/k9/mailstore/AttachmentResolver.java b/k9mail/src/main/java/com/fsck/k9/mailstore/AttachmentResolver.java index 07f084165..5742a1a78 100644 --- a/k9mail/src/main/java/com/fsck/k9/mailstore/AttachmentResolver.java +++ b/k9mail/src/main/java/com/fsck/k9/mailstore/AttachmentResolver.java @@ -8,6 +8,7 @@ import java.util.Stack; import android.net.Uri; import android.support.annotation.Nullable; +import android.support.annotation.VisibleForTesting; import android.support.annotation.WorkerThread; import android.util.Log; @@ -41,11 +42,14 @@ public class AttachmentResolver { @WorkerThread public static AttachmentResolver createFromPart(Part part) { - Map contentIdToAttachmentUriMap = buildCidToAttachmentUriMap(part); + AttachmentInfoExtractor attachmentInfoExtractor = AttachmentInfoExtractor.getInstance(); + Map contentIdToAttachmentUriMap = buildCidToAttachmentUriMap(attachmentInfoExtractor, part); return new AttachmentResolver(contentIdToAttachmentUriMap); } - private static Map buildCidToAttachmentUriMap(Part rootPart) { + @VisibleForTesting + static Map buildCidToAttachmentUriMap(AttachmentInfoExtractor attachmentInfoExtractor, + Part rootPart) { HashMap result = new HashMap<>(); Stack partsToCheck = new Stack<>(); @@ -64,7 +68,7 @@ public class AttachmentResolver { try { String contentId = part.getContentId(); if (contentId != null) { - AttachmentViewInfo attachmentInfo = AttachmentInfoExtractor.extractAttachmentInfo(part); + AttachmentViewInfo attachmentInfo = attachmentInfoExtractor.extractAttachmentInfo(part); result.put(contentId, attachmentInfo.uri); } } catch (MessagingException e) { diff --git a/k9mail/src/main/java/com/fsck/k9/mailstore/LocalFolder.java b/k9mail/src/main/java/com/fsck/k9/mailstore/LocalFolder.java index 974cb9dcd..37ba96e21 100644 --- a/k9mail/src/main/java/com/fsck/k9/mailstore/LocalFolder.java +++ b/k9mail/src/main/java/com/fsck/k9/mailstore/LocalFolder.java @@ -68,6 +68,7 @@ public class LocalFolder extends Folder implements Serializable { private static final long serialVersionUID = -1973296520918624767L; private static final int MAX_BODY_SIZE_FOR_DATABASE = 16 * 1024; + private static final AttachmentInfoExtractor attachmentInfoExtractor = AttachmentInfoExtractor.getInstance(); static final long INVALID_MESSAGE_PART_ID = -1; private final LocalStore localStore; @@ -1401,7 +1402,7 @@ public class LocalFolder extends Folder implements Serializable { } private void missingPartToContentValues(ContentValues cv, Part part) throws MessagingException { - AttachmentViewInfo attachment = AttachmentInfoExtractor.extractAttachmentInfoForDatabase(part); + AttachmentViewInfo attachment = attachmentInfoExtractor.extractAttachmentInfoForDatabase(part); cv.put("display_name", attachment.displayName); cv.put("data_location", DataLocation.MISSING); cv.put("decoded_body_size", attachment.size); @@ -1417,7 +1418,7 @@ public class LocalFolder extends Folder implements Serializable { private File leafPartToContentValues(ContentValues cv, Part part, Body body) throws MessagingException, IOException { - AttachmentViewInfo attachment = AttachmentInfoExtractor.extractAttachmentInfoForDatabase(part); + AttachmentViewInfo attachment = attachmentInfoExtractor.extractAttachmentInfoForDatabase(part); cv.put("display_name", attachment.displayName); String encoding = getTransferEncoding(part); diff --git a/k9mail/src/main/java/com/fsck/k9/mailstore/LocalMessage.java b/k9mail/src/main/java/com/fsck/k9/mailstore/LocalMessage.java index a789ece20..001c45f76 100644 --- a/k9mail/src/main/java/com/fsck/k9/mailstore/LocalMessage.java +++ b/k9mail/src/main/java/com/fsck/k9/mailstore/LocalMessage.java @@ -9,6 +9,7 @@ import java.util.Set; import android.content.ContentValues; import android.database.Cursor; import android.database.sqlite.SQLiteDatabase; +import android.support.annotation.NonNull; import android.util.Log; import com.fsck.k9.Account; @@ -517,10 +518,12 @@ public class LocalMessage extends MimeMessage { super.setHeader(name, value); } + @NonNull @Override public String[] getHeader(String name) throws MessagingException { - if (!mHeadersLoaded) + if (!mHeadersLoaded) { loadHeaders(); + } return super.getHeader(name); } diff --git a/k9mail/src/main/java/com/fsck/k9/mailstore/MessageViewInfoExtractor.java b/k9mail/src/main/java/com/fsck/k9/mailstore/MessageViewInfoExtractor.java index aee469e60..afe32cba1 100644 --- a/k9mail/src/main/java/com/fsck/k9/mailstore/MessageViewInfoExtractor.java +++ b/k9mail/src/main/java/com/fsck/k9/mailstore/MessageViewInfoExtractor.java @@ -10,7 +10,6 @@ import android.content.Context; import android.support.annotation.VisibleForTesting; import com.fsck.k9.R; -import com.fsck.k9.ui.crypto.MessageCryptoSplitter; import com.fsck.k9.helper.HtmlConverter; import com.fsck.k9.mail.Address; import com.fsck.k9.mail.Message; @@ -20,6 +19,7 @@ import com.fsck.k9.mail.internet.MessageExtractor; import com.fsck.k9.mail.internet.Viewable; import com.fsck.k9.message.extractors.AttachmentInfoExtractor; import com.fsck.k9.ui.crypto.MessageCryptoAnnotations; +import com.fsck.k9.ui.crypto.MessageCryptoSplitter; import com.fsck.k9.ui.crypto.MessageCryptoSplitter.CryptoMessageParts; import static com.fsck.k9.mail.internet.MimeUtility.getHeaderParameter; @@ -38,7 +38,11 @@ public class MessageViewInfoExtractor { private static final String FILENAME_SUFFIX = " "; private static final int FILENAME_SUFFIX_LENGTH = FILENAME_SUFFIX.length(); - private MessageViewInfoExtractor() {} + + private static final AttachmentInfoExtractor attachmentInfoExtractor = AttachmentInfoExtractor.getInstance(); + + + private MessageViewInfoExtractor() { } public static MessageViewInfo extractMessageForView(Context context, Message message, MessageCryptoAnnotations annotations) throws MessagingException { @@ -85,7 +89,7 @@ public class MessageViewInfoExtractor { MessageExtractor.findViewablesAndAttachments(part, viewableParts, attachments); } - attachmentInfos.addAll(AttachmentInfoExtractor.extractAttachmentInfos(attachments)); + attachmentInfos.addAll(attachmentInfoExtractor.extractAttachmentInfos(attachments)); return MessageViewInfoExtractor.extractTextFromViewables(context, viewableParts); } diff --git a/k9mail/src/main/java/com/fsck/k9/message/extractors/AttachmentInfoExtractor.java b/k9mail/src/main/java/com/fsck/k9/message/extractors/AttachmentInfoExtractor.java index 4bea11f5b..577401fbf 100644 --- a/k9mail/src/main/java/com/fsck/k9/message/extractors/AttachmentInfoExtractor.java +++ b/k9mail/src/main/java/com/fsck/k9/message/extractors/AttachmentInfoExtractor.java @@ -1,7 +1,6 @@ package com.fsck.k9.message.extractors; -import java.io.File; import java.io.IOException; import java.util.ArrayList; import java.util.List; @@ -22,7 +21,13 @@ import com.fsck.k9.provider.AttachmentProvider; public class AttachmentInfoExtractor { - public static List extractAttachmentInfos(List attachmentParts) + private AttachmentInfoExtractor() { } + + public static AttachmentInfoExtractor getInstance() { + return new AttachmentInfoExtractor(); + } + + public List extractAttachmentInfos(List attachmentParts) throws MessagingException { List attachments = new ArrayList<>(); @@ -33,7 +38,7 @@ public class AttachmentInfoExtractor { return attachments; } - public static AttachmentViewInfo extractAttachmentInfo(Part part) throws MessagingException { + public AttachmentViewInfo extractAttachmentInfo(Part part) throws MessagingException { Uri uri; long size; if (part instanceof LocalPart) { @@ -55,18 +60,18 @@ public class AttachmentInfoExtractor { } return extractAttachmentInfo(part, uri, size); } else { - throw new UnsupportedOperationException(); + throw new IllegalArgumentException("Unsupported part type provided"); } } return extractAttachmentInfo(part, uri, size); } - public static AttachmentViewInfo extractAttachmentInfoForDatabase(Part part) throws MessagingException { + public AttachmentViewInfo extractAttachmentInfoForDatabase(Part part) throws MessagingException { return extractAttachmentInfo(part, Uri.EMPTY, AttachmentViewInfo.UNKNOWN_SIZE); } - private static AttachmentViewInfo extractAttachmentInfo(Part part, Uri uri, long size) throws MessagingException { + private AttachmentViewInfo extractAttachmentInfo(Part part, Uri uri, long size) throws MessagingException { boolean firstClassAttachment = true; String mimeType = part.getMimeType(); @@ -80,7 +85,10 @@ public class AttachmentInfoExtractor { if (name == null) { firstClassAttachment = false; - String extension = MimeUtility.getExtensionByMimeType(mimeType); + String extension = null; + if (mimeType != null) { + extension = MimeUtility.getExtensionByMimeType(mimeType); + } name = "noname" + ((extension != null) ? "." + extension : ""); } @@ -98,7 +106,7 @@ public class AttachmentInfoExtractor { return new AttachmentViewInfo(mimeType, name, attachmentSize, uri, firstClassAttachment, part); } - private static long extractAttachmentSize(String contentDisposition, long size) { + private long extractAttachmentSize(String contentDisposition, long size) { if (size != AttachmentViewInfo.UNKNOWN_SIZE) { return size; } diff --git a/k9mail/src/test/java/com/fsck/k9/mailstore/AttachmentResolverTest.java b/k9mail/src/test/java/com/fsck/k9/mailstore/AttachmentResolverTest.java new file mode 100644 index 000000000..fb67855db --- /dev/null +++ b/k9mail/src/test/java/com/fsck/k9/mailstore/AttachmentResolverTest.java @@ -0,0 +1,105 @@ +package com.fsck.k9.mailstore; + + +import java.util.Arrays; +import java.util.Collections; +import java.util.Map; + +import android.net.Uri; + +import com.fsck.k9.mail.BodyPart; +import com.fsck.k9.mail.Multipart; +import com.fsck.k9.mail.Part; +import com.fsck.k9.message.extractors.AttachmentInfoExtractor; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.annotation.Config; + +import static org.junit.Assert.*; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + + +@SuppressWarnings("unchecked") +@RunWith(RobolectricTestRunner.class) +@Config(manifest = Config.NONE, sdk = 21) +public class AttachmentResolverTest { + public static final Uri ATTACHMENT_TEST_URI_1 = Uri.parse("uri://test/1"); + public static final Uri ATTACHMENT_TEST_URI_2 = Uri.parse("uri://test/2"); + + + @Test + public void buildCidMap__onPartWithNoBody__shouldReturnEmptyMap() throws Exception { + AttachmentInfoExtractor attachmentInfoExtractor = mock(AttachmentInfoExtractor.class); + Part part = mock(Part.class); + when(part.getContentId()).thenReturn(null); + + Map result = AttachmentResolver.buildCidToAttachmentUriMap(attachmentInfoExtractor, part); + + assertTrue(result.isEmpty()); + } + + @Test + public void buildCidMap__onMultipartWithNoParts__shouldReturnEmptyMap() throws Exception { + AttachmentInfoExtractor attachmentInfoExtractor = mock(AttachmentInfoExtractor.class); + Part multipartPart = mock(Part.class); + Multipart multipartBody = mock(Multipart.class); + when(multipartPart.getBody()).thenReturn(multipartBody); + when(multipartBody.getBodyParts()).thenReturn(Collections.EMPTY_LIST); + + Map result = AttachmentResolver.buildCidToAttachmentUriMap(attachmentInfoExtractor, multipartPart); + + assertTrue(result.isEmpty()); + } + + @Test + public void buildCidMap__onMultipartWithEmptyBodyPart__shouldReturnEmptyMap() throws Exception { + AttachmentInfoExtractor attachmentInfoExtractor = mock(AttachmentInfoExtractor.class); + Part multipartPart = mock(Part.class); + Multipart multipartBody = mock(Multipart.class); + BodyPart bodyPart = mock(BodyPart.class); + + when(multipartPart.getBody()).thenReturn(multipartBody); + when(multipartBody.getBodyParts()).thenReturn(Collections.singletonList(bodyPart)); + when(bodyPart.getContentId()).thenReturn(null); + + + Map result = AttachmentResolver.buildCidToAttachmentUriMap(attachmentInfoExtractor, multipartPart); + + + verify(bodyPart).getContentId(); + assertTrue(result.isEmpty()); + } + + @Test + public void buildCidMap__onTwoPart__shouldReturnBothUris() throws Exception { + AttachmentInfoExtractor attachmentInfoExtractor = mock(AttachmentInfoExtractor.class); + Part multipartPart = mock(Part.class); + Multipart multipartBody = mock(Multipart.class); + BodyPart bodyPart = mock(BodyPart.class); + + when(multipartPart.getBody()).thenReturn(multipartBody); + when(multipartBody.getBodyParts()).thenReturn(Collections.singletonList(bodyPart)); + + BodyPart subPart1 = mock(BodyPart.class); + BodyPart subPart2 = mock(BodyPart.class); + when(subPart1.getContentId()).thenReturn("cid-1"); + when(subPart2.getContentId()).thenReturn("cid-2"); + + when(attachmentInfoExtractor.extractAttachmentInfo(subPart1)).thenReturn(new AttachmentViewInfo( + null, null, AttachmentViewInfo.UNKNOWN_SIZE, ATTACHMENT_TEST_URI_1, true, subPart1)); + when(attachmentInfoExtractor.extractAttachmentInfo(subPart2)).thenReturn(new AttachmentViewInfo( + null, null, AttachmentViewInfo.UNKNOWN_SIZE, ATTACHMENT_TEST_URI_2, true, subPart2)); + + when(multipartBody.getBodyParts()).thenReturn(Arrays.asList(subPart1, subPart2)); + + Map result = AttachmentResolver.buildCidToAttachmentUriMap(attachmentInfoExtractor, multipartPart); + + assertEquals(2, result.size()); + assertEquals(ATTACHMENT_TEST_URI_1, result.get("cid-1")); + assertEquals(ATTACHMENT_TEST_URI_2, result.get("cid-2")); + } + +} \ No newline at end of file diff --git a/k9mail/src/test/java/com/fsck/k9/message/extractors/AttachmentInfoExtractorTest.java b/k9mail/src/test/java/com/fsck/k9/message/extractors/AttachmentInfoExtractorTest.java new file mode 100644 index 000000000..694861a8d --- /dev/null +++ b/k9mail/src/test/java/com/fsck/k9/message/extractors/AttachmentInfoExtractorTest.java @@ -0,0 +1,181 @@ +package com.fsck.k9.message.extractors; + + +import android.net.Uri; + +import com.fsck.k9.mail.Part; +import com.fsck.k9.mail.internet.MimeHeader; +import com.fsck.k9.mailstore.AttachmentViewInfo; +import com.fsck.k9.mailstore.LocalBodyPart; +import com.fsck.k9.mailstore.ProvidedTempFileBody; +import com.fsck.k9.provider.AttachmentProvider; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.annotation.Config; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + + +@RunWith(RobolectricTestRunner.class) +@Config(manifest = Config.NONE, sdk = 21) +public class AttachmentInfoExtractorTest { + public static final Uri TEST_URI = Uri.parse("uri://test"); + public static final String TEST_MIME_TYPE = "text/plain"; + public static final long TEST_SIZE = 123L; + public static final String TEST_ACCOUNT_UUID = "uuid"; + public static final long TEST_ID = 234L; + public static final String TEST_DISPLAY_NAME = "test-display-name"; + public static final String[] TEST_CONTENT_ID = new String[] { "test-content-id" }; + + + private AttachmentInfoExtractor attachmentInfoExtractor; + + + @Before + public void setUp() throws Exception { + attachmentInfoExtractor = AttachmentInfoExtractor.getInstance(); + } + + @Test(expected = IllegalArgumentException.class) + public void extractInfo__withGenericPart_shouldThrow() throws Exception { + Part part = mock(Part.class); + + attachmentInfoExtractor.extractAttachmentInfo(part); + } + + @Test + public void extractInfo__fromLocalBodyPart__shouldReturnProvidedValues() throws Exception { + LocalBodyPart part = mock(LocalBodyPart.class); + when(part.getId()).thenReturn(TEST_ID); + when(part.getMimeType()).thenReturn(TEST_MIME_TYPE); + when(part.getSize()).thenReturn(TEST_SIZE); + when(part.getAccountUuid()).thenReturn(TEST_ACCOUNT_UUID); + when(part.getDisplayName()).thenReturn(TEST_DISPLAY_NAME); + + AttachmentViewInfo attachmentViewInfo = attachmentInfoExtractor.extractAttachmentInfo(part); + + assertEquals(AttachmentProvider.getAttachmentUri(TEST_ACCOUNT_UUID, TEST_ID), attachmentViewInfo.uri); + assertEquals(true, attachmentViewInfo.firstClassAttachment); + assertEquals(TEST_SIZE, attachmentViewInfo.size); + assertEquals(TEST_DISPLAY_NAME, attachmentViewInfo.displayName); + assertEquals(TEST_MIME_TYPE, attachmentViewInfo.mimeType); + } + + @Test + public void extractInfoForDb__withNoHeaders__shouldReturnEmptyValues() throws Exception { + Part part = mock(Part.class); + + AttachmentViewInfo attachmentViewInfo = attachmentInfoExtractor.extractAttachmentInfoForDatabase(part); + + assertEquals(Uri.EMPTY, attachmentViewInfo.uri); + assertEquals(AttachmentViewInfo.UNKNOWN_SIZE, attachmentViewInfo.size); + assertEquals("noname", attachmentViewInfo.displayName); + assertNull(attachmentViewInfo.mimeType); + assertFalse(attachmentViewInfo.firstClassAttachment); + } + + @Test + public void extractInfoForDb__withTextMimeType__shouldReturnTxtExtension() throws Exception { + Part part = mock(Part.class); + when(part.getMimeType()).thenReturn("text/plain"); + + AttachmentViewInfo attachmentViewInfo = attachmentInfoExtractor.extractAttachmentInfoForDatabase(part); + + // MimeUtility.getExtensionByMimeType("text/plain"); -> "txt" + assertEquals("noname.txt", attachmentViewInfo.displayName); + assertEquals("text/plain", attachmentViewInfo.mimeType); + } + + @Test + public void extractInfoForDb__withContentTypeAndName__shouldReturnNamedFirstClassAttachment() throws Exception { + Part part = mock(Part.class); + when(part.getMimeType()).thenReturn(TEST_MIME_TYPE); + when(part.getContentType()).thenReturn(TEST_MIME_TYPE + "; name=\"filename.ext\""); + + AttachmentViewInfo attachmentViewInfo = attachmentInfoExtractor.extractAttachmentInfoForDatabase(part); + + assertEquals(Uri.EMPTY, attachmentViewInfo.uri); + assertEquals(TEST_MIME_TYPE, attachmentViewInfo.mimeType); + assertEquals("filename.ext", attachmentViewInfo.displayName); + assertTrue(attachmentViewInfo.firstClassAttachment); + } + + @Test + public void extractInfoForDb__withContentTypeAndEncodedWordName__shouldReturnDecodedName() throws Exception { + Part part = mock(Part.class); + when(part.getContentType()).thenReturn(TEST_MIME_TYPE + "; name=\"=?ISO-8859-1?Q?Sm=F8rrebr=F8d?=\""); + + AttachmentViewInfo attachmentViewInfo = attachmentInfoExtractor.extractAttachmentInfoForDatabase(part); + + assertEquals("Smørrebrød", attachmentViewInfo.displayName); + } + + @Test + public void extractInfoForDb__wWithDispositionAttach__shouldReturnNamedFirstClassAttachment() throws Exception { + Part part = mock(Part.class); + when(part.getDisposition()).thenReturn("attachment" + "; filename=\"filename.ext\"; meaningless=\"dummy\""); + + AttachmentViewInfo attachmentViewInfo = attachmentInfoExtractor.extractAttachmentInfoForDatabase(part); + + assertEquals(Uri.EMPTY, attachmentViewInfo.uri); + assertEquals("filename.ext", attachmentViewInfo.displayName); + assertTrue(attachmentViewInfo.firstClassAttachment); + } + + @Test + public void extractInfoForDb__withDispositionInlineAndContentId__shouldReturnNotFirstClassAttachment() + throws Exception { + Part part = mock(Part.class); + when(part.getHeader(MimeHeader.HEADER_CONTENT_ID)).thenReturn(TEST_CONTENT_ID); + when(part.getDisposition()).thenReturn("inline" + ";\n filename=\"filename.ext\";\n meaningless=\"dummy\""); + + AttachmentViewInfo attachmentViewInfo = attachmentInfoExtractor.extractAttachmentInfoForDatabase(part); + + assertFalse(attachmentViewInfo.firstClassAttachment); + } + + @Test + public void extractInfoForDb__withDispositionSizeParam__shouldReturnThatSize() throws Exception { + Part part = mock(Part.class); + when(part.getDisposition()).thenReturn("doesntmatter" + ";\n size=\"" + TEST_SIZE + "\""); + + AttachmentViewInfo attachmentViewInfo = attachmentInfoExtractor.extractAttachmentInfoForDatabase(part); + + assertEquals(TEST_SIZE, attachmentViewInfo.size); + } + + @Test + public void extractInfoForDb__withDispositionInvalidSizeParam__shouldReturnUnknownSize() throws Exception { + Part part = mock(Part.class); + when(part.getDisposition()).thenReturn("doesntmatter" + "; size=\"notanint\""); + + AttachmentViewInfo attachmentViewInfo = attachmentInfoExtractor.extractAttachmentInfoForDatabase(part); + + assertEquals(AttachmentViewInfo.UNKNOWN_SIZE, attachmentViewInfo.size); + } + + @Test + public void extractInfo__withProvidedTempFileBody() throws Exception { + ProvidedTempFileBody body = mock(ProvidedTempFileBody.class); + Part part = mock(Part.class); + when(part.getBody()).thenReturn(body); + when(part.getMimeType()).thenReturn(TEST_MIME_TYPE); + + when(body.getSize()).thenReturn(TEST_SIZE); + when(body.getProviderUri(TEST_MIME_TYPE)).thenReturn(TEST_URI); + + AttachmentViewInfo attachmentViewInfo = attachmentInfoExtractor.extractAttachmentInfo(part); + + assertEquals(TEST_URI, attachmentViewInfo.uri); + assertEquals(TEST_SIZE, attachmentViewInfo.size); + assertEquals(TEST_MIME_TYPE, attachmentViewInfo.mimeType); + assertFalse(attachmentViewInfo.firstClassAttachment); + } +} \ No newline at end of file From 032b1fb833403723ebd54cb544da4f6fe95aacca Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Fri, 24 Jun 2016 09:23:24 +0200 Subject: [PATCH 06/19] add some debug logging to cleanup receiver --- .../fsck/k9/provider/DecryptedFileProvider.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java b/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java index 417b39686..17db70c85 100644 --- a/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java +++ b/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java @@ -4,6 +4,7 @@ package com.fsck.k9.provider; import java.io.File; import java.io.IOException; import java.util.Date; +import java.util.Locale; import android.content.BroadcastReceiver; import android.content.Context; @@ -66,6 +67,11 @@ public class DecryptedFileProvider extends FileProvider { allFilesDeleted = false; } } else { + if (K9.DEBUG) { + String timeLeftStr = String.format( + Locale.ENGLISH, "%.2f", (lastModified - deletionThreshold) / 1000 / 60.0); + Log.e(K9.LOG_TAG, "Not deleting temp file (for another " + timeLeftStr + " minutes)"); + } allFilesDeleted = false; } } @@ -106,6 +112,9 @@ public class DecryptedFileProvider extends FileProvider { if (receiverRegistered != null) { return; } + if (K9.DEBUG) { + Log.d(K9.LOG_TAG, "Registering temp file cleanup receiver"); + } receiverRegistered = new DecryptedFileProviderCleanupReceiver(); IntentFilter intentFilter = new IntentFilter(); intentFilter.addAction(Intent.ACTION_SCREEN_OFF); @@ -120,8 +129,15 @@ public class DecryptedFileProvider extends FileProvider { throw new IllegalArgumentException("onReceive called with action that isn't screen off!"); } + if (K9.DEBUG) { + Log.d(K9.LOG_TAG, "Cleaning up temp files"); + } + boolean allFilesDeleted = deleteOldTemporaryFiles(context); if (allFilesDeleted) { + if (K9.DEBUG) { + Log.d(K9.LOG_TAG, "Unregistering temp file cleanup receiver"); + } context.unregisterReceiver(this); receiverRegistered = null; } From 7cb6fa102f1e21b7161ccd67c66b7188c9afba56 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Mon, 4 Jul 2016 16:02:59 +0200 Subject: [PATCH 07/19] stream performance optimizations --- .../main/java/com/fsck/k9/mailstore/ProvidedTempFileBody.java | 3 ++- .../main/java/com/fsck/k9/ui/crypto/MessageCryptoHelper.java | 4 ++-- .../openintents/openpgp/util/ParcelFileDescriptorUtil.java | 4 +++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/k9mail/src/main/java/com/fsck/k9/mailstore/ProvidedTempFileBody.java b/k9mail/src/main/java/com/fsck/k9/mailstore/ProvidedTempFileBody.java index a36b3504d..a0e1b9830 100644 --- a/k9mail/src/main/java/com/fsck/k9/mailstore/ProvidedTempFileBody.java +++ b/k9mail/src/main/java/com/fsck/k9/mailstore/ProvidedTempFileBody.java @@ -1,6 +1,7 @@ package com.fsck.k9.mailstore; +import java.io.BufferedInputStream; import java.io.ByteArrayInputStream; import java.io.File; import java.io.FileInputStream; @@ -63,7 +64,7 @@ public class ProvidedTempFileBody extends BinaryAttachmentBody implements SizeAw try { if (file != null) { Log.d(K9.LOG_TAG, "Decrypted data is file-backed."); - return new FileInputStream(file); + return new BufferedInputStream(new FileInputStream(file)); } if (data != null) { Log.d(K9.LOG_TAG, "Decrypted data is memory-backed."); diff --git a/k9mail/src/main/java/com/fsck/k9/ui/crypto/MessageCryptoHelper.java b/k9mail/src/main/java/com/fsck/k9/ui/crypto/MessageCryptoHelper.java index ffac415eb..1542661ee 100644 --- a/k9mail/src/main/java/com/fsck/k9/ui/crypto/MessageCryptoHelper.java +++ b/k9mail/src/main/java/com/fsck/k9/ui/crypto/MessageCryptoHelper.java @@ -34,12 +34,12 @@ import com.fsck.k9.mail.internet.SizeAware; import com.fsck.k9.mail.internet.TextBody; import com.fsck.k9.mailstore.CryptoResultAnnotation; import com.fsck.k9.mailstore.CryptoResultAnnotation.CryptoError; -import com.fsck.k9.mailstore.MimePartStreamParser; import com.fsck.k9.mailstore.LocalMessage; import com.fsck.k9.mailstore.MessageHelper; -import org.apache.commons.io.IOUtils; +import com.fsck.k9.mailstore.MimePartStreamParser; import com.fsck.k9.provider.DecryptedFileProvider; import com.fsck.k9.service.FileProviderInterface; +import org.apache.commons.io.IOUtils; import org.openintents.openpgp.IOpenPgpService2; import org.openintents.openpgp.OpenPgpDecryptionResult; import org.openintents.openpgp.OpenPgpError; diff --git a/plugins/openpgp-api-lib/openpgp-api/src/main/java/org/openintents/openpgp/util/ParcelFileDescriptorUtil.java b/plugins/openpgp-api-lib/openpgp-api/src/main/java/org/openintents/openpgp/util/ParcelFileDescriptorUtil.java index dc0fdab70..911bec60a 100644 --- a/plugins/openpgp-api-lib/openpgp-api/src/main/java/org/openintents/openpgp/util/ParcelFileDescriptorUtil.java +++ b/plugins/openpgp-api-lib/openpgp-api/src/main/java/org/openintents/openpgp/util/ParcelFileDescriptorUtil.java @@ -18,6 +18,7 @@ package org.openintents.openpgp.util; +import java.io.BufferedInputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; @@ -93,8 +94,9 @@ public class ParcelFileDescriptorUtil { public static DataSinkTransferThread asyncPipeToDataSink( OpenPgpDataSink dataSink, ParcelFileDescriptor output) throws IOException { + InputStream inputStream = new BufferedInputStream(new AutoCloseInputStream(output)); DataSinkTransferThread dataSinkTransferThread = - new DataSinkTransferThread(dataSink, new AutoCloseInputStream(output)); + new DataSinkTransferThread(dataSink, inputStream); dataSinkTransferThread.start(); return dataSinkTransferThread; } From 0e3d18e7f706993b67cdf0e53a04553f84a03a66 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Mon, 4 Jul 2016 17:51:23 +0200 Subject: [PATCH 08/19] we don't use displayName from database either (for now), fix tests to reflect that --- .../main/java/com/fsck/k9/mailstore/LocalBodyPart.java | 9 +-------- .../src/main/java/com/fsck/k9/mailstore/LocalFolder.java | 7 ++++--- .../src/main/java/com/fsck/k9/mailstore/LocalPart.java | 1 - .../test/java/com/fsck/k9/mailstore/MigrationTest.java | 1 - .../message/extractors/AttachmentInfoExtractorTest.java | 4 ---- 5 files changed, 5 insertions(+), 17 deletions(-) diff --git a/k9mail/src/main/java/com/fsck/k9/mailstore/LocalBodyPart.java b/k9mail/src/main/java/com/fsck/k9/mailstore/LocalBodyPart.java index d5a14b71a..412c02ce4 100644 --- a/k9mail/src/main/java/com/fsck/k9/mailstore/LocalBodyPart.java +++ b/k9mail/src/main/java/com/fsck/k9/mailstore/LocalBodyPart.java @@ -9,16 +9,14 @@ public class LocalBodyPart extends MimeBodyPart implements LocalPart { private final String accountUuid; private final LocalMessage message; private final long messagePartId; - private final String displayName; private final long size; - public LocalBodyPart(String accountUuid, LocalMessage message, long messagePartId, String displayName, long size) + public LocalBodyPart(String accountUuid, LocalMessage message, long messagePartId, long size) throws MessagingException { super(); this.accountUuid = accountUuid; this.message = message; this.messagePartId = messagePartId; - this.displayName = displayName; this.size = size; } @@ -32,11 +30,6 @@ public class LocalBodyPart extends MimeBodyPart implements LocalPart { return messagePartId; } - @Override - public String getDisplayName() { - return displayName; - } - @Override public long getSize() { return size; diff --git a/k9mail/src/main/java/com/fsck/k9/mailstore/LocalFolder.java b/k9mail/src/main/java/com/fsck/k9/mailstore/LocalFolder.java index 37ba96e21..0ef241eff 100644 --- a/k9mail/src/main/java/com/fsck/k9/mailstore/LocalFolder.java +++ b/k9mail/src/main/java/com/fsck/k9/mailstore/LocalFolder.java @@ -712,11 +712,12 @@ public class LocalFolder extends Folder implements Serializable { long parentId = cursor.getLong(2); String mimeType = cursor.getString(3); long size = cursor.getLong(4); - String displayName = cursor.getString(5); byte[] header = cursor.getBlob(6); int dataLocation = cursor.getInt(9); String serverExtra = cursor.getString(15); - // TODO we don't currently cache the part types. might want to do that at a later point? + // TODO we don't currently cache much of the part data which is computed with AttachmentInfoExtractor, + // TODO might want to do that at a later point? + // String displayName = cursor.getString(5); // int type = cursor.getInt(1); // boolean firstClassAttachment = (type != MessagePartType.HIDDEN_ATTACHMENT); @@ -731,7 +732,7 @@ public class LocalFolder extends Folder implements Serializable { String parentMimeType = parentPart.getMimeType(); if (MimeUtility.isMultipart(parentMimeType)) { - BodyPart bodyPart = new LocalBodyPart(getAccountUuid(), message, id, displayName, size); + BodyPart bodyPart = new LocalBodyPart(getAccountUuid(), message, id, size); ((Multipart) parentPart.getBody()).addBodyPart(bodyPart); part = bodyPart; } else if (MimeUtility.isMessage(parentMimeType)) { diff --git a/k9mail/src/main/java/com/fsck/k9/mailstore/LocalPart.java b/k9mail/src/main/java/com/fsck/k9/mailstore/LocalPart.java index 006173cea..57b0b36de 100644 --- a/k9mail/src/main/java/com/fsck/k9/mailstore/LocalPart.java +++ b/k9mail/src/main/java/com/fsck/k9/mailstore/LocalPart.java @@ -4,7 +4,6 @@ package com.fsck.k9.mailstore; public interface LocalPart { String getAccountUuid(); long getId(); - String getDisplayName(); long getSize(); LocalMessage getMessage(); } diff --git a/k9mail/src/test/java/com/fsck/k9/mailstore/MigrationTest.java b/k9mail/src/test/java/com/fsck/k9/mailstore/MigrationTest.java index 27101ef32..00db71789 100644 --- a/k9mail/src/test/java/com/fsck/k9/mailstore/MigrationTest.java +++ b/k9mail/src/test/java/com/fsck/k9/mailstore/MigrationTest.java @@ -248,7 +248,6 @@ public class MigrationTest { LocalBodyPart attachmentPart = (LocalBodyPart) body.getBodyPart(1); Assert.assertEquals("image/png", attachmentPart.getMimeType()); Assert.assertEquals("2", attachmentPart.getServerExtra()); - Assert.assertEquals("k9small.png", attachmentPart.getDisplayName()); Assert.assertEquals("attachment", MimeUtility.getHeaderParameter(attachmentPart.getDisposition(), null)); Assert.assertEquals("k9small.png", MimeUtility.getHeaderParameter(attachmentPart.getDisposition(), "filename")); Assert.assertEquals("2250", MimeUtility.getHeaderParameter(attachmentPart.getDisposition(), "size")); diff --git a/k9mail/src/test/java/com/fsck/k9/message/extractors/AttachmentInfoExtractorTest.java b/k9mail/src/test/java/com/fsck/k9/message/extractors/AttachmentInfoExtractorTest.java index 694861a8d..0e28bd197 100644 --- a/k9mail/src/test/java/com/fsck/k9/message/extractors/AttachmentInfoExtractorTest.java +++ b/k9mail/src/test/java/com/fsck/k9/message/extractors/AttachmentInfoExtractorTest.java @@ -31,7 +31,6 @@ public class AttachmentInfoExtractorTest { public static final long TEST_SIZE = 123L; public static final String TEST_ACCOUNT_UUID = "uuid"; public static final long TEST_ID = 234L; - public static final String TEST_DISPLAY_NAME = "test-display-name"; public static final String[] TEST_CONTENT_ID = new String[] { "test-content-id" }; @@ -57,14 +56,11 @@ public class AttachmentInfoExtractorTest { when(part.getMimeType()).thenReturn(TEST_MIME_TYPE); when(part.getSize()).thenReturn(TEST_SIZE); when(part.getAccountUuid()).thenReturn(TEST_ACCOUNT_UUID); - when(part.getDisplayName()).thenReturn(TEST_DISPLAY_NAME); AttachmentViewInfo attachmentViewInfo = attachmentInfoExtractor.extractAttachmentInfo(part); assertEquals(AttachmentProvider.getAttachmentUri(TEST_ACCOUNT_UUID, TEST_ID), attachmentViewInfo.uri); - assertEquals(true, attachmentViewInfo.firstClassAttachment); assertEquals(TEST_SIZE, attachmentViewInfo.size); - assertEquals(TEST_DISPLAY_NAME, attachmentViewInfo.displayName); assertEquals(TEST_MIME_TYPE, attachmentViewInfo.mimeType); } From ed628ae67b77a182f7f23c7eacbf89b61e3b0476 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Mon, 11 Jul 2016 16:14:41 +0200 Subject: [PATCH 09/19] make FileProviderDeferredFileOutputStream fail faster --- .../FileProviderDeferredFileOutputStream.java | 38 ++++++++++++------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/k9mail/src/main/java/com/fsck/k9/service/FileProviderDeferredFileOutputStream.java b/k9mail/src/main/java/com/fsck/k9/service/FileProviderDeferredFileOutputStream.java index 1701b1b72..eebee33f3 100644 --- a/k9mail/src/main/java/com/fsck/k9/service/FileProviderDeferredFileOutputStream.java +++ b/k9mail/src/main/java/com/fsck/k9/service/FileProviderDeferredFileOutputStream.java @@ -17,9 +17,6 @@ import org.apache.commons.io.output.ThresholdingOutputStream; * itself. */ public class FileProviderDeferredFileOutputStream extends ThresholdingOutputStream { - - - private ByteArrayOutputStream memoryOutputStream; private OutputStream currentOutputStream; private File outputFile; private final FileProviderInterface fileProviderInterface; @@ -27,36 +24,49 @@ public class FileProviderDeferredFileOutputStream extends ThresholdingOutputStre public FileProviderDeferredFileOutputStream(int threshold, FileProviderInterface fileProviderInterface) { super(threshold); - memoryOutputStream = new ByteArrayOutputStream(); - currentOutputStream = memoryOutputStream; this.fileProviderInterface = fileProviderInterface; - } + // scale it so we expand the ByteArrayOutputStream at most three times (assuming quadratic growth) + int size = threshold < 1024 ? 256 : threshold / 4; + currentOutputStream = new ByteArrayOutputStream(size); + } @Override protected OutputStream getStream() throws IOException { return currentOutputStream; } + private boolean isMemoryBacked() { + return currentOutputStream instanceof ByteArrayOutputStream; + } @Override protected void thresholdReached() throws IOException { + if (outputFile != null) { + throw new IllegalStateException("thresholdReached must not be called if we already have an output file!"); + } + if (!isMemoryBacked()) { + throw new IllegalStateException("currentOutputStream must be memory-based at this point!"); + } + ByteArrayOutputStream memoryOutputStream = (ByteArrayOutputStream) currentOutputStream; + outputFile = fileProviderInterface.createProvidedFile(); - FileOutputStream fos = new FileOutputStream(outputFile); - memoryOutputStream.writeTo(fos); - currentOutputStream = fos; - memoryOutputStream = null; + currentOutputStream = new FileOutputStream(outputFile); + + memoryOutputStream.writeTo(currentOutputStream); } public byte[] getData() { - if (memoryOutputStream != null) - { - return memoryOutputStream.toByteArray(); + if (!isMemoryBacked()) { + throw new IllegalStateException("getData must only be called in memory-backed state!"); } - return null; + return ((ByteArrayOutputStream) currentOutputStream).toByteArray(); } public File getFile() { + if (isMemoryBacked()) { + throw new IllegalStateException("getFile must only be called in file-backed state!"); + } return outputFile; } From 2993078bf2379e88fddab913c1e26416394dfd17 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 14 Jul 2016 12:01:00 +0200 Subject: [PATCH 10/19] keep raw data for decrypted bodies around, decode in DecryptedFileProvider if necessary --- .../k9/mailstore/MimePartStreamParser.java | 24 +------- .../k9/mailstore/ProvidedTempFileBody.java | 29 +++++++--- .../k9/provider/DecryptedFileProvider.java | 57 ++++++++++++++++--- .../k9/service/FileProviderInterface.java | 2 +- 4 files changed, 73 insertions(+), 39 deletions(-) diff --git a/k9mail/src/main/java/com/fsck/k9/mailstore/MimePartStreamParser.java b/k9mail/src/main/java/com/fsck/k9/mailstore/MimePartStreamParser.java index 71fadd402..5666f1905 100644 --- a/k9mail/src/main/java/com/fsck/k9/mailstore/MimePartStreamParser.java +++ b/k9mail/src/main/java/com/fsck/k9/mailstore/MimePartStreamParser.java @@ -19,15 +19,12 @@ import com.fsck.k9.mail.internet.MimeUtility; import com.fsck.k9.service.FileProviderInterface; import org.apache.commons.io.IOUtils; import org.apache.james.mime4j.MimeException; -import org.apache.james.mime4j.codec.Base64InputStream; -import org.apache.james.mime4j.codec.QuotedPrintableInputStream; import org.apache.james.mime4j.io.EOLConvertingInputStream; import org.apache.james.mime4j.parser.ContentHandler; import org.apache.james.mime4j.parser.MimeStreamParser; import org.apache.james.mime4j.stream.BodyDescriptor; import org.apache.james.mime4j.stream.Field; import org.apache.james.mime4j.stream.MimeConfig; -import org.apache.james.mime4j.util.MimeUtil; public class MimePartStreamParser { @@ -58,26 +55,7 @@ public class MimePartStreamParser { ProvidedTempFileBody body = new ProvidedTempFileBody(fileProviderInterface, transferEncoding); OutputStream outputStream = body.getOutputStream(); try { - InputStream decodingInputStream; - boolean closeStream; - if (MimeUtil.ENC_QUOTED_PRINTABLE.equals(transferEncoding)) { - decodingInputStream = new QuotedPrintableInputStream(inputStream, false); - closeStream = true; - } else if (MimeUtil.ENC_BASE64.equals(transferEncoding)) { - decodingInputStream = new Base64InputStream(inputStream); - closeStream = true; - } else { - decodingInputStream = inputStream; - closeStream = false; - } - - try { - IOUtils.copy(decodingInputStream, outputStream); - } finally { - if (closeStream) { - decodingInputStream.close(); - } - } + IOUtils.copy(inputStream, outputStream); } finally { outputStream.close(); } diff --git a/k9mail/src/main/java/com/fsck/k9/mailstore/ProvidedTempFileBody.java b/k9mail/src/main/java/com/fsck/k9/mailstore/ProvidedTempFileBody.java index a0e1b9830..797b13c35 100644 --- a/k9mail/src/main/java/com/fsck/k9/mailstore/ProvidedTempFileBody.java +++ b/k9mail/src/main/java/com/fsck/k9/mailstore/ProvidedTempFileBody.java @@ -16,19 +16,22 @@ import android.util.Log; import com.fsck.k9.K9; import com.fsck.k9.mail.MessagingException; +import com.fsck.k9.mail.internet.RawDataBody; import com.fsck.k9.mail.internet.SizeAware; import com.fsck.k9.service.FileProviderDeferredFileOutputStream; import com.fsck.k9.service.FileProviderInterface; +import org.apache.commons.io.IOUtils; /** This is a body where the body can be accessed through a FileProvider. * @see FileProviderInterface */ -public class ProvidedTempFileBody extends BinaryAttachmentBody implements SizeAware { +public class ProvidedTempFileBody implements RawDataBody, SizeAware { public static final int MEMORY_BACKED_THRESHOLD = 1024 * 8; private final FileProviderInterface fileProviderInterface; + private final String encoding; @Nullable private byte[] data; @@ -37,11 +40,7 @@ public class ProvidedTempFileBody extends BinaryAttachmentBody implements SizeAw public ProvidedTempFileBody(FileProviderInterface fileProviderInterface, String transferEncoding) { this.fileProviderInterface = fileProviderInterface; - try { - setEncoding(transferEncoding); - } catch (MessagingException e) { - throw new AssertionError("setEncoding() must succeed"); - } + this.encoding = transferEncoding; } public OutputStream getOutputStream() throws IOException { @@ -93,7 +92,7 @@ public class ProvidedTempFileBody extends BinaryAttachmentBody implements SizeAw if (file == null) { writeMemoryToFile(); } - return fileProviderInterface.getUriForProvidedFile(file, mimeType); + return fileProviderInterface.getUriForProvidedFile(file, encoding, mimeType); } private void writeMemoryToFile() throws IOException { @@ -112,4 +111,20 @@ public class ProvidedTempFileBody extends BinaryAttachmentBody implements SizeAw data = null; } + + @Override + public void setEncoding(String encoding) throws MessagingException { + throw new UnsupportedOperationException("Cannot re-encode a DecryptedTempFileBody!"); + } + + @Override + public void writeTo(OutputStream out) throws IOException, MessagingException { + InputStream inputStream = getInputStream(); + IOUtils.copy(inputStream, out); + } + + @Override + public String getEncoding() { + return encoding; + } } diff --git a/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java b/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java index 17db70c85..0143369c5 100644 --- a/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java +++ b/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java @@ -2,7 +2,9 @@ package com.fsck.k9.provider; import java.io.File; +import java.io.FileNotFoundException; import java.io.IOException; +import java.io.InputStream; import java.util.Date; import java.util.Locale; @@ -11,13 +13,20 @@ import android.content.Context; import android.content.Intent; import android.content.IntentFilter; import android.net.Uri; +import android.os.ParcelFileDescriptor; import android.support.annotation.MainThread; +import android.support.annotation.Nullable; import android.support.v4.content.FileProvider; +import android.text.TextUtils; import android.util.Log; import com.fsck.k9.BuildConfig; import com.fsck.k9.K9; import com.fsck.k9.service.FileProviderInterface; +import org.apache.james.mime4j.codec.Base64InputStream; +import org.apache.james.mime4j.codec.QuotedPrintableInputStream; +import org.apache.james.mime4j.util.MimeUtil; +import org.openintents.openpgp.util.ParcelFileDescriptorUtil; public class DecryptedFileProvider extends FileProvider { @@ -29,11 +38,6 @@ public class DecryptedFileProvider extends FileProvider { private static DecryptedFileProviderCleanupReceiver receiverRegistered = null; - @Override - public String getType(Uri uri) { - return uri.getQueryParameter("mime_type"); - } - public static FileProviderInterface getFileProviderInterface(Context context) { final Context applicationContext = context.getApplicationContext(); @@ -46,9 +50,13 @@ public class DecryptedFileProvider extends FileProvider { } @Override - public Uri getUriForProvidedFile(File file, String mimeType) throws IOException { - Uri uri = FileProvider.getUriForFile(applicationContext, AUTHORITY, file); - return uri.buildUpon().appendQueryParameter("mime_type", mimeType).build(); + public Uri getUriForProvidedFile(File file, @Nullable String encoding, String mimeType) throws IOException { + Uri.Builder uriBuilder = FileProvider.getUriForFile(applicationContext, AUTHORITY, file).buildUpon(); + if (encoding != null) { + uriBuilder.appendQueryParameter("encoding", encoding); + } + uriBuilder.appendQueryParameter("mime_type", mimeType); + return uriBuilder.build(); } }; } @@ -90,6 +98,39 @@ public class DecryptedFileProvider extends FileProvider { return directory; } + + @Override + public String getType(Uri uri) { + return uri.getQueryParameter("mime_type"); + } + + @Override + public ParcelFileDescriptor openFile(Uri uri, String mode) throws FileNotFoundException { + ParcelFileDescriptor pfd = super.openFile(uri, "r"); + + InputStream decodedInputStream; + String encoding = uri.getQueryParameter("encoding"); + if (MimeUtil.isBase64Encoding(encoding)) { + InputStream inputStream = new ParcelFileDescriptor.AutoCloseInputStream(pfd); + decodedInputStream = new Base64InputStream(inputStream); + } else if (MimeUtil.isQuotedPrintableEncoded(encoding)) { + InputStream inputStream = new ParcelFileDescriptor.AutoCloseInputStream(pfd); + decodedInputStream = new QuotedPrintableInputStream(inputStream); + } else { // no or unknown encoding + if (K9.DEBUG && !TextUtils.isEmpty(encoding)) { + Log.e(K9.LOG_TAG, "unsupported encoding, returning raw stream"); + } + return pfd; + } + + try { + return ParcelFileDescriptorUtil.pipeFrom(decodedInputStream); + } catch (IOException e) { + // not strictly a FileNotFoundException, but failure to create a pipe is basically "can't access right now" + throw new FileNotFoundException(); + } + } + @Override public void onTrimMemory(int level) { if (level < TRIM_MEMORY_COMPLETE) { diff --git a/k9mail/src/main/java/com/fsck/k9/service/FileProviderInterface.java b/k9mail/src/main/java/com/fsck/k9/service/FileProviderInterface.java index 02b5faece..d12a6ccfb 100644 --- a/k9mail/src/main/java/com/fsck/k9/service/FileProviderInterface.java +++ b/k9mail/src/main/java/com/fsck/k9/service/FileProviderInterface.java @@ -10,6 +10,6 @@ import android.net.Uri; public interface FileProviderInterface { File createProvidedFile() throws IOException; - Uri getUriForProvidedFile(File file, String mimeType) throws IOException; + Uri getUriForProvidedFile(File file, String encoding, String mimeType) throws IOException; } From 3b0c1979f1d111585254ec00b79eeb20c37c45b7 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 14 Jul 2016 12:13:39 +0200 Subject: [PATCH 11/19] move file cleanup into AsyncTask in onTrimMemory --- .../com/fsck/k9/provider/DecryptedFileProvider.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java b/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java index 0143369c5..2af8924e7 100644 --- a/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java +++ b/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java @@ -13,6 +13,7 @@ import android.content.Context; import android.content.Intent; import android.content.IntentFilter; import android.net.Uri; +import android.os.AsyncTask; import android.os.ParcelFileDescriptor; import android.support.annotation.MainThread; import android.support.annotation.Nullable; @@ -136,12 +137,19 @@ public class DecryptedFileProvider extends FileProvider { if (level < TRIM_MEMORY_COMPLETE) { return; } - Context context = getContext(); + final Context context = getContext(); if (context == null) { return; } - deleteOldTemporaryFiles(context); + new AsyncTask() { + @Override + protected Void doInBackground(Void... voids) { + deleteOldTemporaryFiles(context); + return null; + } + }.execute(); + if (receiverRegistered != null) { context.unregisterReceiver(receiverRegistered); receiverRegistered = null; From f33c0835394f902c50d930a9246fe5617433d861 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 14 Jul 2016 12:14:08 +0200 Subject: [PATCH 12/19] don't allow file deletion in DecryptedFileProvider --- .../java/com/fsck/k9/provider/DecryptedFileProvider.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java b/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java index 2af8924e7..039e10c2a 100644 --- a/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java +++ b/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java @@ -105,6 +105,11 @@ public class DecryptedFileProvider extends FileProvider { return uri.getQueryParameter("mime_type"); } + @Override + public int delete(Uri uri, String selection, String[] selectionArgs) { + throw new UnsupportedOperationException(); + } + @Override public ParcelFileDescriptor openFile(Uri uri, String mode) throws FileNotFoundException { ParcelFileDescriptor pfd = super.openFile(uri, "r"); From 69b0b3a76310ea97262b2d8d675486cf7e838a4a Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 14 Jul 2016 18:04:46 +0200 Subject: [PATCH 13/19] split FileProvider logic, incorporate some other feedback --- .../fsck/k9/mailstore/AttachmentResolver.java | 9 ++--- ...empFileBody.java => DeferredFileBody.java} | 21 ++++++----- .../mailstore/MessageViewInfoExtractor.java | 4 +-- .../k9/mailstore/MimePartStreamParser.java | 18 +++++----- .../extractors/AttachmentInfoExtractor.java | 19 ++++++---- .../k9/provider/DecryptedFileProvider.java | 36 +++++++++++-------- .../java/com/fsck/k9/service/FileFactory.java | 10 ++++++ .../FileProviderDeferredFileOutputStream.java | 10 +++--- .../k9/service/FileProviderInterface.java | 15 -------- .../k9/ui/compose/QuotedMessagePresenter.java | 5 +-- .../k9/ui/crypto/MessageCryptoHelper.java | 8 ++--- .../k9/mailstore/AttachmentResolverTest.java | 34 +++++++++++------- .../AttachmentInfoExtractorTest.java | 18 ++++++---- 13 files changed, 115 insertions(+), 92 deletions(-) rename k9mail/src/main/java/com/fsck/k9/mailstore/{ProvidedTempFileBody.java => DeferredFileBody.java} (83%) create mode 100644 k9mail/src/main/java/com/fsck/k9/service/FileFactory.java delete mode 100644 k9mail/src/main/java/com/fsck/k9/service/FileProviderInterface.java diff --git a/k9mail/src/main/java/com/fsck/k9/mailstore/AttachmentResolver.java b/k9mail/src/main/java/com/fsck/k9/mailstore/AttachmentResolver.java index 5742a1a78..2d4947b23 100644 --- a/k9mail/src/main/java/com/fsck/k9/mailstore/AttachmentResolver.java +++ b/k9mail/src/main/java/com/fsck/k9/mailstore/AttachmentResolver.java @@ -6,6 +6,7 @@ import java.util.HashMap; import java.util.Map; import java.util.Stack; +import android.content.Context; import android.net.Uri; import android.support.annotation.Nullable; import android.support.annotation.VisibleForTesting; @@ -41,14 +42,14 @@ public class AttachmentResolver { } @WorkerThread - public static AttachmentResolver createFromPart(Part part) { + public static AttachmentResolver createFromPart(Context context, Part part) { AttachmentInfoExtractor attachmentInfoExtractor = AttachmentInfoExtractor.getInstance(); - Map contentIdToAttachmentUriMap = buildCidToAttachmentUriMap(attachmentInfoExtractor, part); + Map contentIdToAttachmentUriMap = buildCidToAttachmentUriMap(context, attachmentInfoExtractor, part); return new AttachmentResolver(contentIdToAttachmentUriMap); } @VisibleForTesting - static Map buildCidToAttachmentUriMap(AttachmentInfoExtractor attachmentInfoExtractor, + static Map buildCidToAttachmentUriMap(Context context, AttachmentInfoExtractor attachmentInfoExtractor, Part rootPart) { HashMap result = new HashMap<>(); @@ -68,7 +69,7 @@ public class AttachmentResolver { try { String contentId = part.getContentId(); if (contentId != null) { - AttachmentViewInfo attachmentInfo = attachmentInfoExtractor.extractAttachmentInfo(part); + AttachmentViewInfo attachmentInfo = attachmentInfoExtractor.extractAttachmentInfo(context, part); result.put(contentId, attachmentInfo.uri); } } catch (MessagingException e) { diff --git a/k9mail/src/main/java/com/fsck/k9/mailstore/ProvidedTempFileBody.java b/k9mail/src/main/java/com/fsck/k9/mailstore/DeferredFileBody.java similarity index 83% rename from k9mail/src/main/java/com/fsck/k9/mailstore/ProvidedTempFileBody.java rename to k9mail/src/main/java/com/fsck/k9/mailstore/DeferredFileBody.java index 797b13c35..95870300a 100644 --- a/k9mail/src/main/java/com/fsck/k9/mailstore/ProvidedTempFileBody.java +++ b/k9mail/src/main/java/com/fsck/k9/mailstore/DeferredFileBody.java @@ -10,7 +10,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import android.net.Uri; import android.support.annotation.Nullable; import android.util.Log; @@ -18,19 +17,19 @@ import com.fsck.k9.K9; import com.fsck.k9.mail.MessagingException; import com.fsck.k9.mail.internet.RawDataBody; import com.fsck.k9.mail.internet.SizeAware; +import com.fsck.k9.service.FileFactory; import com.fsck.k9.service.FileProviderDeferredFileOutputStream; -import com.fsck.k9.service.FileProviderInterface; import org.apache.commons.io.IOUtils; -/** This is a body where the body can be accessed through a FileProvider. - * @see FileProviderInterface +/** This is a body where the data is memory-backed at first and switches to file-backed if it gets larger. + * @see FileFactory */ -public class ProvidedTempFileBody implements RawDataBody, SizeAware { +public class DeferredFileBody implements RawDataBody, SizeAware { public static final int MEMORY_BACKED_THRESHOLD = 1024 * 8; - private final FileProviderInterface fileProviderInterface; + private final FileFactory fileFactory; private final String encoding; @Nullable @@ -38,13 +37,13 @@ public class ProvidedTempFileBody implements RawDataBody, SizeAware { private File file; - public ProvidedTempFileBody(FileProviderInterface fileProviderInterface, String transferEncoding) { - this.fileProviderInterface = fileProviderInterface; + public DeferredFileBody(FileFactory fileFactory, String transferEncoding) { + this.fileFactory = fileFactory; this.encoding = transferEncoding; } public OutputStream getOutputStream() throws IOException { - return new FileProviderDeferredFileOutputStream(MEMORY_BACKED_THRESHOLD, fileProviderInterface) { + return new FileProviderDeferredFileOutputStream(MEMORY_BACKED_THRESHOLD, fileFactory) { @Override public void close() throws IOException { super.close(); @@ -88,11 +87,11 @@ public class ProvidedTempFileBody implements RawDataBody, SizeAware { throw new IllegalStateException("Data must be fully written before it can be read!"); } - public Uri getProviderUri(String mimeType) throws IOException { + public File getFile() throws IOException { if (file == null) { writeMemoryToFile(); } - return fileProviderInterface.getUriForProvidedFile(file, encoding, mimeType); + return file; } private void writeMemoryToFile() throws IOException { diff --git a/k9mail/src/main/java/com/fsck/k9/mailstore/MessageViewInfoExtractor.java b/k9mail/src/main/java/com/fsck/k9/mailstore/MessageViewInfoExtractor.java index afe32cba1..7bad3e10d 100644 --- a/k9mail/src/main/java/com/fsck/k9/mailstore/MessageViewInfoExtractor.java +++ b/k9mail/src/main/java/com/fsck/k9/mailstore/MessageViewInfoExtractor.java @@ -74,7 +74,7 @@ public class MessageViewInfoExtractor { extraViewableText = extraViewable.text; } - AttachmentResolver attachmentResolver = AttachmentResolver.createFromPart(rootPart); + AttachmentResolver attachmentResolver = AttachmentResolver.createFromPart(context, rootPart); return MessageViewInfo.createWithExtractedContent(message, rootPart, viewable.html, attachmentInfos, cryptoResultAnnotation, extraViewableText, extraAttachmentInfos, attachmentResolver); @@ -89,7 +89,7 @@ public class MessageViewInfoExtractor { MessageExtractor.findViewablesAndAttachments(part, viewableParts, attachments); } - attachmentInfos.addAll(attachmentInfoExtractor.extractAttachmentInfos(attachments)); + attachmentInfos.addAll(attachmentInfoExtractor.extractAttachmentInfos(context, attachments)); return MessageViewInfoExtractor.extractTextFromViewables(context, viewableParts); } diff --git a/k9mail/src/main/java/com/fsck/k9/mailstore/MimePartStreamParser.java b/k9mail/src/main/java/com/fsck/k9/mailstore/MimePartStreamParser.java index 5666f1905..4e29f4cf7 100644 --- a/k9mail/src/main/java/com/fsck/k9/mailstore/MimePartStreamParser.java +++ b/k9mail/src/main/java/com/fsck/k9/mailstore/MimePartStreamParser.java @@ -16,7 +16,7 @@ import com.fsck.k9.mail.internet.MimeBodyPart; import com.fsck.k9.mail.internet.MimeMessage; import com.fsck.k9.mail.internet.MimeMultipart; import com.fsck.k9.mail.internet.MimeUtility; -import com.fsck.k9.service.FileProviderInterface; +import com.fsck.k9.service.FileFactory; import org.apache.commons.io.IOUtils; import org.apache.james.mime4j.MimeException; import org.apache.james.mime4j.io.EOLConvertingInputStream; @@ -28,7 +28,7 @@ import org.apache.james.mime4j.stream.MimeConfig; public class MimePartStreamParser { - public static MimeBodyPart parse(FileProviderInterface fileProviderInterface, InputStream inputStream) + public static MimeBodyPart parse(FileFactory fileFactory, InputStream inputStream) throws MessagingException, IOException { MimeBodyPart parsedRootPart = new MimeBodyPart(); @@ -38,7 +38,7 @@ public class MimePartStreamParser { parserConfig.setMaxHeaderCount(-1); MimeStreamParser parser = new MimeStreamParser(parserConfig); - parser.setContentHandler(new PartBuilder(fileProviderInterface, parsedRootPart)); + parser.setContentHandler(new PartBuilder(fileFactory, parsedRootPart)); parser.setRecurse(); try { @@ -51,8 +51,8 @@ public class MimePartStreamParser { } private static Body createBody(InputStream inputStream, String transferEncoding, - FileProviderInterface fileProviderInterface) throws IOException { - ProvidedTempFileBody body = new ProvidedTempFileBody(fileProviderInterface, transferEncoding); + FileFactory fileFactory) throws IOException { + DeferredFileBody body = new DeferredFileBody(fileFactory, transferEncoding); OutputStream outputStream = body.getOutputStream(); try { IOUtils.copy(inputStream, outputStream); @@ -65,13 +65,13 @@ public class MimePartStreamParser { private static class PartBuilder implements ContentHandler { - private final FileProviderInterface fileProviderInterface; + private final FileFactory fileFactory; private final MimeBodyPart decryptedRootPart; private final Stack stack = new Stack<>(); - public PartBuilder(FileProviderInterface fileProviderInterface, MimeBodyPart decryptedRootPart) + public PartBuilder(FileFactory fileFactory, MimeBodyPart decryptedRootPart) throws MessagingException { - this.fileProviderInterface = fileProviderInterface; + this.fileFactory = fileFactory; this.decryptedRootPart = decryptedRootPart; } @@ -173,7 +173,7 @@ public class MimePartStreamParser { Part part = (Part) stack.peek(); String transferEncoding = bd.getTransferEncoding(); - Body body = createBody(inputStream, transferEncoding, fileProviderInterface); + Body body = createBody(inputStream, transferEncoding, fileFactory); part.setBody(body); } diff --git a/k9mail/src/main/java/com/fsck/k9/message/extractors/AttachmentInfoExtractor.java b/k9mail/src/main/java/com/fsck/k9/message/extractors/AttachmentInfoExtractor.java index 577401fbf..d17fb29a2 100644 --- a/k9mail/src/main/java/com/fsck/k9/message/extractors/AttachmentInfoExtractor.java +++ b/k9mail/src/main/java/com/fsck/k9/message/extractors/AttachmentInfoExtractor.java @@ -1,10 +1,12 @@ package com.fsck.k9.message.extractors; +import java.io.File; import java.io.IOException; import java.util.ArrayList; import java.util.List; +import android.content.Context; import android.net.Uri; import android.util.Log; @@ -16,8 +18,9 @@ import com.fsck.k9.mail.internet.MimeHeader; import com.fsck.k9.mail.internet.MimeUtility; import com.fsck.k9.mailstore.AttachmentViewInfo; import com.fsck.k9.mailstore.LocalPart; -import com.fsck.k9.mailstore.ProvidedTempFileBody; +import com.fsck.k9.mailstore.DeferredFileBody; import com.fsck.k9.provider.AttachmentProvider; +import com.fsck.k9.provider.DecryptedFileProvider; public class AttachmentInfoExtractor { @@ -27,18 +30,18 @@ public class AttachmentInfoExtractor { return new AttachmentInfoExtractor(); } - public List extractAttachmentInfos(List attachmentParts) + public List extractAttachmentInfos(Context context, List attachmentParts) throws MessagingException { List attachments = new ArrayList<>(); for (Part part : attachmentParts) { - attachments.add(extractAttachmentInfo(part)); + attachments.add(extractAttachmentInfo(context, part)); } return attachments; } - public AttachmentViewInfo extractAttachmentInfo(Part part) throws MessagingException { + public AttachmentViewInfo extractAttachmentInfo(Context context, Part part) throws MessagingException { Uri uri; long size; if (part instanceof LocalPart) { @@ -49,11 +52,13 @@ public class AttachmentInfoExtractor { uri = AttachmentProvider.getAttachmentUri(accountUuid, messagePartId); } else { Body body = part.getBody(); - if (body instanceof ProvidedTempFileBody) { - ProvidedTempFileBody decryptedTempFileBody = (ProvidedTempFileBody) body; + if (body instanceof DeferredFileBody) { + DeferredFileBody decryptedTempFileBody = (DeferredFileBody) body; size = decryptedTempFileBody.getSize(); try { - uri = decryptedTempFileBody.getProviderUri(part.getMimeType()); + File file = decryptedTempFileBody.getFile(); + uri = DecryptedFileProvider.getUriForProvidedFile( + context, file, decryptedTempFileBody.getEncoding(), part.getMimeType()); } catch (IOException e) { Log.e(K9.LOG_TAG, "Decrypted temp file (no longer?) exists!", e); uri = null; diff --git a/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java b/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java index 039e10c2a..fb43455fc 100644 --- a/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java +++ b/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java @@ -16,6 +16,7 @@ import android.net.Uri; import android.os.AsyncTask; import android.os.ParcelFileDescriptor; import android.support.annotation.MainThread; +import android.support.annotation.NonNull; import android.support.annotation.Nullable; import android.support.v4.content.FileProvider; import android.text.TextUtils; @@ -23,7 +24,7 @@ import android.util.Log; import com.fsck.k9.BuildConfig; import com.fsck.k9.K9; -import com.fsck.k9.service.FileProviderInterface; +import com.fsck.k9.service.FileFactory; import org.apache.james.mime4j.codec.Base64InputStream; import org.apache.james.mime4j.codec.QuotedPrintableInputStream; import org.apache.james.mime4j.util.MimeUtil; @@ -39,29 +40,36 @@ public class DecryptedFileProvider extends FileProvider { private static DecryptedFileProviderCleanupReceiver receiverRegistered = null; - public static FileProviderInterface getFileProviderInterface(Context context) { + public static FileFactory getFileFactory(Context context) { final Context applicationContext = context.getApplicationContext(); - return new FileProviderInterface() { + return new FileFactory() { @Override - public File createProvidedFile() throws IOException { + public File createFile() throws IOException { registerFileCleanupReceiver(applicationContext); File decryptedTempDirectory = getDecryptedTempDirectory(applicationContext); return File.createTempFile("decrypted-", null, decryptedTempDirectory); } - - @Override - public Uri getUriForProvidedFile(File file, @Nullable String encoding, String mimeType) throws IOException { - Uri.Builder uriBuilder = FileProvider.getUriForFile(applicationContext, AUTHORITY, file).buildUpon(); - if (encoding != null) { - uriBuilder.appendQueryParameter("encoding", encoding); - } - uriBuilder.appendQueryParameter("mime_type", mimeType); - return uriBuilder.build(); - } }; } + @Nullable + public static Uri getUriForProvidedFile(@NonNull Context context, File file, + @Nullable String encoding, @Nullable String mimeType) throws IOException { + try { + Uri.Builder uriBuilder = FileProvider.getUriForFile(context, AUTHORITY, file).buildUpon(); + if (mimeType != null) { + uriBuilder.appendQueryParameter("mime_type", mimeType); + } + if (encoding != null) { + uriBuilder.appendQueryParameter("encoding", encoding); + } + return uriBuilder.build(); + } catch (IllegalArgumentException e) { + return null; + } + } + public static boolean deleteOldTemporaryFiles(Context context) { File tempDirectory = getDecryptedTempDirectory(context); boolean allFilesDeleted = true; diff --git a/k9mail/src/main/java/com/fsck/k9/service/FileFactory.java b/k9mail/src/main/java/com/fsck/k9/service/FileFactory.java new file mode 100644 index 000000000..84d954108 --- /dev/null +++ b/k9mail/src/main/java/com/fsck/k9/service/FileFactory.java @@ -0,0 +1,10 @@ +package com.fsck.k9.service; + + +import java.io.File; +import java.io.IOException; + + +public interface FileFactory { + File createFile() throws IOException; +} diff --git a/k9mail/src/main/java/com/fsck/k9/service/FileProviderDeferredFileOutputStream.java b/k9mail/src/main/java/com/fsck/k9/service/FileProviderDeferredFileOutputStream.java index eebee33f3..91121a9de 100644 --- a/k9mail/src/main/java/com/fsck/k9/service/FileProviderDeferredFileOutputStream.java +++ b/k9mail/src/main/java/com/fsck/k9/service/FileProviderDeferredFileOutputStream.java @@ -13,18 +13,18 @@ import org.apache.commons.io.output.ThresholdingOutputStream; /** * This OutputStream is modelled after apache commons' {@link DeferredFileOutputStream}, - * but uses a {@link FileProviderInterface} instead of directly generating temporary files + * but uses a {@link FileFactory} instead of directly generating temporary files * itself. */ public class FileProviderDeferredFileOutputStream extends ThresholdingOutputStream { private OutputStream currentOutputStream; private File outputFile; - private final FileProviderInterface fileProviderInterface; + private final FileFactory fileFactory; - public FileProviderDeferredFileOutputStream(int threshold, FileProviderInterface fileProviderInterface) { + public FileProviderDeferredFileOutputStream(int threshold, FileFactory fileFactory) { super(threshold); - this.fileProviderInterface = fileProviderInterface; + this.fileFactory = fileFactory; // scale it so we expand the ByteArrayOutputStream at most three times (assuming quadratic growth) int size = threshold < 1024 ? 256 : threshold / 4; @@ -50,7 +50,7 @@ public class FileProviderDeferredFileOutputStream extends ThresholdingOutputStre } ByteArrayOutputStream memoryOutputStream = (ByteArrayOutputStream) currentOutputStream; - outputFile = fileProviderInterface.createProvidedFile(); + outputFile = fileFactory.createFile(); currentOutputStream = new FileOutputStream(outputFile); memoryOutputStream.writeTo(currentOutputStream); diff --git a/k9mail/src/main/java/com/fsck/k9/service/FileProviderInterface.java b/k9mail/src/main/java/com/fsck/k9/service/FileProviderInterface.java deleted file mode 100644 index d12a6ccfb..000000000 --- a/k9mail/src/main/java/com/fsck/k9/service/FileProviderInterface.java +++ /dev/null @@ -1,15 +0,0 @@ -package com.fsck.k9.service; - - -import java.io.File; -import java.io.IOException; - -import android.net.Uri; - - -public interface FileProviderInterface { - - File createProvidedFile() throws IOException; - Uri getUriForProvidedFile(File file, String encoding, String mimeType) throws IOException; - -} diff --git a/k9mail/src/main/java/com/fsck/k9/ui/compose/QuotedMessagePresenter.java b/k9mail/src/main/java/com/fsck/k9/ui/compose/QuotedMessagePresenter.java index aa715d588..2009f7732 100644 --- a/k9mail/src/main/java/com/fsck/k9/ui/compose/QuotedMessagePresenter.java +++ b/k9mail/src/main/java/com/fsck/k9/ui/compose/QuotedMessagePresenter.java @@ -132,7 +132,8 @@ public class QuotedMessagePresenter { resources, sourceMessage, content, quoteStyle); // Load the message with the reply header. TODO replace with MessageViewInfo data - view.setQuotedHtml(quotedHtmlContent.getQuotedContent(), AttachmentResolver.createFromPart(sourceMessage)); + view.setQuotedHtml(quotedHtmlContent.getQuotedContent(), + AttachmentResolver.createFromPart(messageCompose, sourceMessage)); // TODO: Also strip the signature from the text/plain part view.setQuotedText(QuotedMessageHelper.quoteOriginalTextMessage(resources, sourceMessage, @@ -297,7 +298,7 @@ public class QuotedMessagePresenter { } // TODO replace with MessageViewInfo data view.setQuotedHtml(quotedHtmlContent.getQuotedContent(), - AttachmentResolver.createFromPart(message)); + AttachmentResolver.createFromPart(messageCompose, message)); } } if (bodyPlainOffset != null && bodyPlainLength != null) { diff --git a/k9mail/src/main/java/com/fsck/k9/ui/crypto/MessageCryptoHelper.java b/k9mail/src/main/java/com/fsck/k9/ui/crypto/MessageCryptoHelper.java index 1542661ee..ca52f16da 100644 --- a/k9mail/src/main/java/com/fsck/k9/ui/crypto/MessageCryptoHelper.java +++ b/k9mail/src/main/java/com/fsck/k9/ui/crypto/MessageCryptoHelper.java @@ -38,7 +38,7 @@ import com.fsck.k9.mailstore.LocalMessage; import com.fsck.k9.mailstore.MessageHelper; import com.fsck.k9.mailstore.MimePartStreamParser; import com.fsck.k9.provider.DecryptedFileProvider; -import com.fsck.k9.service.FileProviderInterface; +import com.fsck.k9.service.FileFactory; import org.apache.commons.io.IOUtils; import org.openintents.openpgp.IOpenPgpService2; import org.openintents.openpgp.OpenPgpDecryptionResult; @@ -427,9 +427,9 @@ public class MessageCryptoHelper { @WorkerThread public MimeBodyPart processData(InputStream is) throws IOException { try { - FileProviderInterface fileProviderInterface = - DecryptedFileProvider.getFileProviderInterface(context); - return MimePartStreamParser.parse(fileProviderInterface, is); + FileFactory fileFactory = + DecryptedFileProvider.getFileFactory(context); + return MimePartStreamParser.parse(fileFactory, is); } catch (MessagingException e) { Log.e(K9.LOG_TAG, "Something went wrong while parsing the decrypted MIME part", e); //TODO: pass error to main thread and display error message to user diff --git a/k9mail/src/test/java/com/fsck/k9/mailstore/AttachmentResolverTest.java b/k9mail/src/test/java/com/fsck/k9/mailstore/AttachmentResolverTest.java index fb67855db..2ebdd304f 100644 --- a/k9mail/src/test/java/com/fsck/k9/mailstore/AttachmentResolverTest.java +++ b/k9mail/src/test/java/com/fsck/k9/mailstore/AttachmentResolverTest.java @@ -11,9 +11,11 @@ import com.fsck.k9.mail.BodyPart; import com.fsck.k9.mail.Multipart; import com.fsck.k9.mail.Part; import com.fsck.k9.message.extractors.AttachmentInfoExtractor; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.robolectric.RobolectricTestRunner; +import org.robolectric.RuntimeEnvironment; import org.robolectric.annotation.Config; import static org.junit.Assert.*; @@ -30,33 +32,40 @@ public class AttachmentResolverTest { public static final Uri ATTACHMENT_TEST_URI_2 = Uri.parse("uri://test/2"); + private AttachmentInfoExtractor attachmentInfoExtractor; + + + @Before + public void setUp() throws Exception { + attachmentInfoExtractor = mock(AttachmentInfoExtractor.class); + } + @Test public void buildCidMap__onPartWithNoBody__shouldReturnEmptyMap() throws Exception { - AttachmentInfoExtractor attachmentInfoExtractor = mock(AttachmentInfoExtractor.class); Part part = mock(Part.class); when(part.getContentId()).thenReturn(null); - Map result = AttachmentResolver.buildCidToAttachmentUriMap(attachmentInfoExtractor, part); + Map result = AttachmentResolver.buildCidToAttachmentUriMap( + RuntimeEnvironment.application, attachmentInfoExtractor, part); assertTrue(result.isEmpty()); } @Test public void buildCidMap__onMultipartWithNoParts__shouldReturnEmptyMap() throws Exception { - AttachmentInfoExtractor attachmentInfoExtractor = mock(AttachmentInfoExtractor.class); Part multipartPart = mock(Part.class); Multipart multipartBody = mock(Multipart.class); when(multipartPart.getBody()).thenReturn(multipartBody); when(multipartBody.getBodyParts()).thenReturn(Collections.EMPTY_LIST); - Map result = AttachmentResolver.buildCidToAttachmentUriMap(attachmentInfoExtractor, multipartPart); + Map result = AttachmentResolver.buildCidToAttachmentUriMap( + RuntimeEnvironment.application, attachmentInfoExtractor, multipartPart); assertTrue(result.isEmpty()); } @Test public void buildCidMap__onMultipartWithEmptyBodyPart__shouldReturnEmptyMap() throws Exception { - AttachmentInfoExtractor attachmentInfoExtractor = mock(AttachmentInfoExtractor.class); Part multipartPart = mock(Part.class); Multipart multipartBody = mock(Multipart.class); BodyPart bodyPart = mock(BodyPart.class); @@ -66,7 +75,8 @@ public class AttachmentResolverTest { when(bodyPart.getContentId()).thenReturn(null); - Map result = AttachmentResolver.buildCidToAttachmentUriMap(attachmentInfoExtractor, multipartPart); + Map result = AttachmentResolver.buildCidToAttachmentUriMap( + RuntimeEnvironment.application, attachmentInfoExtractor, multipartPart); verify(bodyPart).getContentId(); @@ -75,7 +85,6 @@ public class AttachmentResolverTest { @Test public void buildCidMap__onTwoPart__shouldReturnBothUris() throws Exception { - AttachmentInfoExtractor attachmentInfoExtractor = mock(AttachmentInfoExtractor.class); Part multipartPart = mock(Part.class); Multipart multipartBody = mock(Multipart.class); BodyPart bodyPart = mock(BodyPart.class); @@ -88,14 +97,15 @@ public class AttachmentResolverTest { when(subPart1.getContentId()).thenReturn("cid-1"); when(subPart2.getContentId()).thenReturn("cid-2"); - when(attachmentInfoExtractor.extractAttachmentInfo(subPart1)).thenReturn(new AttachmentViewInfo( - null, null, AttachmentViewInfo.UNKNOWN_SIZE, ATTACHMENT_TEST_URI_1, true, subPart1)); - when(attachmentInfoExtractor.extractAttachmentInfo(subPart2)).thenReturn(new AttachmentViewInfo( - null, null, AttachmentViewInfo.UNKNOWN_SIZE, ATTACHMENT_TEST_URI_2, true, subPart2)); + when(attachmentInfoExtractor.extractAttachmentInfo(RuntimeEnvironment.application, subPart1)).thenReturn( + new AttachmentViewInfo(null, null, AttachmentViewInfo.UNKNOWN_SIZE, ATTACHMENT_TEST_URI_1, true, subPart1)); + when(attachmentInfoExtractor.extractAttachmentInfo(RuntimeEnvironment.application, subPart2)).thenReturn( + new AttachmentViewInfo(null, null, AttachmentViewInfo.UNKNOWN_SIZE, ATTACHMENT_TEST_URI_2, true, subPart2)); when(multipartBody.getBodyParts()).thenReturn(Arrays.asList(subPart1, subPart2)); - Map result = AttachmentResolver.buildCidToAttachmentUriMap(attachmentInfoExtractor, multipartPart); + Map result = AttachmentResolver.buildCidToAttachmentUriMap(RuntimeEnvironment.application, + attachmentInfoExtractor, multipartPart); assertEquals(2, result.size()); assertEquals(ATTACHMENT_TEST_URI_1, result.get("cid-1")); diff --git a/k9mail/src/test/java/com/fsck/k9/message/extractors/AttachmentInfoExtractorTest.java b/k9mail/src/test/java/com/fsck/k9/message/extractors/AttachmentInfoExtractorTest.java index 0e28bd197..f4c18c76d 100644 --- a/k9mail/src/test/java/com/fsck/k9/message/extractors/AttachmentInfoExtractorTest.java +++ b/k9mail/src/test/java/com/fsck/k9/message/extractors/AttachmentInfoExtractorTest.java @@ -1,18 +1,21 @@ package com.fsck.k9.message.extractors; +import java.io.File; + import android.net.Uri; import com.fsck.k9.mail.Part; import com.fsck.k9.mail.internet.MimeHeader; import com.fsck.k9.mailstore.AttachmentViewInfo; import com.fsck.k9.mailstore.LocalBodyPart; -import com.fsck.k9.mailstore.ProvidedTempFileBody; +import com.fsck.k9.mailstore.DeferredFileBody; import com.fsck.k9.provider.AttachmentProvider; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.robolectric.RobolectricTestRunner; +import org.robolectric.RuntimeEnvironment; import org.robolectric.annotation.Config; import static org.junit.Assert.assertEquals; @@ -46,7 +49,7 @@ public class AttachmentInfoExtractorTest { public void extractInfo__withGenericPart_shouldThrow() throws Exception { Part part = mock(Part.class); - attachmentInfoExtractor.extractAttachmentInfo(part); + attachmentInfoExtractor.extractAttachmentInfo(RuntimeEnvironment.application, part); } @Test @@ -57,7 +60,8 @@ public class AttachmentInfoExtractorTest { when(part.getSize()).thenReturn(TEST_SIZE); when(part.getAccountUuid()).thenReturn(TEST_ACCOUNT_UUID); - AttachmentViewInfo attachmentViewInfo = attachmentInfoExtractor.extractAttachmentInfo(part); + AttachmentViewInfo attachmentViewInfo = attachmentInfoExtractor.extractAttachmentInfo( + RuntimeEnvironment.application, part); assertEquals(AttachmentProvider.getAttachmentUri(TEST_ACCOUNT_UUID, TEST_ID), attachmentViewInfo.uri); assertEquals(TEST_SIZE, attachmentViewInfo.size); @@ -114,7 +118,7 @@ public class AttachmentInfoExtractorTest { } @Test - public void extractInfoForDb__wWithDispositionAttach__shouldReturnNamedFirstClassAttachment() throws Exception { + public void extractInfoForDb__withDispositionAttach__shouldReturnNamedFirstClassAttachment() throws Exception { Part part = mock(Part.class); when(part.getDisposition()).thenReturn("attachment" + "; filename=\"filename.ext\"; meaningless=\"dummy\""); @@ -159,15 +163,15 @@ public class AttachmentInfoExtractorTest { @Test public void extractInfo__withProvidedTempFileBody() throws Exception { - ProvidedTempFileBody body = mock(ProvidedTempFileBody.class); + DeferredFileBody body = mock(DeferredFileBody.class); Part part = mock(Part.class); when(part.getBody()).thenReturn(body); when(part.getMimeType()).thenReturn(TEST_MIME_TYPE); when(body.getSize()).thenReturn(TEST_SIZE); - when(body.getProviderUri(TEST_MIME_TYPE)).thenReturn(TEST_URI); - AttachmentViewInfo attachmentViewInfo = attachmentInfoExtractor.extractAttachmentInfo(part); + AttachmentViewInfo attachmentViewInfo = attachmentInfoExtractor.extractAttachmentInfo( + RuntimeEnvironment.application, part); assertEquals(TEST_URI, attachmentViewInfo.uri); assertEquals(TEST_SIZE, attachmentViewInfo.size); From 351737512b1e2de4b4c7106448abc3fdfded30ef Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Fri, 15 Jul 2016 10:52:59 +0200 Subject: [PATCH 14/19] fix deferred file body test --- .../extractors/AttachmentInfoExtractor.java | 29 +++++++++++++------ .../AttachmentInfoExtractorTest.java | 13 ++++++++- 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/k9mail/src/main/java/com/fsck/k9/message/extractors/AttachmentInfoExtractor.java b/k9mail/src/main/java/com/fsck/k9/message/extractors/AttachmentInfoExtractor.java index d17fb29a2..9488ba486 100644 --- a/k9mail/src/main/java/com/fsck/k9/message/extractors/AttachmentInfoExtractor.java +++ b/k9mail/src/main/java/com/fsck/k9/message/extractors/AttachmentInfoExtractor.java @@ -8,6 +8,8 @@ import java.util.List; import android.content.Context; import android.net.Uri; +import android.support.annotation.Nullable; +import android.support.annotation.VisibleForTesting; import android.util.Log; import com.fsck.k9.K9; @@ -24,7 +26,8 @@ import com.fsck.k9.provider.DecryptedFileProvider; public class AttachmentInfoExtractor { - private AttachmentInfoExtractor() { } + @VisibleForTesting + AttachmentInfoExtractor() { } public static AttachmentInfoExtractor getInstance() { return new AttachmentInfoExtractor(); @@ -55,14 +58,7 @@ public class AttachmentInfoExtractor { if (body instanceof DeferredFileBody) { DeferredFileBody decryptedTempFileBody = (DeferredFileBody) body; size = decryptedTempFileBody.getSize(); - try { - File file = decryptedTempFileBody.getFile(); - uri = DecryptedFileProvider.getUriForProvidedFile( - context, file, decryptedTempFileBody.getEncoding(), part.getMimeType()); - } catch (IOException e) { - Log.e(K9.LOG_TAG, "Decrypted temp file (no longer?) exists!", e); - uri = null; - } + uri = getDecryptedFileProviderUri(context, decryptedTempFileBody, part.getMimeType()); return extractAttachmentInfo(part, uri, size); } else { throw new IllegalArgumentException("Unsupported part type provided"); @@ -72,6 +68,21 @@ public class AttachmentInfoExtractor { return extractAttachmentInfo(part, uri, size); } + @Nullable + @VisibleForTesting + protected Uri getDecryptedFileProviderUri(Context context, DeferredFileBody decryptedTempFileBody, String mimeType) { + Uri uri; + try { + File file = decryptedTempFileBody.getFile(); + uri = DecryptedFileProvider.getUriForProvidedFile( + context, file, decryptedTempFileBody.getEncoding(), mimeType); + } catch (IOException e) { + Log.e(K9.LOG_TAG, "Decrypted temp file (no longer?) exists!", e); + uri = null; + } + return uri; + } + public AttachmentViewInfo extractAttachmentInfoForDatabase(Part part) throws MessagingException { return extractAttachmentInfo(part, Uri.EMPTY, AttachmentViewInfo.UNKNOWN_SIZE); } diff --git a/k9mail/src/test/java/com/fsck/k9/message/extractors/AttachmentInfoExtractorTest.java b/k9mail/src/test/java/com/fsck/k9/message/extractors/AttachmentInfoExtractorTest.java index f4c18c76d..56783e8ac 100644 --- a/k9mail/src/test/java/com/fsck/k9/message/extractors/AttachmentInfoExtractorTest.java +++ b/k9mail/src/test/java/com/fsck/k9/message/extractors/AttachmentInfoExtractorTest.java @@ -3,7 +3,9 @@ package com.fsck.k9.message.extractors; import java.io.File; +import android.content.Context; import android.net.Uri; +import android.support.annotation.Nullable; import com.fsck.k9.mail.Part; import com.fsck.k9.mail.internet.MimeHeader; @@ -162,7 +164,16 @@ public class AttachmentInfoExtractorTest { } @Test - public void extractInfo__withProvidedTempFileBody() throws Exception { + public void extractInfo__withDeferredFileBody() throws Exception { + attachmentInfoExtractor = new AttachmentInfoExtractor() { + @Nullable + @Override + protected Uri getDecryptedFileProviderUri(Context context, DeferredFileBody decryptedTempFileBody, + String mimeType) { + return TEST_URI; + } + }; + DeferredFileBody body = mock(DeferredFileBody.class); Part part = mock(Part.class); when(part.getBody()).thenReturn(body); From c4cb83d370736d36689123db45f6d29fe209f060 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Fri, 15 Jul 2016 11:03:37 +0200 Subject: [PATCH 15/19] rename some classes from service to mailstore --- .../com/fsck/k9/mailstore/DeferredFileBody.java | 6 +++--- .../fsck/k9/mailstore/MimePartStreamParser.java | 2 +- .../util/DeferredFileOutputStream.java} | 17 ++++++++--------- .../util}/FileFactory.java | 2 +- .../fsck/k9/provider/DecryptedFileProvider.java | 2 +- .../fsck/k9/ui/crypto/MessageCryptoHelper.java | 2 +- .../extractors/AttachmentInfoExtractorTest.java | 2 ++ 7 files changed, 17 insertions(+), 16 deletions(-) rename k9mail/src/main/java/com/fsck/k9/{service/FileProviderDeferredFileOutputStream.java => mailstore/util/DeferredFileOutputStream.java} (84%) rename k9mail/src/main/java/com/fsck/k9/{service => mailstore/util}/FileFactory.java (78%) diff --git a/k9mail/src/main/java/com/fsck/k9/mailstore/DeferredFileBody.java b/k9mail/src/main/java/com/fsck/k9/mailstore/DeferredFileBody.java index 95870300a..e21882f5c 100644 --- a/k9mail/src/main/java/com/fsck/k9/mailstore/DeferredFileBody.java +++ b/k9mail/src/main/java/com/fsck/k9/mailstore/DeferredFileBody.java @@ -17,8 +17,8 @@ import com.fsck.k9.K9; import com.fsck.k9.mail.MessagingException; import com.fsck.k9.mail.internet.RawDataBody; import com.fsck.k9.mail.internet.SizeAware; -import com.fsck.k9.service.FileFactory; -import com.fsck.k9.service.FileProviderDeferredFileOutputStream; +import com.fsck.k9.mailstore.util.DeferredFileOutputStream; +import com.fsck.k9.mailstore.util.FileFactory; import org.apache.commons.io.IOUtils; @@ -43,7 +43,7 @@ public class DeferredFileBody implements RawDataBody, SizeAware { } public OutputStream getOutputStream() throws IOException { - return new FileProviderDeferredFileOutputStream(MEMORY_BACKED_THRESHOLD, fileFactory) { + return new DeferredFileOutputStream(MEMORY_BACKED_THRESHOLD, fileFactory) { @Override public void close() throws IOException { super.close(); diff --git a/k9mail/src/main/java/com/fsck/k9/mailstore/MimePartStreamParser.java b/k9mail/src/main/java/com/fsck/k9/mailstore/MimePartStreamParser.java index 4e29f4cf7..ae86e00ce 100644 --- a/k9mail/src/main/java/com/fsck/k9/mailstore/MimePartStreamParser.java +++ b/k9mail/src/main/java/com/fsck/k9/mailstore/MimePartStreamParser.java @@ -16,7 +16,7 @@ import com.fsck.k9.mail.internet.MimeBodyPart; import com.fsck.k9.mail.internet.MimeMessage; import com.fsck.k9.mail.internet.MimeMultipart; import com.fsck.k9.mail.internet.MimeUtility; -import com.fsck.k9.service.FileFactory; +import com.fsck.k9.mailstore.util.FileFactory; import org.apache.commons.io.IOUtils; import org.apache.james.mime4j.MimeException; import org.apache.james.mime4j.io.EOLConvertingInputStream; diff --git a/k9mail/src/main/java/com/fsck/k9/service/FileProviderDeferredFileOutputStream.java b/k9mail/src/main/java/com/fsck/k9/mailstore/util/DeferredFileOutputStream.java similarity index 84% rename from k9mail/src/main/java/com/fsck/k9/service/FileProviderDeferredFileOutputStream.java rename to k9mail/src/main/java/com/fsck/k9/mailstore/util/DeferredFileOutputStream.java index 91121a9de..a56ea9a9f 100644 --- a/k9mail/src/main/java/com/fsck/k9/service/FileProviderDeferredFileOutputStream.java +++ b/k9mail/src/main/java/com/fsck/k9/mailstore/util/DeferredFileOutputStream.java @@ -1,4 +1,4 @@ -package com.fsck.k9.service; +package com.fsck.k9.mailstore.util; import java.io.ByteArrayOutputStream; @@ -7,22 +7,21 @@ import java.io.FileOutputStream; import java.io.IOException; import java.io.OutputStream; -import org.apache.commons.io.output.DeferredFileOutputStream; import org.apache.commons.io.output.ThresholdingOutputStream; /** - * This OutputStream is modelled after apache commons' {@link DeferredFileOutputStream}, - * but uses a {@link FileFactory} instead of directly generating temporary files - * itself. + * This OutputStream is modelled after apache commons' {@link org.apache.commons.io.output.DeferredFileOutputStream}, + * but uses a {@link FileFactory} instead of directly generating temporary files itself. */ -public class FileProviderDeferredFileOutputStream extends ThresholdingOutputStream { - private OutputStream currentOutputStream; - private File outputFile; +public class DeferredFileOutputStream extends ThresholdingOutputStream { private final FileFactory fileFactory; + private OutputStream currentOutputStream; + private File outputFile; - public FileProviderDeferredFileOutputStream(int threshold, FileFactory fileFactory) { + + public DeferredFileOutputStream(int threshold, FileFactory fileFactory) { super(threshold); this.fileFactory = fileFactory; diff --git a/k9mail/src/main/java/com/fsck/k9/service/FileFactory.java b/k9mail/src/main/java/com/fsck/k9/mailstore/util/FileFactory.java similarity index 78% rename from k9mail/src/main/java/com/fsck/k9/service/FileFactory.java rename to k9mail/src/main/java/com/fsck/k9/mailstore/util/FileFactory.java index 84d954108..749999ca6 100644 --- a/k9mail/src/main/java/com/fsck/k9/service/FileFactory.java +++ b/k9mail/src/main/java/com/fsck/k9/mailstore/util/FileFactory.java @@ -1,4 +1,4 @@ -package com.fsck.k9.service; +package com.fsck.k9.mailstore.util; import java.io.File; diff --git a/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java b/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java index fb43455fc..24edf0e1b 100644 --- a/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java +++ b/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java @@ -24,7 +24,7 @@ import android.util.Log; import com.fsck.k9.BuildConfig; import com.fsck.k9.K9; -import com.fsck.k9.service.FileFactory; +import com.fsck.k9.mailstore.util.FileFactory; import org.apache.james.mime4j.codec.Base64InputStream; import org.apache.james.mime4j.codec.QuotedPrintableInputStream; import org.apache.james.mime4j.util.MimeUtil; diff --git a/k9mail/src/main/java/com/fsck/k9/ui/crypto/MessageCryptoHelper.java b/k9mail/src/main/java/com/fsck/k9/ui/crypto/MessageCryptoHelper.java index ca52f16da..29cdf035e 100644 --- a/k9mail/src/main/java/com/fsck/k9/ui/crypto/MessageCryptoHelper.java +++ b/k9mail/src/main/java/com/fsck/k9/ui/crypto/MessageCryptoHelper.java @@ -37,8 +37,8 @@ import com.fsck.k9.mailstore.CryptoResultAnnotation.CryptoError; import com.fsck.k9.mailstore.LocalMessage; import com.fsck.k9.mailstore.MessageHelper; import com.fsck.k9.mailstore.MimePartStreamParser; +import com.fsck.k9.mailstore.util.FileFactory; import com.fsck.k9.provider.DecryptedFileProvider; -import com.fsck.k9.service.FileFactory; import org.apache.commons.io.IOUtils; import org.openintents.openpgp.IOpenPgpService2; import org.openintents.openpgp.OpenPgpDecryptionResult; diff --git a/k9mail/src/test/java/com/fsck/k9/message/extractors/AttachmentInfoExtractorTest.java b/k9mail/src/test/java/com/fsck/k9/message/extractors/AttachmentInfoExtractorTest.java index 56783e8ac..b90dca38a 100644 --- a/k9mail/src/test/java/com/fsck/k9/message/extractors/AttachmentInfoExtractorTest.java +++ b/k9mail/src/test/java/com/fsck/k9/message/extractors/AttachmentInfoExtractorTest.java @@ -181,9 +181,11 @@ public class AttachmentInfoExtractorTest { when(body.getSize()).thenReturn(TEST_SIZE); + AttachmentViewInfo attachmentViewInfo = attachmentInfoExtractor.extractAttachmentInfo( RuntimeEnvironment.application, part); + assertEquals(TEST_URI, attachmentViewInfo.uri); assertEquals(TEST_SIZE, attachmentViewInfo.size); assertEquals(TEST_MIME_TYPE, attachmentViewInfo.mimeType); From f6fe28d3a84f3fb3afcc85184160981f0111cc10 Mon Sep 17 00:00:00 2001 From: cketti Date: Sun, 17 Jul 2016 13:28:25 +0200 Subject: [PATCH 16/19] Add Globals class so we can avoid passing Context through layers of code If your class requires a Context instance make it a constructor argument. Then create a static factory method that calls Globals.getContext(). The result can then be passed to the constructor. This allows testing individual classes using test doubles by directly invoking the constructor and not having to deal with Globals. For integrated tests spanning multiple classes you might have to use Globals.setContext(). --- k9mail/src/main/java/com/fsck/k9/Globals.java | 21 +++++++++++++++++++ k9mail/src/main/java/com/fsck/k9/K9.java | 1 + 2 files changed, 22 insertions(+) create mode 100644 k9mail/src/main/java/com/fsck/k9/Globals.java diff --git a/k9mail/src/main/java/com/fsck/k9/Globals.java b/k9mail/src/main/java/com/fsck/k9/Globals.java new file mode 100644 index 000000000..31e74e898 --- /dev/null +++ b/k9mail/src/main/java/com/fsck/k9/Globals.java @@ -0,0 +1,21 @@ +package com.fsck.k9; + + +import android.content.Context; + + +public class Globals { + private static Context context; + + static void setContext(Context context) { + Globals.context = context; + } + + public static Context getContext() { + if (context == null) { + throw new IllegalStateException("No context provided"); + } + + return context; + } +} diff --git a/k9mail/src/main/java/com/fsck/k9/K9.java b/k9mail/src/main/java/com/fsck/k9/K9.java index 93ceeccab..34e41347a 100644 --- a/k9mail/src/main/java/com/fsck/k9/K9.java +++ b/k9mail/src/main/java/com/fsck/k9/K9.java @@ -505,6 +505,7 @@ public class K9 extends Application { super.onCreate(); app = this; + Globals.setContext(this); K9MailLib.setDebugStatus(new K9MailLib.DebugStatus() { @Override public boolean enabled() { From 44c6fccc0e65a2111ebb5d82109c227a53f8669e Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Fri, 15 Jul 2016 11:20:37 +0200 Subject: [PATCH 17/19] synchronize cleanupReceiver access --- .../k9/provider/DecryptedFileProvider.java | 48 +++++++++++-------- 1 file changed, 29 insertions(+), 19 deletions(-) diff --git a/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java b/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java index 24edf0e1b..653e42433 100644 --- a/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java +++ b/k9mail/src/main/java/com/fsck/k9/provider/DecryptedFileProvider.java @@ -35,9 +35,10 @@ public class DecryptedFileProvider extends FileProvider { private static final String AUTHORITY = BuildConfig.APPLICATION_ID + ".decryptedfileprovider"; private static final String DECRYPTED_CACHE_DIRECTORY = "decrypted"; private static final long FILE_DELETE_THRESHOLD_MILLISECONDS = 3 * 60 * 1000; + private static final Object cleanupReceiverMonitor = new Object(); - private static DecryptedFileProviderCleanupReceiver receiverRegistered = null; + private static DecryptedFileProviderCleanupReceiver cleanupReceiver = null; public static FileFactory getFileFactory(Context context) { @@ -163,24 +164,37 @@ public class DecryptedFileProvider extends FileProvider { } }.execute(); - if (receiverRegistered != null) { - context.unregisterReceiver(receiverRegistered); - receiverRegistered = null; + unregisterFileCleanupReceiver(context); + } + + private static void unregisterFileCleanupReceiver(Context context) { + synchronized (cleanupReceiverMonitor) { + if (cleanupReceiver == null) { + return; + } + + if (K9.DEBUG) { + Log.d(K9.LOG_TAG, "Unregistering temp file cleanup receiver"); + } + context.unregisterReceiver(cleanupReceiver); + cleanupReceiver = null; } } - @MainThread // no need to synchronize for receiverRegistered private static void registerFileCleanupReceiver(Context context) { - if (receiverRegistered != null) { - return; + synchronized (cleanupReceiverMonitor) { + if (cleanupReceiver != null) { + return; + } + if (K9.DEBUG) { + Log.d(K9.LOG_TAG, "Registering temp file cleanup receiver"); + } + cleanupReceiver = new DecryptedFileProviderCleanupReceiver(); + + IntentFilter intentFilter = new IntentFilter(); + intentFilter.addAction(Intent.ACTION_SCREEN_OFF); + context.registerReceiver(cleanupReceiver, intentFilter); } - if (K9.DEBUG) { - Log.d(K9.LOG_TAG, "Registering temp file cleanup receiver"); - } - receiverRegistered = new DecryptedFileProviderCleanupReceiver(); - IntentFilter intentFilter = new IntentFilter(); - intentFilter.addAction(Intent.ACTION_SCREEN_OFF); - context.registerReceiver(receiverRegistered, intentFilter); } private static class DecryptedFileProviderCleanupReceiver extends BroadcastReceiver { @@ -197,11 +211,7 @@ public class DecryptedFileProvider extends FileProvider { boolean allFilesDeleted = deleteOldTemporaryFiles(context); if (allFilesDeleted) { - if (K9.DEBUG) { - Log.d(K9.LOG_TAG, "Unregistering temp file cleanup receiver"); - } - context.unregisterReceiver(this); - receiverRegistered = null; + unregisterFileCleanupReceiver(context); } } } From d096261c56f6feb9f41648b4d6348975d3ff9a2e Mon Sep 17 00:00:00 2001 From: cketti Date: Sun, 17 Jul 2016 13:51:33 +0200 Subject: [PATCH 18/19] Make use of Globals to simplify code --- .../fsck/k9/mailstore/AttachmentResolver.java | 9 +++---- .../mailstore/MessageViewInfoExtractor.java | 4 +-- .../extractors/AttachmentInfoExtractor.java | 24 +++++++++++------- .../k9/ui/compose/QuotedMessagePresenter.java | 6 ++--- .../k9/mailstore/AttachmentResolverTest.java | 25 ++++++------------- .../AttachmentInfoExtractorTest.java | 21 +++++++--------- 6 files changed, 40 insertions(+), 49 deletions(-) diff --git a/k9mail/src/main/java/com/fsck/k9/mailstore/AttachmentResolver.java b/k9mail/src/main/java/com/fsck/k9/mailstore/AttachmentResolver.java index 2d4947b23..5742a1a78 100644 --- a/k9mail/src/main/java/com/fsck/k9/mailstore/AttachmentResolver.java +++ b/k9mail/src/main/java/com/fsck/k9/mailstore/AttachmentResolver.java @@ -6,7 +6,6 @@ import java.util.HashMap; import java.util.Map; import java.util.Stack; -import android.content.Context; import android.net.Uri; import android.support.annotation.Nullable; import android.support.annotation.VisibleForTesting; @@ -42,14 +41,14 @@ public class AttachmentResolver { } @WorkerThread - public static AttachmentResolver createFromPart(Context context, Part part) { + public static AttachmentResolver createFromPart(Part part) { AttachmentInfoExtractor attachmentInfoExtractor = AttachmentInfoExtractor.getInstance(); - Map contentIdToAttachmentUriMap = buildCidToAttachmentUriMap(context, attachmentInfoExtractor, part); + Map contentIdToAttachmentUriMap = buildCidToAttachmentUriMap(attachmentInfoExtractor, part); return new AttachmentResolver(contentIdToAttachmentUriMap); } @VisibleForTesting - static Map buildCidToAttachmentUriMap(Context context, AttachmentInfoExtractor attachmentInfoExtractor, + static Map buildCidToAttachmentUriMap(AttachmentInfoExtractor attachmentInfoExtractor, Part rootPart) { HashMap result = new HashMap<>(); @@ -69,7 +68,7 @@ public class AttachmentResolver { try { String contentId = part.getContentId(); if (contentId != null) { - AttachmentViewInfo attachmentInfo = attachmentInfoExtractor.extractAttachmentInfo(context, part); + AttachmentViewInfo attachmentInfo = attachmentInfoExtractor.extractAttachmentInfo(part); result.put(contentId, attachmentInfo.uri); } } catch (MessagingException e) { diff --git a/k9mail/src/main/java/com/fsck/k9/mailstore/MessageViewInfoExtractor.java b/k9mail/src/main/java/com/fsck/k9/mailstore/MessageViewInfoExtractor.java index 7bad3e10d..afe32cba1 100644 --- a/k9mail/src/main/java/com/fsck/k9/mailstore/MessageViewInfoExtractor.java +++ b/k9mail/src/main/java/com/fsck/k9/mailstore/MessageViewInfoExtractor.java @@ -74,7 +74,7 @@ public class MessageViewInfoExtractor { extraViewableText = extraViewable.text; } - AttachmentResolver attachmentResolver = AttachmentResolver.createFromPart(context, rootPart); + AttachmentResolver attachmentResolver = AttachmentResolver.createFromPart(rootPart); return MessageViewInfo.createWithExtractedContent(message, rootPart, viewable.html, attachmentInfos, cryptoResultAnnotation, extraViewableText, extraAttachmentInfos, attachmentResolver); @@ -89,7 +89,7 @@ public class MessageViewInfoExtractor { MessageExtractor.findViewablesAndAttachments(part, viewableParts, attachments); } - attachmentInfos.addAll(attachmentInfoExtractor.extractAttachmentInfos(context, attachments)); + attachmentInfos.addAll(attachmentInfoExtractor.extractAttachmentInfos(attachments)); return MessageViewInfoExtractor.extractTextFromViewables(context, viewableParts); } diff --git a/k9mail/src/main/java/com/fsck/k9/message/extractors/AttachmentInfoExtractor.java b/k9mail/src/main/java/com/fsck/k9/message/extractors/AttachmentInfoExtractor.java index 9488ba486..ed1420434 100644 --- a/k9mail/src/main/java/com/fsck/k9/message/extractors/AttachmentInfoExtractor.java +++ b/k9mail/src/main/java/com/fsck/k9/message/extractors/AttachmentInfoExtractor.java @@ -12,6 +12,7 @@ import android.support.annotation.Nullable; import android.support.annotation.VisibleForTesting; import android.util.Log; +import com.fsck.k9.Globals; import com.fsck.k9.K9; import com.fsck.k9.mail.Body; import com.fsck.k9.mail.MessagingException; @@ -26,25 +27,30 @@ import com.fsck.k9.provider.DecryptedFileProvider; public class AttachmentInfoExtractor { - @VisibleForTesting - AttachmentInfoExtractor() { } + private final Context context; + public static AttachmentInfoExtractor getInstance() { - return new AttachmentInfoExtractor(); + Context context = Globals.getContext(); + return new AttachmentInfoExtractor(context); } - public List extractAttachmentInfos(Context context, List attachmentParts) - throws MessagingException { + @VisibleForTesting + AttachmentInfoExtractor(Context context) { + this.context = context; + } + + public List extractAttachmentInfos(List attachmentParts) throws MessagingException { List attachments = new ArrayList<>(); for (Part part : attachmentParts) { - attachments.add(extractAttachmentInfo(context, part)); + attachments.add(extractAttachmentInfo(part)); } return attachments; } - public AttachmentViewInfo extractAttachmentInfo(Context context, Part part) throws MessagingException { + public AttachmentViewInfo extractAttachmentInfo(Part part) throws MessagingException { Uri uri; long size; if (part instanceof LocalPart) { @@ -58,7 +64,7 @@ public class AttachmentInfoExtractor { if (body instanceof DeferredFileBody) { DeferredFileBody decryptedTempFileBody = (DeferredFileBody) body; size = decryptedTempFileBody.getSize(); - uri = getDecryptedFileProviderUri(context, decryptedTempFileBody, part.getMimeType()); + uri = getDecryptedFileProviderUri(decryptedTempFileBody, part.getMimeType()); return extractAttachmentInfo(part, uri, size); } else { throw new IllegalArgumentException("Unsupported part type provided"); @@ -70,7 +76,7 @@ public class AttachmentInfoExtractor { @Nullable @VisibleForTesting - protected Uri getDecryptedFileProviderUri(Context context, DeferredFileBody decryptedTempFileBody, String mimeType) { + protected Uri getDecryptedFileProviderUri(DeferredFileBody decryptedTempFileBody, String mimeType) { Uri uri; try { File file = decryptedTempFileBody.getFile(); diff --git a/k9mail/src/main/java/com/fsck/k9/ui/compose/QuotedMessagePresenter.java b/k9mail/src/main/java/com/fsck/k9/ui/compose/QuotedMessagePresenter.java index 2009f7732..56205d3b8 100644 --- a/k9mail/src/main/java/com/fsck/k9/ui/compose/QuotedMessagePresenter.java +++ b/k9mail/src/main/java/com/fsck/k9/ui/compose/QuotedMessagePresenter.java @@ -133,7 +133,7 @@ public class QuotedMessagePresenter { // Load the message with the reply header. TODO replace with MessageViewInfo data view.setQuotedHtml(quotedHtmlContent.getQuotedContent(), - AttachmentResolver.createFromPart(messageCompose, sourceMessage)); + AttachmentResolver.createFromPart(sourceMessage)); // TODO: Also strip the signature from the text/plain part view.setQuotedText(QuotedMessageHelper.quoteOriginalTextMessage(resources, sourceMessage, @@ -298,7 +298,7 @@ public class QuotedMessagePresenter { } // TODO replace with MessageViewInfo data view.setQuotedHtml(quotedHtmlContent.getQuotedContent(), - AttachmentResolver.createFromPart(messageCompose, message)); + AttachmentResolver.createFromPart(message)); } } if (bodyPlainOffset != null && bodyPlainLength != null) { @@ -404,4 +404,4 @@ public class QuotedMessagePresenter { return quotedTextFormat == SimpleMessageFormat.TEXT; } -} \ No newline at end of file +} diff --git a/k9mail/src/test/java/com/fsck/k9/mailstore/AttachmentResolverTest.java b/k9mail/src/test/java/com/fsck/k9/mailstore/AttachmentResolverTest.java index 1eea15cee..b2bd08572 100644 --- a/k9mail/src/test/java/com/fsck/k9/mailstore/AttachmentResolverTest.java +++ b/k9mail/src/test/java/com/fsck/k9/mailstore/AttachmentResolverTest.java @@ -1,8 +1,6 @@ package com.fsck.k9.mailstore; -import java.util.Arrays; -import java.util.Collections; import java.util.Map; import android.net.Uri; @@ -17,7 +15,6 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.robolectric.RobolectricTestRunner; -import org.robolectric.RuntimeEnvironment; import org.robolectric.annotation.Config; import static org.junit.Assert.*; @@ -46,8 +43,7 @@ public class AttachmentResolverTest { public void buildCidMap__onPartWithNoBody__shouldReturnEmptyMap() throws Exception { Part part = new MimeBodyPart(); - Map result = AttachmentResolver.buildCidToAttachmentUriMap( - RuntimeEnvironment.application, attachmentInfoExtractor, part); + Map result = AttachmentResolver.buildCidToAttachmentUriMap(attachmentInfoExtractor, part); assertTrue(result.isEmpty()); } @@ -57,8 +53,7 @@ public class AttachmentResolverTest { Multipart multipartBody = new MimeMultipart(); Part multipartPart = new MimeBodyPart(multipartBody); - Map result = AttachmentResolver.buildCidToAttachmentUriMap( - RuntimeEnvironment.application, attachmentInfoExtractor, multipartPart); + Map result = AttachmentResolver.buildCidToAttachmentUriMap(attachmentInfoExtractor, multipartPart); assertTrue(result.isEmpty()); } @@ -70,10 +65,7 @@ public class AttachmentResolverTest { Part multipartPart = new MimeBodyPart(multipartBody); multipartBody.addBodyPart(bodyPart); - - Map result = AttachmentResolver.buildCidToAttachmentUriMap( - RuntimeEnvironment.application, attachmentInfoExtractor, multipartPart); - + Map result = AttachmentResolver.buildCidToAttachmentUriMap(attachmentInfoExtractor, multipartPart); verify(bodyPart).getContentId(); assertTrue(result.isEmpty()); @@ -92,19 +84,16 @@ public class AttachmentResolverTest { when(subPart1.getContentId()).thenReturn("cid-1"); when(subPart2.getContentId()).thenReturn("cid-2"); - when(attachmentInfoExtractor.extractAttachmentInfo(RuntimeEnvironment.application, subPart1)).thenReturn( + when(attachmentInfoExtractor.extractAttachmentInfo(subPart1)).thenReturn( new AttachmentViewInfo(null, null, AttachmentViewInfo.UNKNOWN_SIZE, ATTACHMENT_TEST_URI_1, true, subPart1)); - when(attachmentInfoExtractor.extractAttachmentInfo(RuntimeEnvironment.application, subPart2)).thenReturn( + when(attachmentInfoExtractor.extractAttachmentInfo(subPart2)).thenReturn( new AttachmentViewInfo(null, null, AttachmentViewInfo.UNKNOWN_SIZE, ATTACHMENT_TEST_URI_2, true, subPart2)); - Map result = AttachmentResolver.buildCidToAttachmentUriMap(RuntimeEnvironment.application, - attachmentInfoExtractor, multipartPart); - + Map result = AttachmentResolver.buildCidToAttachmentUriMap(attachmentInfoExtractor, multipartPart); assertEquals(2, result.size()); assertEquals(ATTACHMENT_TEST_URI_1, result.get("cid-1")); assertEquals(ATTACHMENT_TEST_URI_2, result.get("cid-2")); } - -} \ No newline at end of file +} diff --git a/k9mail/src/test/java/com/fsck/k9/message/extractors/AttachmentInfoExtractorTest.java b/k9mail/src/test/java/com/fsck/k9/message/extractors/AttachmentInfoExtractorTest.java index b90dca38a..9abdb2a01 100644 --- a/k9mail/src/test/java/com/fsck/k9/message/extractors/AttachmentInfoExtractorTest.java +++ b/k9mail/src/test/java/com/fsck/k9/message/extractors/AttachmentInfoExtractorTest.java @@ -1,8 +1,6 @@ package com.fsck.k9.message.extractors; -import java.io.File; - import android.content.Context; import android.net.Uri; import android.support.annotation.Nullable; @@ -40,18 +38,20 @@ public class AttachmentInfoExtractorTest { private AttachmentInfoExtractor attachmentInfoExtractor; + private Context context; @Before public void setUp() throws Exception { - attachmentInfoExtractor = AttachmentInfoExtractor.getInstance(); + context = RuntimeEnvironment.application; + attachmentInfoExtractor = new AttachmentInfoExtractor(context); } @Test(expected = IllegalArgumentException.class) public void extractInfo__withGenericPart_shouldThrow() throws Exception { Part part = mock(Part.class); - attachmentInfoExtractor.extractAttachmentInfo(RuntimeEnvironment.application, part); + attachmentInfoExtractor.extractAttachmentInfo(part); } @Test @@ -62,8 +62,7 @@ public class AttachmentInfoExtractorTest { when(part.getSize()).thenReturn(TEST_SIZE); when(part.getAccountUuid()).thenReturn(TEST_ACCOUNT_UUID); - AttachmentViewInfo attachmentViewInfo = attachmentInfoExtractor.extractAttachmentInfo( - RuntimeEnvironment.application, part); + AttachmentViewInfo attachmentViewInfo = attachmentInfoExtractor.extractAttachmentInfo(part); assertEquals(AttachmentProvider.getAttachmentUri(TEST_ACCOUNT_UUID, TEST_ID), attachmentViewInfo.uri); assertEquals(TEST_SIZE, attachmentViewInfo.size); @@ -165,11 +164,10 @@ public class AttachmentInfoExtractorTest { @Test public void extractInfo__withDeferredFileBody() throws Exception { - attachmentInfoExtractor = new AttachmentInfoExtractor() { + attachmentInfoExtractor = new AttachmentInfoExtractor(context) { @Nullable @Override - protected Uri getDecryptedFileProviderUri(Context context, DeferredFileBody decryptedTempFileBody, - String mimeType) { + protected Uri getDecryptedFileProviderUri(DeferredFileBody decryptedTempFileBody, String mimeType) { return TEST_URI; } }; @@ -182,8 +180,7 @@ public class AttachmentInfoExtractorTest { when(body.getSize()).thenReturn(TEST_SIZE); - AttachmentViewInfo attachmentViewInfo = attachmentInfoExtractor.extractAttachmentInfo( - RuntimeEnvironment.application, part); + AttachmentViewInfo attachmentViewInfo = attachmentInfoExtractor.extractAttachmentInfo(part); assertEquals(TEST_URI, attachmentViewInfo.uri); @@ -191,4 +188,4 @@ public class AttachmentInfoExtractorTest { assertEquals(TEST_MIME_TYPE, attachmentViewInfo.mimeType); assertFalse(attachmentViewInfo.firstClassAttachment); } -} \ No newline at end of file +} From 8b719a3274b7c1c7879a1191c1fcf889cc18d387 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Fri, 15 Jul 2016 11:37:39 +0200 Subject: [PATCH 19/19] prefer real objects for Part in tests over mocks --- .../k9/mailstore/AttachmentResolverTest.java | 33 ++++++++----------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/k9mail/src/test/java/com/fsck/k9/mailstore/AttachmentResolverTest.java b/k9mail/src/test/java/com/fsck/k9/mailstore/AttachmentResolverTest.java index 2ebdd304f..1eea15cee 100644 --- a/k9mail/src/test/java/com/fsck/k9/mailstore/AttachmentResolverTest.java +++ b/k9mail/src/test/java/com/fsck/k9/mailstore/AttachmentResolverTest.java @@ -10,6 +10,8 @@ import android.net.Uri; import com.fsck.k9.mail.BodyPart; import com.fsck.k9.mail.Multipart; import com.fsck.k9.mail.Part; +import com.fsck.k9.mail.internet.MimeBodyPart; +import com.fsck.k9.mail.internet.MimeMultipart; import com.fsck.k9.message.extractors.AttachmentInfoExtractor; import org.junit.Before; import org.junit.Test; @@ -42,8 +44,7 @@ public class AttachmentResolverTest { @Test public void buildCidMap__onPartWithNoBody__shouldReturnEmptyMap() throws Exception { - Part part = mock(Part.class); - when(part.getContentId()).thenReturn(null); + Part part = new MimeBodyPart(); Map result = AttachmentResolver.buildCidToAttachmentUriMap( RuntimeEnvironment.application, attachmentInfoExtractor, part); @@ -53,10 +54,8 @@ public class AttachmentResolverTest { @Test public void buildCidMap__onMultipartWithNoParts__shouldReturnEmptyMap() throws Exception { - Part multipartPart = mock(Part.class); - Multipart multipartBody = mock(Multipart.class); - when(multipartPart.getBody()).thenReturn(multipartBody); - when(multipartBody.getBodyParts()).thenReturn(Collections.EMPTY_LIST); + Multipart multipartBody = new MimeMultipart(); + Part multipartPart = new MimeBodyPart(multipartBody); Map result = AttachmentResolver.buildCidToAttachmentUriMap( RuntimeEnvironment.application, attachmentInfoExtractor, multipartPart); @@ -66,13 +65,10 @@ public class AttachmentResolverTest { @Test public void buildCidMap__onMultipartWithEmptyBodyPart__shouldReturnEmptyMap() throws Exception { - Part multipartPart = mock(Part.class); - Multipart multipartBody = mock(Multipart.class); + Multipart multipartBody = new MimeMultipart(); BodyPart bodyPart = mock(BodyPart.class); - - when(multipartPart.getBody()).thenReturn(multipartBody); - when(multipartBody.getBodyParts()).thenReturn(Collections.singletonList(bodyPart)); - when(bodyPart.getContentId()).thenReturn(null); + Part multipartPart = new MimeBodyPart(multipartBody); + multipartBody.addBodyPart(bodyPart); Map result = AttachmentResolver.buildCidToAttachmentUriMap( @@ -85,15 +81,14 @@ public class AttachmentResolverTest { @Test public void buildCidMap__onTwoPart__shouldReturnBothUris() throws Exception { - Part multipartPart = mock(Part.class); - Multipart multipartBody = mock(Multipart.class); - BodyPart bodyPart = mock(BodyPart.class); - - when(multipartPart.getBody()).thenReturn(multipartBody); - when(multipartBody.getBodyParts()).thenReturn(Collections.singletonList(bodyPart)); + Multipart multipartBody = new MimeMultipart(); + Part multipartPart = new MimeBodyPart(multipartBody); BodyPart subPart1 = mock(BodyPart.class); BodyPart subPart2 = mock(BodyPart.class); + multipartBody.addBodyPart(subPart1); + multipartBody.addBodyPart(subPart2); + when(subPart1.getContentId()).thenReturn("cid-1"); when(subPart2.getContentId()).thenReturn("cid-2"); @@ -102,11 +97,11 @@ public class AttachmentResolverTest { when(attachmentInfoExtractor.extractAttachmentInfo(RuntimeEnvironment.application, subPart2)).thenReturn( new AttachmentViewInfo(null, null, AttachmentViewInfo.UNKNOWN_SIZE, ATTACHMENT_TEST_URI_2, true, subPart2)); - when(multipartBody.getBodyParts()).thenReturn(Arrays.asList(subPart1, subPart2)); Map result = AttachmentResolver.buildCidToAttachmentUriMap(RuntimeEnvironment.application, attachmentInfoExtractor, multipartPart); + assertEquals(2, result.size()); assertEquals(ATTACHMENT_TEST_URI_1, result.get("cid-1")); assertEquals(ATTACHMENT_TEST_URI_2, result.get("cid-2"));