Reject attachments that exceed the allowed size

Closes #1468

briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentResult.java

@@ -0,0 +1,43 @@
+package org.briarproject.briar.android.conversation;
+
+import android.net.Uri;
+
+import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
+
+import javax.annotation.Nullable;
+import javax.annotation.concurrent.Immutable;
+
+@Immutable
+@NotNullByDefault
+public class AttachmentResult {
+
+	@Nullable
+	private final Uri uri;
+	@Nullable
+	private final String errorMsg;
+
+	public AttachmentResult(Uri uri) {
+		this.uri = uri;
+		this.errorMsg = null;
+	}
+
+	public AttachmentResult(@Nullable String errorMsg) {
+		this.uri = null;
+		this.errorMsg = errorMsg;
+	}
+
+	@Nullable
+	public Uri getUri() {
+		return uri;
+	}
+
+	public boolean isError() {
+		return errorMsg != null;
+	}
+
+	@Nullable
+	public String getErrorMsg() {
+		return errorMsg;
+	}
+
+}

briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java

@@ -27,11 +27,13 @@ import org.briarproject.bramble.api.sync.GroupId;
 import org.briarproject.bramble.api.sync.Message;
 import org.briarproject.bramble.api.sync.MessageId;
 import org.briarproject.bramble.api.system.AndroidExecutor;
+import org.briarproject.briar.R;
 import org.briarproject.briar.android.util.UiUtils;
 import org.briarproject.briar.android.view.TextAttachmentController.AttachmentManager;
 import org.briarproject.briar.android.viewmodel.LiveEvent;
 import org.briarproject.briar.android.viewmodel.MutableLiveEvent;
 import org.briarproject.briar.api.messaging.AttachmentHeader;
+import org.briarproject.briar.api.messaging.FileTooBigException;
 import org.briarproject.briar.api.messaging.MessagingManager;
 import org.briarproject.briar.api.messaging.PrivateMessage;
 import org.briarproject.briar.api.messaging.PrivateMessageFactory;
@@ -54,6 +56,7 @@ import static org.briarproject.bramble.util.LogUtils.now;
 import static org.briarproject.briar.android.conversation.AttachmentDimensions.getAttachmentDimensions;
 import static org.briarproject.briar.android.settings.SettingsFragment.SETTINGS_NAMESPACE;
 import static org.briarproject.briar.android.util.UiUtils.observeForeverOnce;
+import static org.briarproject.briar.api.messaging.MessagingConstants.MAX_IMAGE_SIZE;
 
 @NotNullByDefault
 public class ConversationViewModel extends AndroidViewModel implements
@@ -192,9 +195,11 @@ public class ConversationViewModel extends AndroidViewModel implements
 	}
 
 	@Override
-	public void storeAttachment(Uri uri, boolean needsSize, Runnable onSuccess,
-			Runnable onError) {
+	public LiveData<AttachmentResult> storeAttachment(Uri uri,
+			boolean needsSize) {
 		if (messagingGroupId.getValue() == null) loadGroupId();
+		// use LiveData to not keep references to view scope
+		MutableLiveData<AttachmentResult> result = new MutableLiveData<>();
 		observeForeverOnce(messagingGroupId, groupId -> dbExecutor.execute(()
 				-> {
 			if (groupId == null) throw new IllegalStateException();
@@ -204,13 +209,20 @@ public class ConversationViewModel extends AndroidViewModel implements
 						getApplication().getContentResolver();
 				attachmentController.createAttachmentHeader(contentResolver,
 						groupId, uri, needsSize);
-				androidExecutor.runOnUiThread(onSuccess);
+				result.postValue(new AttachmentResult(uri));
+			} catch(FileTooBigException e) {
+				logException(LOG, WARNING, e);
+				int mb = MAX_IMAGE_SIZE / 1024 / 1024;
+				String errorMsg = getApplication()
+						.getString(R.string.image_attach_error_too_big, mb);
+				result.postValue(new AttachmentResult(errorMsg));
 			} catch (DbException | IOException e) {
 				logException(LOG, WARNING, e);
-				androidExecutor.runOnUiThread(onError);
+				result.postValue(new AttachmentResult((String) null));
 			}
 			logDuration(LOG, "Storing attachment", start);
 		}));
+		return result;
 	}
 
 	@Override

briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewItem.java

