diff --git a/briar-android/src/androidTestOfficial/java/org/briarproject/briar/android/attachment/AttachmentRetrieverIntegrationTest.java b/briar-android/src/androidTestOfficial/java/org/briarproject/briar/android/attachment/AttachmentRetrieverIntegrationTest.java index eea2427f3..9881c74fb 100644 --- a/briar-android/src/androidTestOfficial/java/org/briarproject/briar/android/attachment/AttachmentRetrieverIntegrationTest.java +++ b/briar-android/src/androidTestOfficial/java/org/briarproject/briar/android/attachment/AttachmentRetrieverIntegrationTest.java @@ -54,8 +54,8 @@ public class AttachmentRetrieverIntegrationTest { public void testSmallJpegImage() throws Exception { AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg"); InputStream is = getUrlInputStream(smallKitten); - Attachment a = new Attachment(is); - AttachmentItem item = retriever.getAttachmentItem(h, a, true); + Attachment a = new Attachment(h, is); + AttachmentItem item = retriever.getAttachmentItem(a, true); assertEquals(msgId, item.getMessageId()); assertEquals(160, item.getWidth()); assertEquals(240, item.getHeight()); @@ -70,8 +70,8 @@ public class AttachmentRetrieverIntegrationTest { public void testBigJpegImage() throws Exception { AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg"); InputStream is = getUrlInputStream(originalKitten); - Attachment a = new Attachment(is); - AttachmentItem item = retriever.getAttachmentItem(h, a, true); + Attachment a = new Attachment(h, is); + AttachmentItem item = retriever.getAttachmentItem(a, true); assertEquals(msgId, item.getMessageId()); assertEquals(1728, item.getWidth()); assertEquals(2592, item.getHeight()); @@ -86,8 +86,8 @@ public class AttachmentRetrieverIntegrationTest { public void testSmallPngImage() throws Exception { AttachmentHeader h = new AttachmentHeader(msgId, "image/png"); InputStream is = getUrlInputStream(pngKitten); - Attachment a = new Attachment(is); - AttachmentItem item = retriever.getAttachmentItem(h, a, true); + Attachment a = new Attachment(h, is); + AttachmentItem item = retriever.getAttachmentItem(a, true); assertEquals(msgId, item.getMessageId()); assertEquals(737, item.getWidth()); assertEquals(510, item.getHeight()); @@ -102,8 +102,8 @@ public class AttachmentRetrieverIntegrationTest { public void testUberGif() throws Exception { AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg"); InputStream is = getUrlInputStream(uberGif); - Attachment a = new Attachment(is); - AttachmentItem item = retriever.getAttachmentItem(h, a, true); + Attachment a = new Attachment(h, is); + AttachmentItem item = retriever.getAttachmentItem(a, true); assertEquals(1, item.getWidth()); assertEquals(1, item.getHeight()); assertEquals(dimensions.minHeight, item.getThumbnailWidth()); @@ -117,8 +117,8 @@ public class AttachmentRetrieverIntegrationTest { public void testLottaPixels() throws Exception { AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg"); InputStream is = getUrlInputStream(lottaPixel); - Attachment a = new Attachment(is); - AttachmentItem item = retriever.getAttachmentItem(h, a, true); + Attachment a = new Attachment(h, is); + AttachmentItem item = retriever.getAttachmentItem(a, true); assertEquals(64250, item.getWidth()); assertEquals(64250, item.getHeight()); assertEquals(dimensions.maxWidth, item.getThumbnailWidth()); @@ -132,8 +132,8 @@ public class AttachmentRetrieverIntegrationTest { public void testImageIoCrash() throws Exception { AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg"); InputStream is = getUrlInputStream(imageIoCrash); - Attachment a = new Attachment(is); - AttachmentItem item = retriever.getAttachmentItem(h, a, true); + Attachment a = new Attachment(h, is); + AttachmentItem item = retriever.getAttachmentItem(a, true); assertEquals(1184, item.getWidth()); assertEquals(448, item.getHeight()); assertEquals(dimensions.maxWidth, item.getThumbnailWidth()); @@ -147,8 +147,8 @@ public class AttachmentRetrieverIntegrationTest { public void testGimpCrash() throws Exception { AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg"); InputStream is = getUrlInputStream(gimpCrash); - Attachment a = new Attachment(is); - AttachmentItem item = retriever.getAttachmentItem(h, a, true); + Attachment a = new Attachment(h, is); + AttachmentItem item = retriever.getAttachmentItem(a, true); assertEquals(1, item.getWidth()); assertEquals(1, item.getHeight()); assertEquals(dimensions.minHeight, item.getThumbnailWidth()); @@ -162,8 +162,8 @@ public class AttachmentRetrieverIntegrationTest { public void testOptiPngAfl() throws Exception { AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg"); InputStream is = getUrlInputStream(optiPngAfl); - Attachment a = new Attachment(is); - AttachmentItem item = retriever.getAttachmentItem(h, a, true); + Attachment a = new Attachment(h, is); + AttachmentItem item = retriever.getAttachmentItem(a, true); assertEquals(32, item.getWidth()); assertEquals(32, item.getHeight()); assertEquals(dimensions.minHeight, item.getThumbnailWidth()); @@ -177,8 +177,8 @@ public class AttachmentRetrieverIntegrationTest { public void testLibrawError() throws Exception { AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg"); InputStream is = getUrlInputStream(librawError); - Attachment a = new Attachment(is); - AttachmentItem item = retriever.getAttachmentItem(h, a, true); + Attachment a = new Attachment(h, is); + AttachmentItem item = retriever.getAttachmentItem(a, true); assertTrue(item.hasError()); } @@ -186,8 +186,8 @@ public class AttachmentRetrieverIntegrationTest { public void testSmallAnimatedGifMaxDimensions() throws Exception { AttachmentHeader h = new AttachmentHeader(msgId, "image/gif"); InputStream is = getAssetInputStream("animated.gif"); - Attachment a = new Attachment(is); - AttachmentItem item = retriever.getAttachmentItem(h, a, true); + Attachment a = new Attachment(h, is); + AttachmentItem item = retriever.getAttachmentItem(a, true); assertEquals(65535, item.getWidth()); assertEquals(65535, item.getHeight()); assertEquals(dimensions.maxWidth, item.getThumbnailWidth()); @@ -201,8 +201,8 @@ public class AttachmentRetrieverIntegrationTest { public void testSmallAnimatedGifHugeDimensions() throws Exception { AttachmentHeader h = new AttachmentHeader(msgId, "image/gif"); InputStream is = getAssetInputStream("animated2.gif"); - Attachment a = new Attachment(is); - AttachmentItem item = retriever.getAttachmentItem(h, a, true); + Attachment a = new Attachment(h, is); + AttachmentItem item = retriever.getAttachmentItem(a, true); assertEquals(10000, item.getWidth()); assertEquals(10000, item.getHeight()); assertEquals(dimensions.maxWidth, item.getThumbnailWidth()); @@ -216,8 +216,8 @@ public class AttachmentRetrieverIntegrationTest { public void testSmallGifLargeDimensions() throws Exception { AttachmentHeader h = new AttachmentHeader(msgId, "image/gif"); InputStream is = getAssetInputStream("error_large.gif"); - Attachment a = new Attachment(is); - AttachmentItem item = retriever.getAttachmentItem(h, a, true); + Attachment a = new Attachment(h, is); + AttachmentItem item = retriever.getAttachmentItem(a, true); assertEquals(16384, item.getWidth()); assertEquals(16384, item.getHeight()); assertEquals(dimensions.maxWidth, item.getThumbnailWidth()); @@ -231,8 +231,8 @@ public class AttachmentRetrieverIntegrationTest { public void testHighError() throws Exception { AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg"); InputStream is = getAssetInputStream("error_high.jpg"); - Attachment a = new Attachment(is); - AttachmentItem item = retriever.getAttachmentItem(h, a, true); + Attachment a = new Attachment(h, is); + AttachmentItem item = retriever.getAttachmentItem(a, true); assertEquals(1, item.getWidth()); assertEquals(10000, item.getHeight()); assertEquals(dimensions.minWidth, item.getThumbnailWidth()); @@ -246,8 +246,8 @@ public class AttachmentRetrieverIntegrationTest { public void testWideError() throws Exception { AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg"); InputStream is = getAssetInputStream("error_wide.jpg"); - Attachment a = new Attachment(is); - AttachmentItem item = retriever.getAttachmentItem(h, a, true); + Attachment a = new Attachment(h, is); + AttachmentItem item = retriever.getAttachmentItem(a, true); assertEquals(1920, item.getWidth()); assertEquals(1, item.getHeight()); assertEquals(dimensions.maxWidth, item.getThumbnailWidth()); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java index c22d46024..fce645c65 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java @@ -98,7 +98,7 @@ public class AttachmentCreator { // get and cache AttachmentItem for ImagePreview try { Attachment a = retriever.getMessageAttachment(h); - AttachmentItem item = retriever.getAttachmentItem(h, a, needsSize); + AttachmentItem item = retriever.getAttachmentItem(a, needsSize); if (item.hasError()) throw new IOException(); AttachmentItemResult itemResult = new AttachmentItemResult(uri, item); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentItem.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentItem.java index 8caafac7c..43410db0f 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentItem.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentItem.java @@ -68,7 +68,7 @@ public class AttachmentItem implements Parcelable { header = new AttachmentHeader(messageId, mimeType); } - AttachmentHeader getHeader() { + public AttachmentHeader getHeader() { return header; } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentRetriever.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentRetriever.java index d3fb1188b..31ba87e4a 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentRetriever.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentRetriever.java @@ -9,8 +9,6 @@ import android.webkit.MimeTypeMap; import com.bumptech.glide.util.MarkEnforcingInputStream; -import org.briarproject.bramble.api.Pair; -import org.briarproject.bramble.api.db.DatabaseExecutor; import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.sync.MessageId; @@ -22,7 +20,6 @@ import org.briarproject.briar.api.messaging.MessagingManager; import java.io.BufferedInputStream; import java.io.IOException; import java.io.InputStream; -import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; @@ -38,9 +35,7 @@ import static android.support.media.ExifInterface.TAG_ORIENTATION; import static java.util.logging.Level.WARNING; import static java.util.logging.Logger.getLogger; import static org.briarproject.bramble.util.IoUtils.tryToClose; -import static org.briarproject.bramble.util.LogUtils.logDuration; import static org.briarproject.bramble.util.LogUtils.logException; -import static org.briarproject.bramble.util.LogUtils.now; @NotNullByDefault public class AttachmentRetriever { @@ -93,7 +88,8 @@ public class AttachmentRetriever { }); } - public void cachePut(MessageId messageId, List attachments) { + public void cachePut(MessageId messageId, + List attachments) { attachmentCache.put(messageId, attachments); } @@ -102,48 +98,17 @@ public class AttachmentRetriever { return attachmentCache.get(messageId); } - @DatabaseExecutor - public List> getMessageAttachments( - List headers) throws DbException { - long start = now(); - List> attachments = - new ArrayList<>(headers.size()); - for (AttachmentHeader h : headers) { - Attachment a = messagingManager.getAttachment(h.getMessageId()); - attachments.add(new Pair<>(h, a)); - } - logDuration(LOG, "Loading attachments", start); - return attachments; - } - - @DatabaseExecutor - Attachment getMessageAttachment(AttachmentHeader h) throws DbException { - return messagingManager.getAttachment(h.getMessageId()); - } - - /** - * Creates {@link AttachmentItem}s from the passed headers and Attachments. - *

- * Note: This closes the {@link Attachment}'s {@link InputStream}. - */ - public List getAttachmentItems( - List> attachments) { - boolean needsSize = attachments.size() == 1; - List items = new ArrayList<>(attachments.size()); - for (Pair a : attachments) { - AttachmentItem item = - getAttachmentItem(a.getFirst(), a.getSecond(), needsSize); - items.add(item); - } - return items; + public Attachment getMessageAttachment(AttachmentHeader h) + throws DbException { + return messagingManager.getAttachment(h); } /** * Creates an {@link AttachmentItem} from the {@link Attachment}'s * {@link InputStream} which will be closed when this method returns. */ - AttachmentItem getAttachmentItem(AttachmentHeader h, Attachment a, - boolean needsSize) { + public AttachmentItem getAttachmentItem(Attachment a, boolean needsSize) { + AttachmentHeader h = a.getHeader(); if (!needsSize) { String extension = imageHelper.getExtensionFromMimeType(h.getContentType()); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationActivity.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationActivity.java index 52cc1a19a..d983479b2 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationActivity.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationActivity.java @@ -37,6 +37,7 @@ import org.briarproject.bramble.api.contact.event.ContactRemovedEvent; import org.briarproject.bramble.api.db.DatabaseExecutor; import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.db.NoSuchContactException; +import org.briarproject.bramble.api.db.NoSuchMessageException; import org.briarproject.bramble.api.event.Event; import org.briarproject.bramble.api.event.EventBus; import org.briarproject.bramble.api.event.EventListener; @@ -81,6 +82,7 @@ import org.briarproject.briar.api.messaging.Attachment; import org.briarproject.briar.api.messaging.AttachmentHeader; import org.briarproject.briar.api.messaging.MessagingManager; import org.briarproject.briar.api.messaging.PrivateMessageHeader; +import org.briarproject.briar.api.messaging.event.AttachmentReceivedEvent; import org.briarproject.briar.api.privategroup.invitation.GroupInvitationManager; import java.util.ArrayList; @@ -107,10 +109,12 @@ import static android.support.v7.util.SortedList.INVALID_POSITION; import static android.view.Gravity.RIGHT; import static android.widget.Toast.LENGTH_SHORT; import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; import static java.util.Collections.sort; import static java.util.Objects.requireNonNull; import static java.util.logging.Level.INFO; import static java.util.logging.Level.WARNING; +import static java.util.logging.Logger.getLogger; import static org.briarproject.bramble.util.LogUtils.logDuration; import static org.briarproject.bramble.util.LogUtils.logException; import static org.briarproject.bramble.util.LogUtils.now; @@ -138,7 +142,7 @@ public class ConversationActivity extends BriarActivity public static final String CONTACT_ID = "briar.CONTACT_ID"; private static final Logger LOG = - Logger.getLogger(ConversationActivity.class.getName()); + getLogger(ConversationActivity.class.getName()); private static final int TRANSITION_DURATION_MS = 500; private static final int ONBOARDING_DELAY_MS = 250; @@ -171,6 +175,8 @@ public class ConversationActivity extends BriarActivity volatile GroupInvitationManager groupInvitationManager; private final Map textCache = new ConcurrentHashMap<>(); + private final Map missingAttachments = + new ConcurrentHashMap<>(); private final Observer contactNameObserver = name -> { requireNonNull(name); loadMessages(); @@ -434,29 +440,40 @@ public class ConversationActivity extends BriarActivity }); } - private void eagerlyLoadMessageSize(PrivateMessageHeader h) - throws DbException { - MessageId id = h.getId(); - // If the message has text, load it - if (h.hasText()) { - String text = textCache.get(id); - if (text == null) { - LOG.info("Eagerly loading text for latest message"); - text = messagingManager.getMessageText(id); - textCache.put(id, requireNonNull(text)); + private void eagerlyLoadMessageSize(PrivateMessageHeader h) { + try { + MessageId id = h.getId(); + // If the message has text, load it + if (h.hasText()) { + String text = textCache.get(id); + if (text == null) { + LOG.info("Eagerly loading text for latest message"); + text = messagingManager.getMessageText(id); + textCache.put(id, requireNonNull(text)); + } } - } - // If the message has a single image, load its size - for multiple - // images we use a grid so the size is fixed - if (h.getAttachmentHeaders().size() == 1) { - List items = attachmentRetriever.cacheGet(id); - if (items == null) { - LOG.info("Eagerly loading image size for latest message"); - items = attachmentRetriever.getAttachmentItems( - attachmentRetriever.getMessageAttachments( - h.getAttachmentHeaders())); - attachmentRetriever.cachePut(id, items); + // If the message has a single image, load its size - for multiple + // images we use a grid so the size is fixed + List headers = h.getAttachmentHeaders(); + if (headers.size() == 1) { + List items = attachmentRetriever.cacheGet(id); + if (items == null) { + LOG.info("Eagerly loading image size for latest message"); + AttachmentHeader header = headers.get(0); + try { + Attachment a = attachmentRetriever + .getMessageAttachment(header); + AttachmentItem item = + attachmentRetriever.getAttachmentItem(a, true); + attachmentRetriever.cachePut(id, singletonList(item)); + } catch (NoSuchMessageException e) { + LOG.info("Attachment not received yet"); + missingAttachments.put(header.getMessageId(), h); + } + } } + } catch (DbException e) { + logException(LOG, WARNING, e); } } @@ -534,16 +551,30 @@ public class ConversationActivity extends BriarActivity && adapter.isScrolledToBottom(layoutManager); } - private void loadMessageAttachments(MessageId messageId, - List headers) { + private void loadMessageAttachments(PrivateMessageHeader h) { + // TODO: Use placeholders for missing/invalid attachments runOnDbThread(() -> { try { - List> attachments = - attachmentRetriever.getMessageAttachments(headers); // TODO move getting the items off to IoExecutor, if size == 1 - List items = - attachmentRetriever.getAttachmentItems(attachments); - displayMessageAttachments(messageId, items); + List headers = h.getAttachmentHeaders(); + boolean needsSize = headers.size() == 1; + List items = new ArrayList<>(headers.size()); + for (AttachmentHeader header : headers) { + try { + Attachment a = attachmentRetriever + .getMessageAttachment(header); + AttachmentItem item = attachmentRetriever + .getAttachmentItem(a, needsSize); + items.add(item); + } catch (NoSuchMessageException e) { + LOG.info("Attachment not received yet"); + missingAttachments.put(header.getMessageId(), h); + return; + } + } + // Don't cache items unless all are present and valid + attachmentRetriever.cachePut(h.getId(), items); + displayMessageAttachments(h.getId(), items); } catch (DbException e) { logException(LOG, WARNING, e); } @@ -553,7 +584,6 @@ public class ConversationActivity extends BriarActivity private void displayMessageAttachments(MessageId m, List items) { runOnUiThreadUnlessDestroyed(() -> { - attachmentRetriever.cachePut(m, items); Pair pair = adapter.getMessageItem(m); if (pair != null) { @@ -567,6 +597,13 @@ public class ConversationActivity extends BriarActivity @Override public void eventOccurred(Event e) { + if (e instanceof AttachmentReceivedEvent) { + AttachmentReceivedEvent a = (AttachmentReceivedEvent) e; + if (a.getContactId().equals(contactId)) { + LOG.info("Attachment received"); + onAttachmentReceived(a.getMessageId()); + } + } if (e instanceof ContactRemovedEvent) { ContactRemovedEvent c = (ContactRemovedEvent) e; if (c.getContactId().equals(contactId)) { @@ -617,6 +654,15 @@ public class ConversationActivity extends BriarActivity scrollToBottom(); } + @UiThread + private void onAttachmentReceived(MessageId attachmentId) { + PrivateMessageHeader h = missingAttachments.remove(attachmentId); + if (h != null) { + LOG.info("Missing attachment received"); + loadMessageAttachments(h); + } + } + @UiThread private void onNewConversationMessage(ConversationMessageHeader h) { if (h instanceof ConversationRequest || @@ -903,11 +949,11 @@ public class ConversationActivity extends BriarActivity } @Override - public List getAttachmentItems(MessageId m, - List headers) { - List attachments = attachmentRetriever.cacheGet(m); + public List getAttachmentItems(PrivateMessageHeader h) { + List attachments = + attachmentRetriever.cacheGet(h.getId()); if (attachments == null) { - loadMessageAttachments(m, headers); + loadMessageAttachments(h); return emptyList(); } return attachments; diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationVisitor.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationVisitor.java index 48f39e666..c7d8fc9a4 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationVisitor.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationVisitor.java @@ -15,7 +15,6 @@ import org.briarproject.briar.api.forum.ForumInvitationRequest; import org.briarproject.briar.api.forum.ForumInvitationResponse; import org.briarproject.briar.api.introduction.IntroductionRequest; import org.briarproject.briar.api.introduction.IntroductionResponse; -import org.briarproject.briar.api.messaging.AttachmentHeader; import org.briarproject.briar.api.messaging.PrivateMessageHeader; import org.briarproject.briar.api.privategroup.invitation.GroupInvitationRequest; import org.briarproject.briar.api.privategroup.invitation.GroupInvitationResponse; @@ -56,8 +55,7 @@ class ConversationVisitor implements if (h.getAttachmentHeaders().isEmpty()) { attachments = emptyList(); } else { - attachments = attachmentCache - .getAttachmentItems(h.getId(), h.getAttachmentHeaders()); + attachments = attachmentCache.getAttachmentItems(h); } if (h.isLocal()) { item = new ConversationMessageItem( @@ -295,7 +293,6 @@ class ConversationVisitor implements } interface AttachmentCache { - List getAttachmentItems(MessageId m, - List headers); + List getAttachmentItems(PrivateMessageHeader h); } } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageViewModel.java index 33f29d3e3..7218efa5f 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageViewModel.java @@ -12,7 +12,6 @@ import org.briarproject.bramble.api.db.DatabaseExecutor; import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.lifecycle.IoExecutor; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; -import org.briarproject.bramble.api.sync.MessageId; import org.briarproject.briar.android.attachment.AttachmentItem; import org.briarproject.briar.android.viewmodel.LiveEvent; import org.briarproject.briar.android.viewmodel.MutableLiveEvent; @@ -135,10 +134,10 @@ public class ImageViewModel extends AndroidViewModel { private void saveImage(AttachmentItem attachment, OutputStreamProvider osp, @Nullable Runnable afterCopy) { - MessageId messageId = attachment.getMessageId(); dbExecutor.execute(() -> { try { - Attachment a = messagingManager.getAttachment(messageId); + Attachment a = + messagingManager.getAttachment(attachment.getHeader()); copyImageFromDb(a, osp, afterCopy); } catch (DbException e) { logException(LOG, WARNING, e); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/glide/BriarDataFetcher.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/glide/BriarDataFetcher.java index 1394f8035..852b8c536 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/glide/BriarDataFetcher.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/glide/BriarDataFetcher.java @@ -9,8 +9,8 @@ import com.bumptech.glide.load.data.DataFetcher; import org.briarproject.bramble.api.db.DatabaseExecutor; import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; -import org.briarproject.bramble.api.sync.MessageId; import org.briarproject.briar.android.attachment.AttachmentItem; +import org.briarproject.briar.api.messaging.Attachment; import org.briarproject.briar.api.messaging.MessagingManager; import java.io.InputStream; @@ -50,11 +50,12 @@ class BriarDataFetcher implements DataFetcher { @Override public void loadData(Priority priority, DataCallback callback) { - MessageId id = attachment.getMessageId(); dbExecutor.execute(() -> { if (cancel) return; try { - inputStream = messagingManager.getAttachment(id).getStream(); + Attachment a = + messagingManager.getAttachment(attachment.getHeader()); + inputStream = a.getStream(); callback.onDataReady(inputStream); } catch (DbException e) { callback.onLoadFailed(e); diff --git a/briar-android/src/test/java/org/briarproject/briar/android/attachment/AttachmentRetrieverTest.java b/briar-android/src/test/java/org/briarproject/briar/android/attachment/AttachmentRetrieverTest.java index f0f38bdb1..67670e846 100644 --- a/briar-android/src/test/java/org/briarproject/briar/android/attachment/AttachmentRetrieverTest.java +++ b/briar-android/src/test/java/org/briarproject/briar/android/attachment/AttachmentRetrieverTest.java @@ -9,7 +9,6 @@ import org.briarproject.briar.api.messaging.MessagingManager; import org.jmock.Expectations; import org.junit.Test; -import java.io.BufferedInputStream; import java.io.ByteArrayInputStream; import java.io.InputStream; @@ -25,32 +24,26 @@ public class AttachmentRetrieverTest extends BrambleMockTestCase { 100, 50, 200, 75, 300 ); private final MessageId msgId = new MessageId(getRandomId()); - private final Attachment attachment = new Attachment( - new BufferedInputStream( - new ByteArrayInputStream(getRandomBytes(42)))); - private final MessagingManager messagingManager = context.mock(MessagingManager.class); private final ImageHelper imageHelper = context.mock(ImageHelper.class); - private final AttachmentRetriever controller = - new AttachmentRetriever( - messagingManager, - dimensions, - imageHelper - ); + private final AttachmentRetriever retriever = new AttachmentRetriever( + messagingManager, + dimensions, + imageHelper + ); @Test public void testNoSize() { String mimeType = "image/jpeg"; - AttachmentHeader h = getAttachmentHeader(mimeType); + Attachment attachment = getAttachment(mimeType); context.checking(new Expectations() {{ oneOf(imageHelper).getExtensionFromMimeType(mimeType); will(returnValue("jpg")); }}); - AttachmentItem item = - controller.getAttachmentItem(h, attachment, false); + AttachmentItem item = retriever.getAttachmentItem(attachment, false); assertEquals(mimeType, item.getMimeType()); assertEquals("jpg", item.getExtension()); assertFalse(item.hasError()); @@ -59,22 +52,21 @@ public class AttachmentRetrieverTest extends BrambleMockTestCase { @Test public void testNoSizeWrongMimeTypeProducesError() { String mimeType = "application/octet-stream"; - AttachmentHeader h = getAttachmentHeader(mimeType); + Attachment attachment = getAttachment(mimeType); context.checking(new Expectations() {{ oneOf(imageHelper).getExtensionFromMimeType(mimeType); will(returnValue(null)); }}); - AttachmentItem item = - controller.getAttachmentItem(h, attachment, false); + AttachmentItem item = retriever.getAttachmentItem(attachment, false); assertTrue(item.hasError()); } @Test public void testSmallJpegImage() { String mimeType = "image/jpeg"; - AttachmentHeader h = getAttachmentHeader(mimeType); + Attachment attachment = getAttachment(mimeType); context.checking(new Expectations() {{ oneOf(imageHelper).decodeStream(with(any(InputStream.class))); @@ -83,7 +75,7 @@ public class AttachmentRetrieverTest extends BrambleMockTestCase { will(returnValue("jpg")); }}); - AttachmentItem item = controller.getAttachmentItem(h, attachment, true); + AttachmentItem item = retriever.getAttachmentItem(attachment, true); assertEquals(msgId, item.getMessageId()); assertEquals(160, item.getWidth()); assertEquals(240, item.getHeight()); @@ -97,7 +89,7 @@ public class AttachmentRetrieverTest extends BrambleMockTestCase { @Test public void testBigJpegImage() { String mimeType = "image/jpeg"; - AttachmentHeader h = getAttachmentHeader(mimeType); + Attachment attachment = getAttachment(mimeType); context.checking(new Expectations() {{ oneOf(imageHelper).decodeStream(with(any(InputStream.class))); @@ -106,7 +98,7 @@ public class AttachmentRetrieverTest extends BrambleMockTestCase { will(returnValue("jpg")); }}); - AttachmentItem item = controller.getAttachmentItem(h, attachment, true); + AttachmentItem item = retriever.getAttachmentItem(attachment, true); assertEquals(1728, item.getWidth()); assertEquals(2592, item.getHeight()); assertEquals(dimensions.maxWidth, item.getThumbnailWidth()); @@ -116,7 +108,7 @@ public class AttachmentRetrieverTest extends BrambleMockTestCase { @Test public void testMalformedError() { - AttachmentHeader h = getAttachmentHeader("image/jpeg"); + Attachment attachment = getAttachment("image/jpeg"); context.checking(new Expectations() {{ oneOf(imageHelper).decodeStream(with(any(InputStream.class))); @@ -125,12 +117,13 @@ public class AttachmentRetrieverTest extends BrambleMockTestCase { will(returnValue(null)); }}); - AttachmentItem item = controller.getAttachmentItem(h, attachment, true); + AttachmentItem item = retriever.getAttachmentItem(attachment, true); assertTrue(item.hasError()); } - private AttachmentHeader getAttachmentHeader(String contentType) { - return new AttachmentHeader(msgId, contentType); + private Attachment getAttachment(String contentType) { + AttachmentHeader header = new AttachmentHeader(msgId, contentType); + InputStream in = new ByteArrayInputStream(getRandomBytes(42)); + return new Attachment(header, in); } - } diff --git a/briar-api/src/main/java/org/briarproject/briar/api/messaging/Attachment.java b/briar-api/src/main/java/org/briarproject/briar/api/messaging/Attachment.java index 526beced1..61bb0b0eb 100644 --- a/briar-api/src/main/java/org/briarproject/briar/api/messaging/Attachment.java +++ b/briar-api/src/main/java/org/briarproject/briar/api/messaging/Attachment.java @@ -1,15 +1,27 @@ package org.briarproject.briar.api.messaging; +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; + import java.io.InputStream; +import javax.annotation.concurrent.Immutable; + +@Immutable +@NotNullByDefault public class Attachment { + private final AttachmentHeader header; private final InputStream stream; - public Attachment(InputStream stream) { + public Attachment(AttachmentHeader header, InputStream stream) { + this.header = header; this.stream = stream; } + public AttachmentHeader getHeader() { + return header; + } + public InputStream getStream() { return stream; } diff --git a/briar-api/src/main/java/org/briarproject/briar/api/messaging/InvalidAttachmentException.java b/briar-api/src/main/java/org/briarproject/briar/api/messaging/InvalidAttachmentException.java new file mode 100644 index 000000000..df32602d1 --- /dev/null +++ b/briar-api/src/main/java/org/briarproject/briar/api/messaging/InvalidAttachmentException.java @@ -0,0 +1,14 @@ +package org.briarproject.briar.api.messaging; + +import org.briarproject.bramble.api.db.DbException; +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; + +/** + * An exception that is thrown when an {@link AttachmentHeader} is used to + * load an {@link Attachment}, and the header refers to a message that is not + * an attachment, or to an attachment that does not have the expected content + * type. + */ +@NotNullByDefault +public class InvalidAttachmentException extends DbException { +} diff --git a/briar-api/src/main/java/org/briarproject/briar/api/messaging/MessagingManager.java b/briar-api/src/main/java/org/briarproject/briar/api/messaging/MessagingManager.java index 6352047fc..d4c6074d4 100644 --- a/briar-api/src/main/java/org/briarproject/briar/api/messaging/MessagingManager.java +++ b/briar-api/src/main/java/org/briarproject/briar/api/messaging/MessagingManager.java @@ -40,7 +40,7 @@ public interface MessagingManager extends ConversationClient { /** * Stores a local attachment message. * - * @throws FileTooBigException + * @throws FileTooBigException If the attachment is too big */ AttachmentHeader addLocalAttachment(GroupId groupId, long timestamp, String contentType, InputStream is) throws DbException, IOException; @@ -68,9 +68,13 @@ public interface MessagingManager extends ConversationClient { String getMessageText(MessageId m) throws DbException; /** - * Returns the attachment with the given ID. + * Returns the attachment with the given message ID and content type. + * + * @throws InvalidAttachmentException If the header refers to a message + * that is not an attachment, or to an attachment that does not have the + * expected content type */ - Attachment getAttachment(MessageId m) throws DbException; + Attachment getAttachment(AttachmentHeader h) throws DbException; /** * Returns true if the contact with the given {@link ContactId} does support diff --git a/briar-core/src/main/java/org/briarproject/briar/messaging/MessagingManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/messaging/MessagingManagerImpl.java index 2fbbab6ac..c85a2a22d 100644 --- a/briar-core/src/main/java/org/briarproject/briar/messaging/MessagingManagerImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/messaging/MessagingManagerImpl.java @@ -32,6 +32,7 @@ import org.briarproject.briar.api.conversation.ConversationMessageHeader; import org.briarproject.briar.api.messaging.Attachment; import org.briarproject.briar.api.messaging.AttachmentHeader; import org.briarproject.briar.api.messaging.FileTooBigException; +import org.briarproject.briar.api.messaging.InvalidAttachmentException; import org.briarproject.briar.api.messaging.MessagingManager; import org.briarproject.briar.api.messaging.PrivateMessage; import org.briarproject.briar.api.messaging.PrivateMessageHeader; @@ -374,13 +375,20 @@ class MessagingManagerImpl implements MessagingManager, IncomingMessageHook, } @Override - public Attachment getAttachment(MessageId m) throws DbException { + public Attachment getAttachment(AttachmentHeader h) throws DbException { // TODO: Support large messages + MessageId m = h.getMessageId(); byte[] body = clientHelper.getMessage(m).getBody(); try { BdfDictionary meta = clientHelper.getMessageMetadataAsDictionary(m); + Long messageType = meta.getOptionalLong(MSG_KEY_MSG_TYPE); + if (messageType == null || messageType != ATTACHMENT) + throw new InvalidAttachmentException(); + String contentType = meta.getString(MSG_KEY_CONTENT_TYPE); + if (!contentType.equals(h.getContentType())) + throw new InvalidAttachmentException(); int offset = meta.getLong(MSG_KEY_DESCRIPTOR_LENGTH).intValue(); - return new Attachment(new ByteArrayInputStream(body, offset, + return new Attachment(h, new ByteArrayInputStream(body, offset, body.length - offset)); } catch (FormatException e) { throw new DbException(e);