@@ -15,7 +15,7 @@ class ImagePreviewItem {
 	private final Uri uri;
 	private boolean waitForLoading = true;
 
-	private ImagePreviewItem(Uri uri) {
+	ImagePreviewItem(Uri uri) {
 		this.uri = uri;
 	}
 

briar-android/src/main/java/org/briarproject/briar/android/view/TextAttachmentController.java

@@ -1,6 +1,8 @@
 package org.briarproject.briar.android.view;
 
 import android.app.Activity;
+import android.arch.lifecycle.LifecycleOwner;
+import android.arch.lifecycle.LiveData;
 import android.content.ClipData;
 import android.content.Context;
 import android.content.Intent;
@@ -15,6 +17,7 @@ import android.widget.Toast;
 
 import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
 import org.briarproject.briar.R;
+import org.briarproject.briar.android.conversation.AttachmentResult;
 import org.briarproject.briar.android.view.ImagePreview.ImagePreviewListener;
 import org.briarproject.briar.api.messaging.AttachmentHeader;
 
@@ -149,9 +152,17 @@ public class TextAttachmentController extends TextSendController
 		// store attachments and show preview when successful
 		boolean needsSize = items.size() == 1;
 		for (ImagePreviewItem item : items) {
-			attachmentManager.storeAttachment(item.getUri(), needsSize,
-					() -> imagePreview.loadPreviewImage(item),
-					this::onError);
+			attachmentManager.storeAttachment(item.getUri(), needsSize)
+					.observe(imageListener, this::onAttachmentResultReceived);
+		}
+	}
+
+	private void onAttachmentResultReceived(AttachmentResult result) {
+		if (result.isError() || result.getUri() == null) {
+			onError(result.getErrorMsg());
+		} else {
+			ImagePreviewItem item = new ImagePreviewItem(result.getUri());
+			imagePreview.loadPreviewImage(item);
 		}
 	}
 
@@ -194,8 +205,16 @@ public class TextAttachmentController extends TextSendController
 
 	@Override
 	public void onError() {
-		Toast.makeText(textInput.getContext(), R.string.image_attach_error,
-				LENGTH_LONG).show();
+		onError(null);
+	}
+
+	@UiThread
+	private void onError(@Nullable String errorMsg) {
+		if (errorMsg == null) {
+			errorMsg = imagePreview.getContext()
+					.getString(R.string.image_attach_error);
+		}
+		Toast.makeText(textInput.getContext(), errorMsg, LENGTH_LONG).show();
 		onCancel();
 	}
 
@@ -268,7 +287,7 @@ public class TextAttachmentController extends TextSendController
 				};
 	}
 
-	public interface AttachImageListener {
+	public interface AttachImageListener extends LifecycleOwner {
 		void onAttachImage(Intent intent);
 	}
 
@@ -277,11 +296,10 @@ public class TextAttachmentController extends TextSendController
 		 * Stores a new attachment in the database.
 		 *
 		 * @param uri The Uri of the attachment to store.
-		 * @param onSuccess will be run on the UiThread when the attachment was stored successfully.
-		 * @param onError will be run on the UiThread when the attachment could not be stored.
+		 * @param needsSize true if this is the only image in the message
+		 * and therefore needs to know its size.
 		 */
-		void storeAttachment(Uri uri, boolean needsSize, Runnable onSuccess,
-				Runnable onError);
+		LiveData<AttachmentResult> storeAttachment(Uri uri, boolean needsSize);
 
 		List<AttachmentHeader> getAttachmentHeaders();
 

briar-android/src/main/res/values/strings.xml

@@ -130,6 +130,7 @@
 	<string name="image_caption_hint">Add a caption (optional)</string>
 	<string name="image_attach">Attach image</string>
 	<string name="image_attach_error">Could not attach image(s)</string>
+	<string name="image_attach_error_too_big">Image too big. Limit is %d MB.</string>
 	<string name="set_contact_alias">Change contact name</string>
 	<string name="set_contact_alias_hint">Contact name</string>
 	<string name="set_alias_button">Change</string>

briar-api/src/main/java/org/briarproject/briar/api/messaging/FileTooBigException.java

@@ -0,0 +1,6 @@
+package org.briarproject.briar.api.messaging;
+
+import java.io.IOException;
+
+public class FileTooBigException extends IOException {
+}

briar-api/src/main/java/org/briarproject/briar/api/messaging/MessagingConstants.java

@@ -20,7 +20,8 @@ public interface MessagingConstants {
 
 	/**
 	 * The maximum allowed size of image attachments.
+	 * TODO: Different limit for GIFs?
 	 */
-	int MAX_IMAGE_SIZE = 6 * 1024 * 1024;
+	int MAX_IMAGE_SIZE = MAX_MESSAGE_BODY_LENGTH; // 6 * 1024 * 1024;
 
 }

briar-api/src/main/java/org/briarproject/briar/api/messaging/MessagingManager.java

@@ -37,6 +37,8 @@ public interface MessagingManager extends ConversationClient {
 
 	/**
 	 * Stores a local attachment message.
+	 *
+	 * @throws FileTooBigException
 	 */
 	AttachmentHeader addLocalAttachment(GroupId groupId, long timestamp,
 			String contentType, InputStream is) throws DbException, IOException;

briar-core/src/main/java/org/briarproject/briar/messaging/MessagingManagerImpl.java

@@ -18,14 +18,17 @@ import org.briarproject.bramble.api.sync.Group;
 import org.briarproject.bramble.api.sync.Group.Visibility;
 import org.briarproject.bramble.api.sync.GroupId;
 import org.briarproject.bramble.api.sync.Message;
+import org.briarproject.bramble.api.sync.MessageFactory;
 import org.briarproject.bramble.api.sync.MessageId;
 import org.briarproject.bramble.api.sync.MessageStatus;
 import org.briarproject.bramble.api.versioning.ClientVersioningManager;
 import org.briarproject.bramble.api.versioning.ClientVersioningManager.ClientVersioningHook;
+import org.briarproject.bramble.util.IoUtils;
 import org.briarproject.briar.api.client.MessageTracker;
 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.MessagingManager;
 import org.briarproject.briar.api.messaging.PrivateMessage;
 import org.briarproject.briar.api.messaging.PrivateMessageHeader;
@@ -33,18 +36,18 @@ import org.briarproject.briar.api.messaging.event.PrivateMessageReceivedEvent;
 import org.briarproject.briar.client.ConversationClientImpl;
 
 import java.io.ByteArrayInputStream;
+import java.io.EOFException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Map;
-import java.util.Random;
 
 import javax.annotation.concurrent.Immutable;
 import javax.inject.Inject;
 
 import static java.util.Collections.emptyList;
-import static org.briarproject.bramble.util.StringUtils.fromHexString;
+import static org.briarproject.bramble.api.sync.SyncConstants.MAX_MESSAGE_BODY_LENGTH;
 import static org.briarproject.briar.client.MessageTrackerConstants.MSG_KEY_READ;
 
 @Immutable
@@ -55,15 +58,18 @@ class MessagingManagerImpl extends ConversationClientImpl
 
 	private final ClientVersioningManager clientVersioningManager;
 	private final ContactGroupFactory contactGroupFactory;
+	private final MessageFactory messageFactory;
 
 	@Inject
 	MessagingManagerImpl(DatabaseComponent db, ClientHelper clientHelper,
 			ClientVersioningManager clientVersioningManager,
 			MetadataParser metadataParser, MessageTracker messageTracker,
-			ContactGroupFactory contactGroupFactory) {
+			ContactGroupFactory contactGroupFactory,
+			MessageFactory messageFactory) {
 		super(db, clientHelper, metadataParser, messageTracker);
 		this.clientVersioningManager = clientVersioningManager;
 		this.contactGroupFactory = contactGroupFactory;
+		this.messageFactory = messageFactory;
 	}
 
 	@Override
@@ -158,17 +164,25 @@ class MessagingManagerImpl extends ConversationClientImpl
 
 	@Override
 	public AttachmentHeader addLocalAttachment(GroupId groupId, long timestamp,
-			String contentType, InputStream is) throws IOException {
+			String contentType, InputStream is)
+			throws DbException, IOException {
 		// TODO add real implementation
-		if (is.available() == 0) throw new IOException();
-		byte[] b = new byte[MessageId.LENGTH];
-		new Random().nextBytes(b);
-		return new AttachmentHeader(new MessageId(b), contentType);
+		byte[] body = new byte[MAX_MESSAGE_BODY_LENGTH];
+		try {
+			IoUtils.read(is, body);
+		} catch (EOFException ignored) {
+		}
+		if (is.available() > 0) throw new FileTooBigException();
+		is.close();
+		Message m = messageFactory.createMessage(groupId, timestamp, body);
+		clientHelper.addLocalMessage(m, new BdfDictionary(), false);
+		return new AttachmentHeader(m.getId(), contentType);
 	}
 
 	@Override
 	public void removeAttachment(AttachmentHeader header) throws DbException {
-		// TODO add real implementation
+		db.transaction(false,
+				txn -> db.removeMessage(txn, header.getMessageId()));
 	}
 
 	private ContactId getContactId(Transaction txn, GroupId g)
@@ -247,12 +261,10 @@ class MessagingManagerImpl extends ConversationClientImpl
 	}
 
 	@Override
-	public Attachment getAttachment(MessageId m) {
+	public Attachment getAttachment(MessageId mId) throws DbException {
 		// TODO add real implementation
-		byte[] bytes = fromHexString("89504E470D0A1A0A0000000D49484452" +
-				"000000010000000108060000001F15C4" +
-				"890000000A49444154789C6300010000" +
-				"0500010D0A2DB40000000049454E44AE426082");
+		Message m = clientHelper.getMessage(mId);
+		byte[] bytes = m.getBody();
 		return new Attachment(new ByteArrayInputStream(bytes));
 	}