diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/client/ClientHelper.java b/bramble-api/src/main/java/org/briarproject/bramble/api/client/ClientHelper.java index bd5b8c55c..df68d7933 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/client/ClientHelper.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/client/ClientHelper.java @@ -25,7 +25,10 @@ public interface ClientHelper { throws DbException, FormatException; void addLocalMessage(Transaction txn, Message m, BdfDictionary metadata, - boolean shared) throws DbException, FormatException; + boolean shared, boolean temporary) + throws DbException, FormatException; + + Message createMessage(GroupId g, long timestamp, byte[] body); Message createMessage(GroupId g, long timestamp, BdfList body) throws FormatException; @@ -108,7 +111,7 @@ public interface ClientHelper { Author parseAndValidateAuthor(BdfList author) throws FormatException; PublicKey parseAndValidateAgreementPublicKey(byte[] publicKeyBytes) - throws FormatException; + throws FormatException; TransportProperties parseAndValidateTransportProperties( BdfDictionary properties) throws FormatException; diff --git a/bramble-core/src/main/java/org/briarproject/bramble/client/ClientHelperImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/client/ClientHelperImpl.java index 7a0ca9e6e..604570aee 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/client/ClientHelperImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/client/ClientHelperImpl.java @@ -85,15 +85,21 @@ class ClientHelperImpl implements ClientHelper { @Override public void addLocalMessage(Message m, BdfDictionary metadata, boolean shared) throws DbException, FormatException { - db.transaction(false, txn -> addLocalMessage(txn, m, metadata, shared)); + db.transaction(false, txn -> addLocalMessage(txn, m, metadata, shared, + false)); } @Override public void addLocalMessage(Transaction txn, Message m, - BdfDictionary metadata, boolean shared) + BdfDictionary metadata, boolean shared, boolean temporary) throws DbException, FormatException { db.addLocalMessage(txn, m, metadataEncoder.encode(metadata), shared, - false); + temporary); + } + + @Override + public Message createMessage(GroupId g, long timestamp, byte[] body) { + return messageFactory.createMessage(g, timestamp, body); } @Override diff --git a/bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyManagerImpl.java index 6549f070c..9e79bfd66 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyManagerImpl.java @@ -284,7 +284,7 @@ class TransportPropertyManagerImpl implements TransportPropertyManager, meta.put("transportId", t.getString()); meta.put("version", version); meta.put("local", local); - clientHelper.addLocalMessage(txn, m, meta, shared); + clientHelper.addLocalMessage(txn, m, meta, shared, false); } catch (FormatException e) { throw new RuntimeException(e); } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/sync/validation/ValidationManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/sync/validation/ValidationManagerImpl.java index ddd5193d3..d0d583195 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/sync/validation/ValidationManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/sync/validation/ValidationManagerImpl.java @@ -314,6 +314,7 @@ class ValidationManagerImpl implements ValidationManager, Service, try { shareMsg = hook.incomingMessage(txn, m, meta); } catch (InvalidMessageException e) { + logException(LOG, INFO, e); invalidateMessage(txn, m.getId()); return new DeliveryResult(false, false); } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/versioning/ClientVersioningManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/versioning/ClientVersioningManagerImpl.java index 0f1573d82..929432c8d 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/versioning/ClientVersioningManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/versioning/ClientVersioningManagerImpl.java @@ -438,7 +438,7 @@ class ClientVersioningManagerImpl implements ClientVersioningManager, BdfDictionary meta = new BdfDictionary(); meta.put(MSG_KEY_UPDATE_VERSION, updateVersion); meta.put(MSG_KEY_LOCAL, true); - clientHelper.addLocalMessage(txn, m, meta, true); + clientHelper.addLocalMessage(txn, m, meta, true, false); } catch (FormatException e) { throw new RuntimeException(e); } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/properties/TransportPropertyManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/properties/TransportPropertyManagerImplTest.java index 8ec2e8631..007d264b7 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/properties/TransportPropertyManagerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/properties/TransportPropertyManagerImplTest.java @@ -637,7 +637,8 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { will(returnValue(timestamp)); oneOf(clientHelper).createMessage(g, timestamp, body); will(returnValue(message)); - oneOf(clientHelper).addLocalMessage(txn, message, meta, shared); + oneOf(clientHelper).addLocalMessage(txn, message, meta, shared, + false); }}); } } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/versioning/ClientVersioningManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/versioning/ClientVersioningManagerImplTest.java index 8007c7c4a..197d5a167 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/versioning/ClientVersioningManagerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/versioning/ClientVersioningManagerImplTest.java @@ -131,7 +131,7 @@ public class ClientVersioningManagerImplTest extends BrambleMockTestCase { localUpdateBody); will(returnValue(localUpdate)); oneOf(clientHelper).addLocalMessage(txn, localUpdate, - localUpdateMeta, true); + localUpdateMeta, true, false); }}); } @@ -284,7 +284,7 @@ public class ClientVersioningManagerImplTest extends BrambleMockTestCase { newLocalUpdateBody); will(returnValue(newLocalUpdate)); oneOf(clientHelper).addLocalMessage(txn, newLocalUpdate, - newLocalUpdateMeta, true); + newLocalUpdateMeta, true, false); // No visibilities have changed }}); @@ -382,7 +382,7 @@ public class ClientVersioningManagerImplTest extends BrambleMockTestCase { newLocalUpdateBody); will(returnValue(newLocalUpdate)); oneOf(clientHelper).addLocalMessage(txn, newLocalUpdate, - newLocalUpdateMeta, true); + newLocalUpdateMeta, true, false); // The client's visibility has changed oneOf(hook).onClientVisibilityChanging(txn, contact, visibility); }}); @@ -567,7 +567,7 @@ public class ClientVersioningManagerImplTest extends BrambleMockTestCase { newLocalUpdateBody); will(returnValue(newLocalUpdate)); oneOf(clientHelper).addLocalMessage(txn, newLocalUpdate, - newLocalUpdateMeta, true); + newLocalUpdateMeta, true, false); // The client's visibility has changed oneOf(clientHelper).getGroupMetadataAsDictionary(txn, contactGroup.getId()); @@ -640,7 +640,7 @@ public class ClientVersioningManagerImplTest extends BrambleMockTestCase { newLocalUpdateBody); will(returnValue(newLocalUpdate)); oneOf(clientHelper).addLocalMessage(txn, newLocalUpdate, - newLocalUpdateMeta, true); + newLocalUpdateMeta, true, false); // The client's visibility has changed oneOf(clientHelper).getGroupMetadataAsDictionary(txn, contactGroup.getId()); 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 e8dac0f32..eb38d51c4 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 @@ -6,6 +6,7 @@ import android.arch.lifecycle.ViewModelProvider; import android.arch.lifecycle.ViewModelProviders; import android.content.DialogInterface; import android.content.Intent; +import android.net.Uri; import android.os.Bundle; import android.os.Parcelable; import android.support.annotation.Nullable; @@ -34,7 +35,6 @@ import org.briarproject.bramble.api.Pair; import org.briarproject.bramble.api.contact.ContactId; import org.briarproject.bramble.api.contact.ContactManager; import org.briarproject.bramble.api.contact.event.ContactRemovedEvent; -import org.briarproject.bramble.api.crypto.CryptoExecutor; import org.briarproject.bramble.api.db.DatabaseExecutor; import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.db.NoSuchContactException; @@ -46,7 +46,6 @@ import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault; import org.briarproject.bramble.api.plugin.ConnectionRegistry; import org.briarproject.bramble.api.plugin.event.ContactConnectedEvent; import org.briarproject.bramble.api.plugin.event.ContactDisconnectedEvent; -import org.briarproject.bramble.api.settings.SettingsManager; import org.briarproject.bramble.api.sync.MessageId; import org.briarproject.bramble.api.sync.event.MessagesAckedEvent; import org.briarproject.bramble.api.sync.event.MessagesSentEvent; @@ -65,10 +64,9 @@ import org.briarproject.briar.android.util.BriarSnackbarBuilder; import org.briarproject.briar.android.view.BriarRecyclerView; import org.briarproject.briar.android.view.ImagePreview; import org.briarproject.briar.android.view.TextAttachmentController; -import org.briarproject.briar.android.view.TextAttachmentController.AttachImageListener; +import org.briarproject.briar.android.view.TextAttachmentController.AttachmentListener; import org.briarproject.briar.android.view.TextInputView; import org.briarproject.briar.android.view.TextSendController; -import org.briarproject.briar.android.view.TextSendController.SendListener; import org.briarproject.briar.api.android.AndroidNotificationManager; import org.briarproject.briar.api.blog.BlogSharingManager; import org.briarproject.briar.api.client.ProtocolStateException; @@ -83,7 +81,6 @@ import org.briarproject.briar.api.introduction.IntroductionManager; 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.PrivateMessageFactory; import org.briarproject.briar.api.messaging.PrivateMessageHeader; import org.briarproject.briar.api.privategroup.invitation.GroupInvitationManager; @@ -94,7 +91,6 @@ import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.Executor; import java.util.logging.Logger; import javax.inject.Inject; @@ -129,6 +125,7 @@ import static org.briarproject.briar.android.conversation.ImageActivity.NAME; import static org.briarproject.briar.android.util.UiUtils.getAvatarTransitionName; import static org.briarproject.briar.android.util.UiUtils.getBulbTransitionName; import static org.briarproject.briar.android.util.UiUtils.observeOnce; +import static org.briarproject.briar.api.messaging.MessagingConstants.MAX_ATTACHMENTS_PER_MESSAGE; import static org.briarproject.briar.api.messaging.MessagingConstants.MAX_PRIVATE_MESSAGE_TEXT_LENGTH; import static uk.co.samuelwall.materialtaptargetprompt.MaterialTapTargetPrompt.STATE_DISMISSED; import static uk.co.samuelwall.materialtaptargetprompt.MaterialTapTargetPrompt.STATE_FINISHED; @@ -136,8 +133,8 @@ import static uk.co.samuelwall.materialtaptargetprompt.MaterialTapTargetPrompt.S @MethodsNotNullByDefault @ParametersNotNullByDefault public class ConversationActivity extends BriarActivity - implements EventListener, ConversationListener, SendListener, - TextCache, AttachmentCache, AttachImageListener { + implements EventListener, ConversationListener, TextCache, + AttachmentCache, AttachmentListener { public static final String CONTACT_ID = "briar.CONTACT_ID"; @@ -152,9 +149,6 @@ public class ConversationActivity extends BriarActivity @Inject ConnectionRegistry connectionRegistry; @Inject - @CryptoExecutor - Executor cryptoExecutor; - @Inject ViewModelProvider.Factory viewModelFactory; @Inject FeatureFlags featureFlags; @@ -169,10 +163,6 @@ public class ConversationActivity extends BriarActivity @Inject volatile EventBus eventBus; @Inject - volatile SettingsManager settingsManager; - @Inject - volatile PrivateMessageFactory privateMessageFactory; - @Inject volatile IntroductionManager introductionManager; @Inject volatile ForumSharingManager forumSharingManager; @@ -267,10 +257,10 @@ public class ConversationActivity extends BriarActivity if (featureFlags.shouldEnableImageAttachments()) { ImagePreview imagePreview = findViewById(R.id.imagePreview); sendController = new TextAttachmentController(textInputView, - imagePreview, this, this, viewModel); + imagePreview, this, viewModel); observeOnce(viewModel.hasImageSupport(), this, hasSupport -> { if (hasSupport != null && hasSupport) { - // remove cast when removing FEATURE_FLAG_IMAGE_ATTACHMENTS + // TODO: remove cast when removing feature flag ((TextAttachmentController) sendController) .setImagesSupported(); } @@ -305,7 +295,7 @@ public class ConversationActivity extends BriarActivity Snackbar.LENGTH_SHORT) .show(); } else if (request == REQUEST_ATTACH_IMAGE && result == RESULT_OK) { - // remove cast when removing FEATURE_FLAG_IMAGE_ATTACHMENTS + // TODO: remove cast when removing feature flag ((TextAttachmentController) sendController).onImageReceived(data); } } @@ -454,7 +444,7 @@ public class ConversationActivity extends BriarActivity if (text == null) { LOG.info("Eagerly loading text for latest message"); text = messagingManager.getMessageText(id); - textCache.put(id, text); + textCache.put(id, requireNonNull(text)); } } // If the message has a single image, load its size - for multiple @@ -478,8 +468,10 @@ public class ConversationActivity extends BriarActivity adapter.incrementRevision(); textInputView.setReady(true); // start observing onboarding after enabling - viewModel.showImageOnboarding().observeEvent(this, - this::showImageOnboarding); + if (featureFlags.shouldEnableImageAttachments()) { + viewModel.showImageOnboarding().observeEvent(this, + this::showImageOnboarding); + } List items = createItems(headers); adapter.addAll(items); list.showData(); @@ -515,7 +507,7 @@ public class ConversationActivity extends BriarActivity long start = now(); String text = messagingManager.getMessageText(m); logDuration(LOG, "Loading text", start); - displayMessageText(m, text); + displayMessageText(m, requireNonNull(text)); } catch (DbException e) { logException(LOG, WARNING, e); } @@ -660,6 +652,18 @@ public class ConversationActivity extends BriarActivity startActivityForResult(intent, REQUEST_ATTACH_IMAGE); } + @Override + public List filterAttachmentUris(List uris) { + if (uris.size() > MAX_ATTACHMENTS_PER_MESSAGE) { + String format = getResources().getString( + R.string.messaging_too_many_attachments_toast); + String warning = String.format(format, MAX_ATTACHMENTS_PER_MESSAGE); + Toast.makeText(this, warning, LENGTH_SHORT).show(); + uris = uris.subList(0, MAX_ATTACHMENTS_PER_MESSAGE); + } + return new ArrayList<>(uris); + } + @Override public void onSendClick(@Nullable String text, List attachmentHeaders) { @@ -729,7 +733,7 @@ public class ConversationActivity extends BriarActivity } private void showImageOnboarding() { - // remove cast when removing FEATURE_FLAG_IMAGE_ATTACHMENTS + // TODO: remove cast when removing feature flag ((TextAttachmentController) sendController) .showImageOnboarding(this, () -> viewModel.onImageOnboardingSeen()); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java index 45d99c1f3..fc565d3a2 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java @@ -61,6 +61,7 @@ public class ConversationViewModel extends AndroidViewModel private static Logger LOG = getLogger(ConversationViewModel.class.getName()); + private static final String SHOW_ONBOARDING_IMAGE = "showOnboardingImage"; private static final String SHOW_ONBOARDING_INTRODUCTION = @@ -181,12 +182,17 @@ public class ConversationViewModel extends AndroidViewModel }); } + @UiThread void sendMessage(@Nullable String text, - List attachmentHeaders, long timestamp) { + List headers, long timestamp) { // messagingGroupId is loaded with the contact observeForeverOnce(messagingGroupId, groupId -> { - if (groupId == null) throw new IllegalStateException(); - createMessage(groupId, text, attachmentHeaders, timestamp); + requireNonNull(groupId); + observeForeverOnce(imageSupport, hasImageSupport -> { + requireNonNull(hasImageSupport); + createMessage(groupId, text, headers, timestamp, + hasImageSupport); + }); }); } @@ -270,21 +276,24 @@ public class ConversationViewModel extends AndroidViewModel } private void createMessage(GroupId groupId, @Nullable String text, - List attachments, long timestamp) { + List headers, long timestamp, + boolean hasImageSupport) { try { - // TODO remove when text can be null in the backend - String msgText = text == null ? "null" : text; - PrivateMessage pm = privateMessageFactory - .createPrivateMessage(groupId, timestamp, msgText, - attachments); - storeMessage(pm, msgText, attachments); + PrivateMessage pm; + if (hasImageSupport) { + pm = privateMessageFactory.createPrivateMessage(groupId, + timestamp, text, headers); + } else { + pm = privateMessageFactory.createLegacyPrivateMessage( + groupId, timestamp, requireNonNull(text)); + } + storeMessage(pm); } catch (FormatException e) { - throw new RuntimeException(e); + throw new AssertionError(e); } } - private void storeMessage(PrivateMessage m, @Nullable String text, - List attachments) { + private void storeMessage(PrivateMessage m) { attachmentCreator.onAttachmentsSent(m.getMessage().getId()); dbExecutor.execute(() -> { try { @@ -295,7 +304,7 @@ public class ConversationViewModel extends AndroidViewModel PrivateMessageHeader h = new PrivateMessageHeader( message.getId(), message.getGroupId(), message.getTimestamp(), true, true, false, false, - text != null, attachments); + m.hasText(), m.getAttachmentHeaders()); // TODO add text to cache when available here addedHeader.postEvent(h); } catch (DbException e) { diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/TextAttachmentController.java b/briar-android/src/main/java/org/briarproject/briar/android/view/TextAttachmentController.java index 34a097846..68dd8204f 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/TextAttachmentController.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/TextAttachmentController.java @@ -41,7 +41,6 @@ import static android.support.v4.content.ContextCompat.getColor; import static android.support.v4.view.AbsSavedState.EMPTY_STATE; import static android.view.View.GONE; import static android.widget.Toast.LENGTH_LONG; -import static java.util.Objects.requireNonNull; import static org.briarproject.briar.android.util.UiUtils.resolveColorAttribute; import static org.briarproject.briar.api.messaging.MessagingConstants.IMAGE_MIME_TYPES; import static uk.co.samuelwall.materialtaptargetprompt.MaterialTapTargetPrompt.STATE_DISMISSED; @@ -53,7 +52,7 @@ public class TextAttachmentController extends TextSendController implements ImagePreviewListener { private final ImagePreview imagePreview; - private final AttachImageListener imageListener; + private final AttachmentListener attachmentListener; private final CompositeSendButton sendButton; private final AttachmentManager attachmentManager; @@ -62,10 +61,10 @@ public class TextAttachmentController extends TextSendController private boolean loadingUris = false; public TextAttachmentController(TextInputView v, ImagePreview imagePreview, - SendListener listener, AttachImageListener imageListener, + AttachmentListener attachmentListener, AttachmentManager attachmentManager) { - super(v, listener, false); - this.imageListener = imageListener; + super(v, attachmentListener, false); + this.attachmentListener = attachmentListener; this.imagePreview = imagePreview; this.attachmentManager = attachmentManager; this.imagePreview.setImagePreviewListener(this); @@ -124,8 +123,8 @@ public class TextAttachmentController extends TextSendController return; } Intent intent = getAttachFileIntent(); - if (imageListener.getLifecycle().getCurrentState() != DESTROYED) { - requireNonNull(imageListener).onAttachImage(intent); + if (attachmentListener.getLifecycle().getCurrentState() != DESTROYED) { + attachmentListener.onAttachImage(intent); } } @@ -144,11 +143,13 @@ public class TextAttachmentController extends TextSendController * returned by the Activity started with {@link #getAttachFileIntent()}. *

* This method must be called at most once per call to - * {@link AttachImageListener#onAttachImage(Intent)}. + * {@link AttachmentListener#onAttachImage(Intent)}. * Normally, this is true if called from * {@link Activity#onActivityResult(int, int, Intent)} since this is called - * at most once per call to {@link Activity#startActivityForResult(Intent, int)}. + * at most once per call to + * {@link Activity#startActivityForResult(Intent, int)}. */ + @SuppressWarnings("JavadocReference") public void onImageReceived(@Nullable Intent resultData) { if (resultData == null) return; if (loadingUris || !imageUris.isEmpty()) throw new AssertionError(); @@ -168,6 +169,9 @@ public class TextAttachmentController extends TextSendController if (imageUris.isEmpty()) return; if (loadingUris) throw new AssertionError(); loadingUris = true; + List filtered = attachmentListener.filterAttachmentUris(imageUris); + imageUris.clear(); + imageUris.addAll(filtered); updateViewState(); textInput.setHint(R.string.image_caption_hint); List items = ImagePreviewItem.fromUris(imageUris); @@ -175,7 +179,7 @@ public class TextAttachmentController extends TextSendController // store attachments and show preview when successful LiveData result = attachmentManager.storeAttachments(imageUris, restart); - result.observe(imageListener, new Observer() { + result.observe(attachmentListener, new Observer() { @Override public void onChanged(@Nullable AttachmentResult attachmentResult) { if (attachmentResult == null) { @@ -316,8 +320,12 @@ public class TextAttachmentController extends TextSendController }; } - public interface AttachImageListener extends LifecycleOwner { + @UiThread + public interface AttachmentListener extends SendListener, LifecycleOwner { + void onAttachImage(Intent intent); + + List filterAttachmentUris(List uris); } } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/TextSendController.java b/briar-android/src/main/java/org/briarproject/briar/android/view/TextSendController.java index 725d23baa..180039f0a 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/TextSendController.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/TextSendController.java @@ -84,6 +84,7 @@ public class TextSendController implements TextInputListener { return state; } + @UiThread public interface SendListener { void onSendClick(@Nullable String text, List headers); } diff --git a/briar-android/src/main/res/values/strings.xml b/briar-android/src/main/res/values/strings.xml index a80cae6e3..c360f2f0b 100644 --- a/briar-android/src/main/res/values/strings.xml +++ b/briar-android/src/main/res/values/strings.xml @@ -150,6 +150,7 @@ Your contact\'s Briar does not yet support image attachments. Once they upgrade you\'ll see a different icon. You can now send images to this contact Tap this icon to attach images. + Only the first %d images will be sent diff --git a/briar-api/src/main/java/org/briarproject/briar/api/client/ThreadedMessage.java b/briar-api/src/main/java/org/briarproject/briar/api/client/ThreadedMessage.java index 37b0dc86f..71a1b442b 100644 --- a/briar-api/src/main/java/org/briarproject/briar/api/client/ThreadedMessage.java +++ b/briar-api/src/main/java/org/briarproject/briar/api/client/ThreadedMessage.java @@ -4,26 +4,30 @@ import org.briarproject.bramble.api.identity.Author; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.sync.Message; import org.briarproject.bramble.api.sync.MessageId; -import org.briarproject.briar.api.messaging.PrivateMessage; import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; @Immutable @NotNullByDefault -public abstract class ThreadedMessage extends PrivateMessage { +public abstract class ThreadedMessage { + private final Message message; @Nullable private final MessageId parent; private final Author author; public ThreadedMessage(Message message, @Nullable MessageId parent, Author author) { - super(message); + this.message = message; this.parent = parent; this.author = author; } + public Message getMessage() { + return message; + } + @Nullable public MessageId getParent() { return parent; diff --git a/briar-api/src/main/java/org/briarproject/briar/api/conversation/event/ConversationMessageReceivedEvent.java b/briar-api/src/main/java/org/briarproject/briar/api/conversation/event/ConversationMessageReceivedEvent.java index b0a04097b..2a070a533 100644 --- a/briar-api/src/main/java/org/briarproject/briar/api/conversation/event/ConversationMessageReceivedEvent.java +++ b/briar-api/src/main/java/org/briarproject/briar/api/conversation/event/ConversationMessageReceivedEvent.java @@ -18,7 +18,8 @@ public abstract class ConversationMessageReceivedEvent attachmentHeaders; + /** + * Constructor for private messages in the legacy format, which does not + * support attachments. + */ public PrivateMessage(Message message) { this.message = message; + legacyFormat = true; + hasText = true; + attachmentHeaders = emptyList(); + } + + /** + * Constructor for private messages in the current format, which supports + * attachments. + */ + public PrivateMessage(Message message, boolean hasText, + List headers) { + this.message = message; + this.hasText = hasText; + this.attachmentHeaders = headers; + legacyFormat = false; } public Message getMessage() { return message; } + public boolean isLegacyFormat() { + return legacyFormat; + } + + public boolean hasText() { + return hasText; + } + + public List getAttachmentHeaders() { + return attachmentHeaders; + } } diff --git a/briar-api/src/main/java/org/briarproject/briar/api/messaging/PrivateMessageFactory.java b/briar-api/src/main/java/org/briarproject/briar/api/messaging/PrivateMessageFactory.java index f87f318cb..2f7e1127b 100644 --- a/briar-api/src/main/java/org/briarproject/briar/api/messaging/PrivateMessageFactory.java +++ b/briar-api/src/main/java/org/briarproject/briar/api/messaging/PrivateMessageFactory.java @@ -6,11 +6,16 @@ import org.briarproject.bramble.api.sync.GroupId; import java.util.List; +import javax.annotation.Nullable; + @NotNullByDefault public interface PrivateMessageFactory { + PrivateMessage createLegacyPrivateMessage(GroupId groupId, long timestamp, + String text) throws FormatException; + PrivateMessage createPrivateMessage(GroupId groupId, long timestamp, - String text, List attachments) + @Nullable String text, List headers) throws FormatException; } diff --git a/briar-api/src/main/java/org/briarproject/briar/api/messaging/PrivateMessageHeader.java b/briar-api/src/main/java/org/briarproject/briar/api/messaging/PrivateMessageHeader.java index 2faddbc77..010466f6f 100644 --- a/briar-api/src/main/java/org/briarproject/briar/api/messaging/PrivateMessageHeader.java +++ b/briar-api/src/main/java/org/briarproject/briar/api/messaging/PrivateMessageHeader.java @@ -19,10 +19,10 @@ public class PrivateMessageHeader extends ConversationMessageHeader { public PrivateMessageHeader(MessageId id, GroupId groupId, long timestamp, boolean local, boolean read, boolean sent, boolean seen, - boolean hasText, List attachmentHeaders) { + boolean hasText, List headers) { super(id, groupId, timestamp, local, read, sent, seen); this.hasText = hasText; - this.attachmentHeaders = attachmentHeaders; + this.attachmentHeaders = headers; } public boolean hasText() { diff --git a/briar-api/src/main/java/org/briarproject/briar/api/messaging/event/AttachmentReceivedEvent.java b/briar-api/src/main/java/org/briarproject/briar/api/messaging/event/AttachmentReceivedEvent.java new file mode 100644 index 000000000..e5f6e2b4f --- /dev/null +++ b/briar-api/src/main/java/org/briarproject/briar/api/messaging/event/AttachmentReceivedEvent.java @@ -0,0 +1,32 @@ +package org.briarproject.briar.api.messaging.event; + +import org.briarproject.bramble.api.contact.ContactId; +import org.briarproject.bramble.api.event.Event; +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; +import org.briarproject.bramble.api.sync.MessageId; + +import javax.annotation.concurrent.Immutable; + +/** + * An event that is broadcast when a new attachment is received. + */ +@Immutable +@NotNullByDefault +public class AttachmentReceivedEvent extends Event { + + private final MessageId messageId; + private final ContactId contactId; + + public AttachmentReceivedEvent(MessageId messageId, ContactId contactId) { + this.messageId = messageId; + this.contactId = contactId; + } + + public MessageId getMessageId() { + return messageId; + } + + public ContactId getContactId() { + return contactId; + } +} diff --git a/briar-api/src/main/java/org/briarproject/briar/api/test/TestDataCreator.java b/briar-api/src/main/java/org/briarproject/briar/api/test/TestDataCreator.java index 073a7b38a..3db7b48b9 100644 --- a/briar-api/src/main/java/org/briarproject/briar/api/test/TestDataCreator.java +++ b/briar-api/src/main/java/org/briarproject/briar/api/test/TestDataCreator.java @@ -1,6 +1,5 @@ package org.briarproject.briar.api.test; -import org.briarproject.bramble.api.FormatException; import org.briarproject.bramble.api.contact.Contact; import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.lifecycle.IoExecutor; @@ -24,9 +23,4 @@ public interface TestDataCreator { @IoExecutor Contact addContact(String name) throws DbException; - - @IoExecutor - void addPrivateMessage(Contact contact, String text, long time, - boolean local) throws DbException, FormatException; - } diff --git a/briar-core/src/main/java/org/briarproject/briar/blog/BlogManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/blog/BlogManagerImpl.java index 3b24d49b2..009b5030d 100644 --- a/briar-core/src/main/java/org/briarproject/briar/blog/BlogManagerImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/blog/BlogManagerImpl.java @@ -234,7 +234,8 @@ class BlogManagerImpl extends BdfIncomingMessageHook implements BlogManager, meta.put(KEY_AUTHOR, clientHelper.toList(p.getAuthor())); meta.put(KEY_READ, true); meta.put(KEY_RSS_FEED, b.isRssFeed()); - clientHelper.addLocalMessage(txn, p.getMessage(), meta, true); + clientHelper.addLocalMessage(txn, p.getMessage(), meta, true, + false); // broadcast event about new post MessageId postId = p.getMessage().getId(); @@ -279,7 +280,7 @@ class BlogManagerImpl extends BdfIncomingMessageHook implements BlogManager, meta.put(KEY_AUTHOR, clientHelper.toList(author)); // Send comment - clientHelper.addLocalMessage(txn, message, meta, true); + clientHelper.addLocalMessage(txn, message, meta, true, false); // broadcast event BlogPostHeader h = getPostHeaderFromMetadata(txn, groupId, @@ -377,7 +378,7 @@ class BlogManagerImpl extends BdfIncomingMessageHook implements BlogManager, meta.put(KEY_TIME_RECEIVED, header.getTimeReceived()); // Send wrapped message and store metadata - clientHelper.addLocalMessage(txn, wrappedMessage, meta, true); + clientHelper.addLocalMessage(txn, wrappedMessage, meta, true, false); return wrappedMessage.getId(); } diff --git a/briar-core/src/main/java/org/briarproject/briar/forum/ForumManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/forum/ForumManagerImpl.java index 6e196b196..ee9c578ec 100644 --- a/briar-core/src/main/java/org/briarproject/briar/forum/ForumManagerImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/forum/ForumManagerImpl.java @@ -136,7 +136,8 @@ class ForumManagerImpl extends BdfIncomingMessageHook implements ForumManager { meta.put(KEY_AUTHOR, clientHelper.toList(a)); meta.put(KEY_LOCAL, true); meta.put(MSG_KEY_READ, true); - clientHelper.addLocalMessage(txn, p.getMessage(), meta, true); + clientHelper.addLocalMessage(txn, p.getMessage(), meta, true, + false); messageTracker.trackOutgoingMessage(txn, p.getMessage()); } catch (FormatException e) { throw new AssertionError(e); diff --git a/briar-core/src/main/java/org/briarproject/briar/introduction/AbstractProtocolEngine.java b/briar-core/src/main/java/org/briarproject/briar/introduction/AbstractProtocolEngine.java index b7b20d065..b84f62463 100644 --- a/briar-core/src/main/java/org/briarproject/briar/introduction/AbstractProtocolEngine.java +++ b/briar-core/src/main/java/org/briarproject/briar/introduction/AbstractProtocolEngine.java @@ -140,7 +140,7 @@ abstract class AbstractProtocolEngine .encodeMetadata(type, sessionId, m.getTimestamp(), true, true, visibleInConversation); try { - clientHelper.addLocalMessage(txn, m, meta, true); + clientHelper.addLocalMessage(txn, m, meta, true, false); } catch (FormatException e) { throw new AssertionError(e); } @@ -177,8 +177,7 @@ abstract class AbstractProtocolEngine boolean isInvalidDependency(@Nullable MessageId lastRemoteMessageId, @Nullable MessageId dependency) { if (dependency == null) return lastRemoteMessageId != null; - return lastRemoteMessageId == null || - !dependency.equals(lastRemoteMessageId); + return !dependency.equals(lastRemoteMessageId); } long getLocalTimestamp(long localTimestamp, long requestTimestamp) { diff --git a/briar-core/src/main/java/org/briarproject/briar/messaging/CountingInputStream.java b/briar-core/src/main/java/org/briarproject/briar/messaging/CountingInputStream.java new file mode 100644 index 000000000..4f6063565 --- /dev/null +++ b/briar-core/src/main/java/org/briarproject/briar/messaging/CountingInputStream.java @@ -0,0 +1,60 @@ +package org.briarproject.briar.messaging; + +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; + +import java.io.IOException; +import java.io.InputStream; + +import javax.annotation.concurrent.NotThreadSafe; + +/** + * An {@link InputStream} that wraps another InputStream, counting the bytes + * read and only allowing a limited number of bytes to be read. + */ +@NotThreadSafe +@NotNullByDefault +class CountingInputStream extends InputStream { + + private final InputStream delegate; + private final long maxBytesToRead; + + private long bytesRead = 0; + + CountingInputStream(InputStream delegate, long maxBytesToRead) { + this.delegate = delegate; + this.maxBytesToRead = maxBytesToRead; + } + + long getBytesRead() { + return bytesRead; + } + + @Override + public int available() throws IOException { + return delegate.available(); + } + + @Override + public void close() throws IOException { + delegate.close(); + } + + @Override + public int read() throws IOException { + if (bytesRead == maxBytesToRead) return -1; + int i = delegate.read(); + if (i != -1) bytesRead++; + return i; + } + + @Override + public int read(byte[] b, int off, int len) throws IOException { + if (len == 0) return 0; + if (bytesRead == maxBytesToRead) return -1; + if (bytesRead + len > maxBytesToRead) + len = (int) (maxBytesToRead - bytesRead); + int read = delegate.read(b, off, len); + if (read != -1) bytesRead += read; + return read; + } +} diff --git a/briar-core/src/main/java/org/briarproject/briar/messaging/MessageTypes.java b/briar-core/src/main/java/org/briarproject/briar/messaging/MessageTypes.java new file mode 100644 index 000000000..a383a22df --- /dev/null +++ b/briar-core/src/main/java/org/briarproject/briar/messaging/MessageTypes.java @@ -0,0 +1,7 @@ +package org.briarproject.briar.messaging; + +interface MessageTypes { + + int PRIVATE_MESSAGE = 0; + int ATTACHMENT = 1; +} diff --git a/briar-core/src/main/java/org/briarproject/briar/messaging/MessagingConstants.java b/briar-core/src/main/java/org/briarproject/briar/messaging/MessagingConstants.java new file mode 100644 index 000000000..c9cb6b1eb --- /dev/null +++ b/briar-core/src/main/java/org/briarproject/briar/messaging/MessagingConstants.java @@ -0,0 +1,16 @@ +package org.briarproject.briar.messaging; + +interface MessagingConstants { + + // Metadata keys for groups + String GROUP_KEY_CONTACT_ID = "contactId"; + + // Metadata keys for messages + String MSG_KEY_TIMESTAMP = "timestamp"; + String MSG_KEY_LOCAL = "local"; + String MSG_KEY_MSG_TYPE = "messageType"; + String MSG_KEY_CONTENT_TYPE = "contentType"; + String MSG_KEY_DESCRIPTOR_LENGTH = "descriptorLength"; + String MSG_KEY_HAS_TEXT = "hasText"; + String MSG_KEY_ATTACHMENT_HEADERS = "attachmentHeaders"; +} 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 366eb768c..2fbbab6ac 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 @@ -11,20 +11,23 @@ import org.briarproject.bramble.api.data.BdfList; import org.briarproject.bramble.api.data.MetadataParser; import org.briarproject.bramble.api.db.DatabaseComponent; import org.briarproject.bramble.api.db.DbException; +import org.briarproject.bramble.api.db.Metadata; import org.briarproject.bramble.api.db.Transaction; import org.briarproject.bramble.api.lifecycle.LifecycleManager.OpenDatabaseHook; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; 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.InvalidMessageException; 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.sync.validation.IncomingMessageHook; 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.client.MessageTracker.GroupCount; +import org.briarproject.briar.api.conversation.ConversationManager.ConversationClient; import org.briarproject.briar.api.conversation.ConversationMessageHeader; import org.briarproject.briar.api.messaging.Attachment; import org.briarproject.briar.api.messaging.AttachmentHeader; @@ -32,15 +35,16 @@ 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; +import org.briarproject.briar.api.messaging.event.AttachmentReceivedEvent; 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.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; import java.util.Collection; +import java.util.List; import java.util.Map; import javax.annotation.concurrent.Immutable; @@ -48,28 +52,57 @@ import javax.inject.Inject; import static java.util.Collections.emptyList; import static org.briarproject.bramble.api.sync.SyncConstants.MAX_MESSAGE_BODY_LENGTH; +import static org.briarproject.bramble.util.IoUtils.copyAndClose; import static org.briarproject.briar.client.MessageTrackerConstants.MSG_KEY_READ; +import static org.briarproject.briar.messaging.MessageTypes.ATTACHMENT; +import static org.briarproject.briar.messaging.MessageTypes.PRIVATE_MESSAGE; +import static org.briarproject.briar.messaging.MessagingConstants.GROUP_KEY_CONTACT_ID; +import static org.briarproject.briar.messaging.MessagingConstants.MSG_KEY_ATTACHMENT_HEADERS; +import static org.briarproject.briar.messaging.MessagingConstants.MSG_KEY_CONTENT_TYPE; +import static org.briarproject.briar.messaging.MessagingConstants.MSG_KEY_DESCRIPTOR_LENGTH; +import static org.briarproject.briar.messaging.MessagingConstants.MSG_KEY_HAS_TEXT; +import static org.briarproject.briar.messaging.MessagingConstants.MSG_KEY_LOCAL; +import static org.briarproject.briar.messaging.MessagingConstants.MSG_KEY_MSG_TYPE; +import static org.briarproject.briar.messaging.MessagingConstants.MSG_KEY_TIMESTAMP; @Immutable @NotNullByDefault -class MessagingManagerImpl extends ConversationClientImpl - implements MessagingManager, OpenDatabaseHook, ContactHook, +class MessagingManagerImpl implements MessagingManager, IncomingMessageHook, + ConversationClient, OpenDatabaseHook, ContactHook, ClientVersioningHook { + private final DatabaseComponent db; + private final ClientHelper clientHelper; + private final MetadataParser metadataParser; + private final MessageTracker messageTracker; 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, - MessageFactory messageFactory) { - super(db, clientHelper, metadataParser, messageTracker); + ContactGroupFactory contactGroupFactory) { + this.db = db; + this.clientHelper = clientHelper; + this.metadataParser = metadataParser; + this.messageTracker = messageTracker; this.clientVersioningManager = clientVersioningManager; this.contactGroupFactory = contactGroupFactory; - this.messageFactory = messageFactory; + } + + @Override + public GroupCount getGroupCount(Transaction txn, ContactId contactId) + throws DbException { + Contact contact = db.getContact(txn, contactId); + GroupId groupId = getContactGroup(contact).getId(); + return messageTracker.getGroupCount(txn, groupId); + } + + @Override + public void setReadFlag(GroupId g, MessageId m, boolean read) + throws DbException { + messageTracker.setReadFlag(g, m, read); } @Override @@ -94,14 +127,14 @@ class MessagingManagerImpl extends ConversationClientImpl db.setGroupVisibility(txn, c.getId(), g.getId(), client); // Attach the contact ID to the group BdfDictionary d = new BdfDictionary(); - d.put("contactId", c.getId().getInt()); + d.put(GROUP_KEY_CONTACT_ID, c.getId().getInt()); try { clientHelper.mergeGroupMetadata(txn, g.getId(), d); } catch (FormatException e) { throw new AssertionError(e); } // Initialize the group count with current time - initializeGroupCount(txn, g.getId()); + messageTracker.initializeGroupCount(txn, g.getId()); } @Override @@ -124,24 +157,66 @@ class MessagingManagerImpl extends ConversationClientImpl } @Override - protected boolean incomingMessage(Transaction txn, Message m, BdfList body, - BdfDictionary meta) throws DbException, FormatException { + public boolean incomingMessage(Transaction txn, Message m, Metadata meta) + throws DbException, InvalidMessageException { + try { + BdfDictionary metaDict = metadataParser.parse(meta); + // Message type is null for version 0.0 private messages + Long messageType = metaDict.getOptionalLong(MSG_KEY_MSG_TYPE); + if (messageType == null) { + incomingPrivateMessage(txn, m, metaDict, true, emptyList()); + } else if (messageType == PRIVATE_MESSAGE) { + boolean hasText = metaDict.getBoolean(MSG_KEY_HAS_TEXT); + List headers = + parseAttachmentHeaders(metaDict); + incomingPrivateMessage(txn, m, metaDict, hasText, headers); + } else if (messageType == ATTACHMENT) { + incomingAttachment(txn, m); + } else { + throw new InvalidMessageException(); + } + } catch (FormatException e) { + throw new InvalidMessageException(e); + } + // Don't share message + return false; + } + private void incomingPrivateMessage(Transaction txn, Message m, + BdfDictionary meta, boolean hasText, List headers) + throws DbException, FormatException { GroupId groupId = m.getGroupId(); - long timestamp = meta.getLong("timestamp"); - boolean local = meta.getBoolean("local"); + long timestamp = meta.getLong(MSG_KEY_TIMESTAMP); + boolean local = meta.getBoolean(MSG_KEY_LOCAL); boolean read = meta.getBoolean(MSG_KEY_READ); PrivateMessageHeader header = new PrivateMessageHeader(m.getId(), groupId, timestamp, local, - read, false, false, true, emptyList()); + read, false, false, hasText, headers); ContactId contactId = getContactId(txn, groupId); PrivateMessageReceivedEvent event = new PrivateMessageReceivedEvent(header, contactId); txn.attach(event); messageTracker.trackIncomingMessage(txn, m); + } - // don't share message - return false; + private List parseAttachmentHeaders(BdfDictionary meta) + throws FormatException { + BdfList attachmentHeaders = meta.getList(MSG_KEY_ATTACHMENT_HEADERS); + int length = attachmentHeaders.size(); + List headers = new ArrayList<>(length); + for (int i = 0; i < length; i++) { + BdfList header = attachmentHeaders.getList(i); + MessageId id = new MessageId(header.getRaw(0)); + String contentType = header.getString(1); + headers.add(new AttachmentHeader(id, contentType)); + } + return headers; + } + + private void incomingAttachment(Transaction txn, Message m) + throws DbException { + ContactId contactId = getContactId(txn, m.getGroupId()); + txn.attach(new AttachmentReceivedEvent(m.getId(), contactId)); } @Override @@ -149,14 +224,30 @@ class MessagingManagerImpl extends ConversationClientImpl Transaction txn = db.startTransaction(false); try { BdfDictionary meta = new BdfDictionary(); - meta.put("timestamp", m.getMessage().getTimestamp()); - meta.put("local", true); - meta.put("read", true); - clientHelper.addLocalMessage(txn, m.getMessage(), meta, true); + meta.put(MSG_KEY_TIMESTAMP, m.getMessage().getTimestamp()); + meta.put(MSG_KEY_LOCAL, true); + meta.put(MSG_KEY_READ, true); + if (!m.isLegacyFormat()) { + meta.put(MSG_KEY_MSG_TYPE, PRIVATE_MESSAGE); + meta.put(MSG_KEY_HAS_TEXT, m.hasText()); + BdfList headers = new BdfList(); + for (AttachmentHeader a : m.getAttachmentHeaders()) { + headers.add( + BdfList.of(a.getMessageId(), a.getContentType())); + } + meta.put(MSG_KEY_ATTACHMENT_HEADERS, headers); + } + // Mark attachments as shared and permanent now we're ready to send + for (AttachmentHeader a : m.getAttachmentHeaders()) { + db.setMessageShared(txn, a.getMessageId()); + db.setMessagePermanent(txn, a.getMessageId()); + } + clientHelper.addLocalMessage(txn, m.getMessage(), meta, true, + false); messageTracker.trackOutgoingMessage(txn, m.getMessage()); db.commitTransaction(txn); } catch (FormatException e) { - throw new RuntimeException(e); + throw new AssertionError(e); } finally { db.endTransaction(txn); } @@ -164,22 +255,27 @@ class MessagingManagerImpl extends ConversationClientImpl @Override public AttachmentHeader addLocalAttachment(GroupId groupId, long timestamp, - String contentType, InputStream is) + String contentType, InputStream in) throws DbException, IOException { - // TODO add real implementation - 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(); - try { - Thread.sleep(1000); - } catch (InterruptedException ignored) { - } - Message m = messageFactory.createMessage(groupId, timestamp, body); - clientHelper.addLocalMessage(m, new BdfDictionary(), false); + // TODO: Support large messages + ByteArrayOutputStream bodyOut = new ByteArrayOutputStream(); + byte[] descriptor = + clientHelper.toByteArray(BdfList.of(ATTACHMENT, contentType)); + bodyOut.write(descriptor); + copyAndClose(in, bodyOut); + if (bodyOut.size() > MAX_MESSAGE_BODY_LENGTH) + throw new FileTooBigException(); + byte[] body = bodyOut.toByteArray(); + BdfDictionary meta = new BdfDictionary(); + meta.put(MSG_KEY_TIMESTAMP, timestamp); + meta.put(MSG_KEY_LOCAL, true); + meta.put(MSG_KEY_MSG_TYPE, ATTACHMENT); + meta.put(MSG_KEY_CONTENT_TYPE, contentType); + meta.put(MSG_KEY_DESCRIPTOR_LENGTH, descriptor.length); + Message m = clientHelper.createMessage(groupId, timestamp, body); + // Mark attachments as temporary, not shared until we're ready to send + db.transaction(false, txn -> + clientHelper.addLocalMessage(txn, m, meta, false, true)); return new AttachmentHeader(m.getId(), contentType); } @@ -242,11 +338,23 @@ class MessagingManagerImpl extends ConversationClientImpl BdfDictionary meta = metadata.get(id); if (meta == null) continue; try { - long timestamp = meta.getLong("timestamp"); - boolean local = meta.getBoolean("local"); - boolean read = meta.getBoolean("read"); - headers.add(new PrivateMessageHeader(id, g, timestamp, local, - read, s.isSent(), s.isSeen(), true, emptyList())); + // Message type is null for version 0.0 private messages + Long messageType = meta.getOptionalLong(MSG_KEY_MSG_TYPE); + if (messageType != null && messageType != PRIVATE_MESSAGE) + continue; + long timestamp = meta.getLong(MSG_KEY_TIMESTAMP); + boolean local = meta.getBoolean(MSG_KEY_LOCAL); + boolean read = meta.getBoolean(MSG_KEY_READ); + if (messageType == null) { + headers.add(new PrivateMessageHeader(id, g, timestamp, + local, read, s.isSent(), s.isSeen(), true, + emptyList())); + } else { + boolean hasText = meta.getBoolean(MSG_KEY_HAS_TEXT); + headers.add(new PrivateMessageHeader(id, g, timestamp, + local, read, s.isSent(), s.isSeen(), hasText, + parseAttachmentHeaders(meta))); + } } catch (FormatException e) { throw new DbException(e); } @@ -257,19 +365,26 @@ class MessagingManagerImpl extends ConversationClientImpl @Override public String getMessageText(MessageId m) throws DbException { try { - // 0: private message text - return clientHelper.getMessageAsList(m).getString(0); + BdfList body = clientHelper.getMessageAsList(m); + if (body.size() == 1) return body.getString(0); // Legacy format + else return body.getOptionalString(1); } catch (FormatException e) { throw new DbException(e); } } @Override - public Attachment getAttachment(MessageId mId) throws DbException { - // TODO add real implementation - Message m = clientHelper.getMessage(mId); - byte[] bytes = m.getBody(); - return new Attachment(new ByteArrayInputStream(bytes)); + public Attachment getAttachment(MessageId m) throws DbException { + // TODO: Support large messages + byte[] body = clientHelper.getMessage(m).getBody(); + try { + BdfDictionary meta = clientHelper.getMessageMetadataAsDictionary(m); + int offset = meta.getLong(MSG_KEY_DESCRIPTOR_LENGTH).intValue(); + return new Attachment(new ByteArrayInputStream(body, offset, + body.length - offset)); + } catch (FormatException e) { + throw new DbException(e); + } } @Override @@ -278,7 +393,7 @@ class MessagingManagerImpl extends ConversationClientImpl int minorVersion = clientVersioningManager .getClientMinorVersion(txn, c, CLIENT_ID, 0); // support was added in 0.1 - return minorVersion == 1; + return minorVersion > 0; } } diff --git a/briar-core/src/main/java/org/briarproject/briar/messaging/MessagingModule.java b/briar-core/src/main/java/org/briarproject/briar/messaging/MessagingModule.java index 8e2e4e042..c9a55cff8 100644 --- a/briar-core/src/main/java/org/briarproject/briar/messaging/MessagingModule.java +++ b/briar-core/src/main/java/org/briarproject/briar/messaging/MessagingModule.java @@ -1,7 +1,7 @@ package org.briarproject.briar.messaging; -import org.briarproject.bramble.api.client.ClientHelper; import org.briarproject.bramble.api.contact.ContactManager; +import org.briarproject.bramble.api.data.BdfReaderFactory; import org.briarproject.bramble.api.data.MetadataEncoder; import org.briarproject.bramble.api.lifecycle.LifecycleManager; import org.briarproject.bramble.api.sync.validation.ValidationManager; @@ -42,10 +42,10 @@ public class MessagingModule { @Provides @Singleton PrivateMessageValidator getValidator(ValidationManager validationManager, - ClientHelper clientHelper, MetadataEncoder metadataEncoder, + BdfReaderFactory bdfReaderFactory, MetadataEncoder metadataEncoder, Clock clock) { PrivateMessageValidator validator = new PrivateMessageValidator( - clientHelper, metadataEncoder, clock); + bdfReaderFactory, metadataEncoder, clock); validationManager.registerMessageValidator(CLIENT_ID, MAJOR_VERSION, validator); return validator; diff --git a/briar-core/src/main/java/org/briarproject/briar/messaging/PrivateMessageFactoryImpl.java b/briar-core/src/main/java/org/briarproject/briar/messaging/PrivateMessageFactoryImpl.java index d2c592787..467540a3c 100644 --- a/briar-core/src/main/java/org/briarproject/briar/messaging/PrivateMessageFactoryImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/messaging/PrivateMessageFactoryImpl.java @@ -12,11 +12,13 @@ import org.briarproject.briar.api.messaging.PrivateMessageFactory; import java.util.List; +import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; import javax.inject.Inject; import static org.briarproject.bramble.util.StringUtils.utf8IsTooLong; import static org.briarproject.briar.api.messaging.MessagingConstants.MAX_PRIVATE_MESSAGE_TEXT_LENGTH; +import static org.briarproject.briar.messaging.MessageTypes.PRIVATE_MESSAGE; @Immutable @NotNullByDefault @@ -30,15 +32,36 @@ class PrivateMessageFactoryImpl implements PrivateMessageFactory { } @Override - public PrivateMessage createPrivateMessage(GroupId groupId, long timestamp, - String text, List attachments) - throws FormatException { + public PrivateMessage createLegacyPrivateMessage(GroupId groupId, + long timestamp, String text) throws FormatException { // Validate the arguments if (utf8IsTooLong(text, MAX_PRIVATE_MESSAGE_TEXT_LENGTH)) throw new IllegalArgumentException(); // Serialise the message - BdfList message = BdfList.of(text); - Message m = clientHelper.createMessage(groupId, timestamp, message); + BdfList body = BdfList.of(text); + Message m = clientHelper.createMessage(groupId, timestamp, body); return new PrivateMessage(m); } + + @Override + public PrivateMessage createPrivateMessage(GroupId groupId, long timestamp, + @Nullable String text, List headers) + throws FormatException { + // Validate the arguments + if (text == null) { + if (headers.isEmpty()) throw new IllegalArgumentException(); + } else if (utf8IsTooLong(text, MAX_PRIVATE_MESSAGE_TEXT_LENGTH)) { + throw new IllegalArgumentException(); + } + // Serialise the attachment headers + BdfList attachmentList = new BdfList(); + for (AttachmentHeader a : headers) { + attachmentList.add( + BdfList.of(a.getMessageId(), a.getContentType())); + } + // Serialise the message + BdfList body = BdfList.of(PRIVATE_MESSAGE, text, attachmentList); + Message m = clientHelper.createMessage(groupId, timestamp, body); + return new PrivateMessage(m, text != null, headers); + } } diff --git a/briar-core/src/main/java/org/briarproject/briar/messaging/PrivateMessageValidator.java b/briar-core/src/main/java/org/briarproject/briar/messaging/PrivateMessageValidator.java index bc1bbcddf..0b4909bf8 100644 --- a/briar-core/src/main/java/org/briarproject/briar/messaging/PrivateMessageValidator.java +++ b/briar-core/src/main/java/org/briarproject/briar/messaging/PrivateMessageValidator.java @@ -1,35 +1,103 @@ package org.briarproject.briar.messaging; import org.briarproject.bramble.api.FormatException; +import org.briarproject.bramble.api.UniqueId; import org.briarproject.bramble.api.client.BdfMessageContext; -import org.briarproject.bramble.api.client.BdfMessageValidator; -import org.briarproject.bramble.api.client.ClientHelper; import org.briarproject.bramble.api.data.BdfDictionary; import org.briarproject.bramble.api.data.BdfList; +import org.briarproject.bramble.api.data.BdfReader; +import org.briarproject.bramble.api.data.BdfReaderFactory; import org.briarproject.bramble.api.data.MetadataEncoder; +import org.briarproject.bramble.api.db.Metadata; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.sync.Group; +import org.briarproject.bramble.api.sync.InvalidMessageException; import org.briarproject.bramble.api.sync.Message; +import org.briarproject.bramble.api.sync.MessageContext; +import org.briarproject.bramble.api.sync.validation.MessageValidator; import org.briarproject.bramble.api.system.Clock; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; + import javax.annotation.concurrent.Immutable; +import static org.briarproject.bramble.api.sync.SyncConstants.MAX_MESSAGE_BODY_LENGTH; +import static org.briarproject.bramble.api.transport.TransportConstants.MAX_CLOCK_DIFFERENCE; import static org.briarproject.bramble.util.ValidationUtils.checkLength; import static org.briarproject.bramble.util.ValidationUtils.checkSize; +import static org.briarproject.briar.api.messaging.MessagingConstants.MAX_ATTACHMENTS_PER_MESSAGE; +import static org.briarproject.briar.api.messaging.MessagingConstants.MAX_CONTENT_TYPE_BYTES; import static org.briarproject.briar.api.messaging.MessagingConstants.MAX_PRIVATE_MESSAGE_TEXT_LENGTH; import static org.briarproject.briar.client.MessageTrackerConstants.MSG_KEY_READ; +import static org.briarproject.briar.messaging.MessageTypes.ATTACHMENT; +import static org.briarproject.briar.messaging.MessageTypes.PRIVATE_MESSAGE; +import static org.briarproject.briar.messaging.MessagingConstants.MSG_KEY_ATTACHMENT_HEADERS; +import static org.briarproject.briar.messaging.MessagingConstants.MSG_KEY_CONTENT_TYPE; +import static org.briarproject.briar.messaging.MessagingConstants.MSG_KEY_DESCRIPTOR_LENGTH; +import static org.briarproject.briar.messaging.MessagingConstants.MSG_KEY_HAS_TEXT; +import static org.briarproject.briar.messaging.MessagingConstants.MSG_KEY_LOCAL; +import static org.briarproject.briar.messaging.MessagingConstants.MSG_KEY_MSG_TYPE; +import static org.briarproject.briar.messaging.MessagingConstants.MSG_KEY_TIMESTAMP; @Immutable @NotNullByDefault -class PrivateMessageValidator extends BdfMessageValidator { +class PrivateMessageValidator implements MessageValidator { - PrivateMessageValidator(ClientHelper clientHelper, + private final BdfReaderFactory bdfReaderFactory; + private final MetadataEncoder metadataEncoder; + private final Clock clock; + + PrivateMessageValidator(BdfReaderFactory bdfReaderFactory, MetadataEncoder metadataEncoder, Clock clock) { - super(clientHelper, metadataEncoder, clock); + this.bdfReaderFactory = bdfReaderFactory; + this.metadataEncoder = metadataEncoder; + this.clock = clock; } @Override - protected BdfMessageContext validateMessage(Message m, Group g, + public MessageContext validateMessage(Message m, Group g) + throws InvalidMessageException { + // Reject the message if it's too far in the future + long now = clock.currentTimeMillis(); + if (m.getTimestamp() - now > MAX_CLOCK_DIFFERENCE) { + throw new InvalidMessageException( + "Timestamp is too far in the future"); + } + try { + // TODO: Support large messages + InputStream in = new ByteArrayInputStream(m.getBody()); + CountingInputStream countIn = + new CountingInputStream(in, MAX_MESSAGE_BODY_LENGTH); + BdfReader reader = bdfReaderFactory.createReader(countIn); + BdfList list = reader.readList(); + long bytesRead = countIn.getBytesRead(); + BdfMessageContext context; + if (list.size() == 1) { + // Legacy private message + if (!reader.eof()) throw new FormatException(); + context = validateLegacyPrivateMessage(m, list); + } else { + // Private message or attachment + int messageType = list.getLong(0).intValue(); + if (messageType == PRIVATE_MESSAGE) { + if (!reader.eof()) throw new FormatException(); + context = validatePrivateMessage(m, list); + } else if (messageType == ATTACHMENT) { + context = validateAttachment(m, list, bytesRead); + } else { + throw new InvalidMessageException(); + } + } + Metadata meta = metadataEncoder.encode(context.getDictionary()); + return new MessageContext(meta, context.getDependencies()); + } catch (IOException e) { + throw new InvalidMessageException(e); + } + } + + private BdfMessageContext validateLegacyPrivateMessage(Message m, BdfList body) throws FormatException { // Private message text checkSize(body, 1); @@ -37,9 +105,54 @@ class PrivateMessageValidator extends BdfMessageValidator { checkLength(text, 0, MAX_PRIVATE_MESSAGE_TEXT_LENGTH); // Return the metadata BdfDictionary meta = new BdfDictionary(); - meta.put("timestamp", m.getTimestamp()); - meta.put("local", false); + meta.put(MSG_KEY_TIMESTAMP, m.getTimestamp()); + meta.put(MSG_KEY_LOCAL, false); meta.put(MSG_KEY_READ, false); return new BdfMessageContext(meta); } + + private BdfMessageContext validatePrivateMessage(Message m, BdfList body) + throws FormatException { + // Message type, optional private message text, attachment headers + checkSize(body, 3); + String text = body.getOptionalString(1); + checkLength(text, 0, MAX_PRIVATE_MESSAGE_TEXT_LENGTH); + BdfList headers = body.getList(2); + if (text == null) checkSize(headers, 1, MAX_ATTACHMENTS_PER_MESSAGE); + else checkSize(headers, 0, MAX_ATTACHMENTS_PER_MESSAGE); + for (int i = 0; i < headers.size(); i++) { + BdfList header = headers.getList(i); + // Message ID, content type + checkSize(header, 2); + byte[] id = header.getRaw(0); + checkLength(id, UniqueId.LENGTH); + String contentType = header.getString(1); + checkLength(contentType, 1, MAX_CONTENT_TYPE_BYTES); + } + // Return the metadata + BdfDictionary meta = new BdfDictionary(); + meta.put(MSG_KEY_TIMESTAMP, m.getTimestamp()); + meta.put(MSG_KEY_LOCAL, false); + meta.put(MSG_KEY_READ, false); + meta.put(MSG_KEY_MSG_TYPE, PRIVATE_MESSAGE); + meta.put(MSG_KEY_HAS_TEXT, text != null); + meta.put(MSG_KEY_ATTACHMENT_HEADERS, headers); + return new BdfMessageContext(meta); + } + + private BdfMessageContext validateAttachment(Message m, BdfList descriptor, + long descriptorLength) throws FormatException { + // Message type, content type + checkSize(descriptor, 2); + String contentType = descriptor.getString(1); + checkLength(contentType, 1, MAX_CONTENT_TYPE_BYTES); + // Return the metadata + BdfDictionary meta = new BdfDictionary(); + meta.put(MSG_KEY_TIMESTAMP, m.getTimestamp()); + meta.put(MSG_KEY_LOCAL, false); + meta.put(MSG_KEY_MSG_TYPE, ATTACHMENT); + meta.put(MSG_KEY_DESCRIPTOR_LENGTH, descriptorLength); + meta.put(MSG_KEY_CONTENT_TYPE, contentType); + return new BdfMessageContext(meta); + } } diff --git a/briar-core/src/main/java/org/briarproject/briar/privategroup/PrivateGroupManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/privategroup/PrivateGroupManagerImpl.java index 2e89dc8cd..27f5fd649 100644 --- a/briar-core/src/main/java/org/briarproject/briar/privategroup/PrivateGroupManagerImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/privategroup/PrivateGroupManagerImpl.java @@ -138,7 +138,7 @@ class PrivateGroupManagerImpl extends BdfIncomingMessageHook meta.put(KEY_TYPE, JOIN.getInt()); meta.put(KEY_INITIAL_JOIN_MSG, creator); addMessageMetadata(meta, m); - clientHelper.addLocalMessage(txn, m.getMessage(), meta, true); + clientHelper.addLocalMessage(txn, m.getMessage(), meta, true, false); messageTracker.trackOutgoingMessage(txn, m.getMessage()); addMember(txn, m.getMessage().getGroupId(), m.getMember(), VISIBLE); setPreviousMsgId(txn, m.getMessage().getGroupId(), @@ -217,7 +217,8 @@ class PrivateGroupManagerImpl extends BdfIncomingMessageHook meta.put(KEY_PARENT_MSG_ID, m.getParent()); addMessageMetadata(meta, m); GroupId g = m.getMessage().getGroupId(); - clientHelper.addLocalMessage(txn, m.getMessage(), meta, true); + clientHelper.addLocalMessage(txn, m.getMessage(), meta, true, + false); // track message setPreviousMsgId(txn, g, m.getMessage().getId()); diff --git a/briar-core/src/main/java/org/briarproject/briar/privategroup/invitation/AbstractProtocolEngine.java b/briar-core/src/main/java/org/briarproject/briar/privategroup/invitation/AbstractProtocolEngine.java index aa018042e..3e5a9842a 100644 --- a/briar-core/src/main/java/org/briarproject/briar/privategroup/invitation/AbstractProtocolEngine.java +++ b/briar-core/src/main/java/org/briarproject/briar/privategroup/invitation/AbstractProtocolEngine.java @@ -93,7 +93,7 @@ abstract class AbstractProtocolEngine boolean isValidDependency(S session, @Nullable MessageId dependency) { MessageId expected = session.getLastRemoteMessageId(); if (dependency == null) return expected == null; - return expected != null && dependency.equals(expected); + return dependency.equals(expected); } void setPrivateGroupVisibility(Transaction txn, S session, @@ -223,7 +223,7 @@ abstract class AbstractProtocolEngine .encodeMetadata(type, privateGroupId, m.getTimestamp(), true, true, visibleInConversation, false, false); try { - clientHelper.addLocalMessage(txn, m, meta, true); + clientHelper.addLocalMessage(txn, m, meta, true, false); } catch (FormatException e) { throw new AssertionError(e); } diff --git a/briar-core/src/main/java/org/briarproject/briar/sharing/ProtocolEngineImpl.java b/briar-core/src/main/java/org/briarproject/briar/sharing/ProtocolEngineImpl.java index b5741d031..473d6a833 100644 --- a/briar-core/src/main/java/org/briarproject/briar/sharing/ProtocolEngineImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/sharing/ProtocolEngineImpl.java @@ -567,7 +567,7 @@ abstract class ProtocolEngineImpl .encodeMetadata(type, shareableId, m.getTimestamp(), true, true, visibleInConversation, false, false); try { - clientHelper.addLocalMessage(txn, m, meta, true); + clientHelper.addLocalMessage(txn, m, meta, true, false); } catch (FormatException e) { throw new AssertionError(e); } @@ -627,7 +627,7 @@ abstract class ProtocolEngineImpl @Nullable MessageId dependency) { MessageId expected = session.getLastRemoteMessageId(); if (dependency == null) return expected != null; - return expected == null || !dependency.equals(expected); + return !dependency.equals(expected); } private long getLocalTimestamp(Session session) { diff --git a/briar-core/src/main/java/org/briarproject/briar/test/TestDataCreatorImpl.java b/briar-core/src/main/java/org/briarproject/briar/test/TestDataCreatorImpl.java index ea9512aa9..cc9c67824 100644 --- a/briar-core/src/main/java/org/briarproject/briar/test/TestDataCreatorImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/test/TestDataCreatorImpl.java @@ -325,7 +325,8 @@ public class TestDataCreatorImpl implements TestDataCreator { Transaction txn = db.startTransaction(false); try { - clientHelper.addLocalMessage(txn, m.getMessage(), meta, true); + clientHelper.addLocalMessage(txn, m.getMessage(), meta, true, + false); if (local) messageTracker.trackOutgoingMessage(txn, m.getMessage()); else messageTracker.trackIncomingMessage(txn, m.getMessage()); db.commitTransaction(txn); @@ -334,13 +335,6 @@ public class TestDataCreatorImpl implements TestDataCreator { } } - @Override - public void addPrivateMessage(Contact contact, String text, long time, - boolean local) throws DbException, FormatException { - Group group = messagingManager.getContactGroup(contact); - createPrivateMessage(group.getId(), text, time, local); - } - private void createBlogPosts(List contacts, int numBlogPosts) throws DbException { for (int i = 0; i < numBlogPosts; i++) { diff --git a/briar-core/src/test/java/org/briarproject/briar/blog/BlogManagerImplTest.java b/briar-core/src/test/java/org/briarproject/briar/blog/BlogManagerImplTest.java index b40573815..6922337fc 100644 --- a/briar-core/src/test/java/org/briarproject/briar/blog/BlogManagerImplTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/blog/BlogManagerImplTest.java @@ -287,7 +287,8 @@ public class BlogManagerImplTest extends BriarTestCase { will(returnValue(blog1)); oneOf(clientHelper).toList(localAuthor1); will(returnValue(authorList1)); - oneOf(clientHelper).addLocalMessage(txn, message, meta, true); + oneOf(clientHelper).addLocalMessage(txn, message, meta, true, + false); oneOf(clientHelper).parseAndValidateAuthor(authorList1); will(returnValue(localAuthor1)); oneOf(contactManager).getAuthorInfo(txn, localAuthor1.getId()); @@ -340,7 +341,8 @@ public class BlogManagerImplTest extends BriarTestCase { will(returnValue(rssBlog)); oneOf(clientHelper).toList(rssLocalAuthor); will(returnValue(rssAuthorList)); - oneOf(clientHelper).addLocalMessage(txn, rssMessage, meta, true); + oneOf(clientHelper).addLocalMessage(txn, rssMessage, meta, true, + false); oneOf(clientHelper).parseAndValidateAuthor(rssAuthorList); will(returnValue(rssLocalAuthor)); oneOf(db).commitTransaction(txn); @@ -407,7 +409,7 @@ public class BlogManagerImplTest extends BriarTestCase { will(returnValue(authorList1)); // Store the comment oneOf(clientHelper).addLocalMessage(txn, commentMsg, commentMeta, - true); + true, false); // Create the headers for the comment and its parent oneOf(clientHelper).parseAndValidateAuthor(authorList1); will(returnValue(localAuthor1)); @@ -508,7 +510,7 @@ public class BlogManagerImplTest extends BriarTestCase { will(returnValue(authorList1)); // Store the wrapped post oneOf(clientHelper).addLocalMessage(txn, wrappedPostMsg, - wrappedPostMeta, true); + wrappedPostMeta, true, false); // Create the comment oneOf(blogPostFactory).createBlogComment(blog2.getId(), localAuthor2, comment, messageId, wrappedPostId); @@ -517,7 +519,7 @@ public class BlogManagerImplTest extends BriarTestCase { will(returnValue(authorList2)); // Store the comment oneOf(clientHelper).addLocalMessage(txn, commentMsg, commentMeta, - true); + true, false); // Create the headers for the comment and the wrapped post oneOf(clientHelper).parseAndValidateAuthor(authorList2); will(returnValue(localAuthor2)); @@ -619,7 +621,7 @@ public class BlogManagerImplTest extends BriarTestCase { will(returnValue(rssAuthorList)); // Store the wrapped post oneOf(clientHelper).addLocalMessage(txn, wrappedPostMsg, - wrappedPostMeta, true); + wrappedPostMeta, true, false); // Create the comment oneOf(blogPostFactory).createBlogComment(blog1.getId(), localAuthor1, comment, rssMessageId, wrappedPostId); @@ -628,7 +630,7 @@ public class BlogManagerImplTest extends BriarTestCase { will(returnValue(authorList1)); // Store the comment oneOf(clientHelper).addLocalMessage(txn, commentMsg, commentMeta, - true); + true, false); // Create the headers for the comment and the wrapped post oneOf(clientHelper).parseAndValidateAuthor(authorList1); will(returnValue(localAuthor1)); @@ -741,7 +743,7 @@ public class BlogManagerImplTest extends BriarTestCase { will(returnValue(rssAuthorList)); // Store the rewrapped post oneOf(clientHelper).addLocalMessage(txn, rewrappedPostMsg, - rewrappedPostMeta, true); + rewrappedPostMeta, true, false); // Wrap the original comment for blog 2 oneOf(clientHelper).getMessageAsList(txn, originalCommentId); will(returnValue(originalCommentBody)); @@ -758,7 +760,7 @@ public class BlogManagerImplTest extends BriarTestCase { will(returnValue(authorList1)); // Store the wrapped comment oneOf(clientHelper).addLocalMessage(txn, wrappedCommentMsg, - wrappedCommentMeta, true); + wrappedCommentMeta, true, false); // Create the new comment oneOf(blogPostFactory).createBlogComment(blog2.getId(), localAuthor2, localComment, originalCommentId, @@ -768,7 +770,7 @@ public class BlogManagerImplTest extends BriarTestCase { will(returnValue(authorList2)); // Store the new comment oneOf(clientHelper).addLocalMessage(txn, localCommentMsg, - localCommentMeta, true); + localCommentMeta, true, false); // Create the headers for the new comment, the wrapped comment and // the rewrapped post oneOf(clientHelper).parseAndValidateAuthor(authorList2); diff --git a/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTest.java b/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTest.java index e8bd96dab..895fb5a16 100644 --- a/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTest.java @@ -1075,8 +1075,8 @@ public class IntroductionIntegrationTest m.getPreviousMessageId(), m.getSessionId(), m.getEphemeralPublicKey(), m.getAcceptTimestamp(), m.getTransportProperties()); - c0.getClientHelper() - .addLocalMessage(txn, msg, new BdfDictionary(), true); + c0.getClientHelper().addLocalMessage(txn, msg, new BdfDictionary(), + true, false); Group group0 = getLocalGroup(); BdfDictionary query = BdfDictionary.of( new BdfEntry(SESSION_KEY_SESSION_ID, m.getSessionId()) diff --git a/briar-core/src/test/java/org/briarproject/briar/messaging/CountingInputStreamTest.java b/briar-core/src/test/java/org/briarproject/briar/messaging/CountingInputStreamTest.java new file mode 100644 index 000000000..7c7d2b294 --- /dev/null +++ b/briar-core/src/test/java/org/briarproject/briar/messaging/CountingInputStreamTest.java @@ -0,0 +1,185 @@ +package org.briarproject.briar.messaging; + +import org.briarproject.bramble.test.BrambleTestCase; +import org.junit.Test; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.util.Random; + +import static org.briarproject.bramble.test.TestUtils.getRandomBytes; +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; + +public class CountingInputStreamTest extends BrambleTestCase { + + private final Random random = new Random(); + private final byte[] src = getRandomBytes(123); + + @Test + public void testCountsSingleByteReads() throws Exception { + InputStream delegate = new ByteArrayInputStream(src); + // The limit is high enough to read the whole src array + CountingInputStream in = + new CountingInputStream(delegate, src.length + 1); + + // No bytes should have been read initially + assertEquals(0L, in.getBytesRead()); + // The reads should return the contents of the src array + for (int i = 0; i < src.length; i++) { + assertEquals(i, in.getBytesRead()); + assertEquals(src[i] & 0xFF, in.read()); + } + // The count should match the length of the src array + assertEquals(src.length, in.getBytesRead()); + // Trying to read another byte should return EOF + assertEquals(-1, in.read()); + // Reading EOF shouldn't affect the count + assertEquals(src.length, in.getBytesRead()); + } + + @Test + public void testCountsMultiByteReads() throws Exception { + InputStream delegate = new ByteArrayInputStream(src); + // The limit is high enough to read the whole src array + CountingInputStream in = + new CountingInputStream(delegate, src.length + 1); + + // No bytes should have been read initially + assertEquals(0L, in.getBytesRead()); + // Copy the src array in random-sized pieces + byte[] dest = new byte[src.length]; + int offset = 0; + while (offset < dest.length) { + assertEquals(offset, in.getBytesRead()); + int length = Math.min(random.nextInt(10), dest.length - offset); + assertEquals(length, in.read(dest, offset, length)); + offset += length; + } + // The dest array should be a copy of the src array + assertArrayEquals(src, dest); + // The count should match the length of the src array + assertEquals(src.length, in.getBytesRead()); + // Trying to read another byte should return EOF + assertEquals(-1, in.read(dest, 0, 1)); + // Reading EOF shouldn't affect the count + assertEquals(src.length, in.getBytesRead()); + } + + @Test + public void testCountsSkips() throws Exception { + InputStream delegate = new ByteArrayInputStream(src); + // The limit is high enough to read the whole src array + CountingInputStream in = + new CountingInputStream(delegate, src.length + 1); + + // No bytes should have been read initially + assertEquals(0L, in.getBytesRead()); + // Skip the src array in random-sized pieces + int offset = 0; + while (offset < src.length) { + assertEquals(offset, in.getBytesRead()); + int length = Math.min(random.nextInt(10), src.length - offset); + assertEquals(length, in.skip(length)); + offset += length; + } + // The count should match the length of the src array + assertEquals(src.length, in.getBytesRead()); + // Trying to skip another byte should return zero + assertEquals(0, in.skip(1)); + // Returning zero shouldn't affect the count + assertEquals(src.length, in.getBytesRead()); + } + + @Test + public void testReturnsEofWhenSingleByteReadReachesLimit() + throws Exception { + InputStream delegate = new ByteArrayInputStream(src); + // The limit is one byte lower than the length of the src array + CountingInputStream in = + new CountingInputStream(delegate, src.length - 1); + + // The reads should return the contents of the src array, except the + // last byte + for (int i = 0; i < src.length - 1; i++) { + assertEquals(src[i] & 0xFF, in.read()); + } + // The count should match the limit + assertEquals(src.length - 1, in.getBytesRead()); + // Trying to read another byte should return EOF + assertEquals(-1, in.read()); + // Reading EOF shouldn't affect the count + assertEquals(src.length - 1, in.getBytesRead()); + } + + @Test + public void testReturnsEofWhenMultiByteReadReachesLimit() throws Exception { + InputStream delegate = new ByteArrayInputStream(src); + // The limit is one byte lower than the length of the src array + CountingInputStream in = + new CountingInputStream(delegate, src.length - 1); + + // Copy the src array in random-sized pieces, except the last two bytes + byte[] dest = new byte[src.length]; + int offset = 0; + while (offset < dest.length - 2) { + int length = Math.min(random.nextInt(10), dest.length - 2 - offset); + assertEquals(length, in.read(dest, offset, length)); + offset += length; + } + // Trying to read two bytes should only return one, reaching the limit + assertEquals(1, in.read(dest, offset, 2)); + // The dest array should be a copy of the src array, except the last + // byte + for (int i = 0; i < src.length - 1; i++) assertEquals(src[i], dest[i]); + // The count should match the limit + assertEquals(src.length - 1, in.getBytesRead()); + // Trying to read another byte should return EOF + assertEquals(-1, in.read(dest, 0, 1)); + // Reading EOF shouldn't affect the count + assertEquals(src.length - 1, in.getBytesRead()); + } + + @Test + public void testReturnsZeroWhenSkipReachesLimit() throws Exception { + InputStream delegate = new ByteArrayInputStream(src); + // The limit is one byte lower than the length of the src array + CountingInputStream in = + new CountingInputStream(delegate, src.length - 1); + + // Skip the src array in random-sized pieces, except the last two bytes + int offset = 0; + while (offset < src.length - 2) { + assertEquals(offset, in.getBytesRead()); + int length = Math.min(random.nextInt(10), src.length - 2 - offset); + assertEquals(length, in.skip(length)); + offset += length; + } + // Trying to skip two bytes should only skip one, reaching the limit + assertEquals(1, in.skip(2)); + // The count should match the limit + assertEquals(src.length - 1, in.getBytesRead()); + // Trying to skip another byte should return zero + assertEquals(0, in.skip(1)); + // Returning zero shouldn't affect the count + assertEquals(src.length - 1, in.getBytesRead()); + } + + @Test + public void testMarkIsNotSupported() { + InputStream delegate = new ByteArrayInputStream(src); + CountingInputStream in = new CountingInputStream(delegate, src.length); + assertFalse(in.markSupported()); + } + + @Test(expected = IOException.class) + public void testResetIsNotSupported() throws Exception { + InputStream delegate = new ByteArrayInputStream(src); + CountingInputStream in = new CountingInputStream(delegate, src.length); + in.mark(src.length); + assertEquals(src.length, in.read(new byte[src.length])); + in.reset(); + } +} diff --git a/briar-core/src/test/java/org/briarproject/briar/messaging/MessageSizeIntegrationTest.java b/briar-core/src/test/java/org/briarproject/briar/messaging/MessageSizeIntegrationTest.java index 1d53b0907..96151f471 100644 --- a/briar-core/src/test/java/org/briarproject/briar/messaging/MessageSizeIntegrationTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/messaging/MessageSizeIntegrationTest.java @@ -8,20 +8,25 @@ import org.briarproject.bramble.api.sync.GroupId; import org.briarproject.bramble.api.sync.MessageId; import org.briarproject.briar.api.forum.ForumPost; import org.briarproject.briar.api.forum.ForumPostFactory; +import org.briarproject.briar.api.messaging.AttachmentHeader; import org.briarproject.briar.api.messaging.PrivateMessage; import org.briarproject.briar.api.messaging.PrivateMessageFactory; import org.briarproject.briar.test.BriarTestCase; import org.junit.Test; +import java.util.ArrayList; +import java.util.List; + import javax.inject.Inject; -import static java.util.Collections.emptyList; import static org.briarproject.bramble.api.identity.AuthorConstants.MAX_AUTHOR_NAME_LENGTH; import static org.briarproject.bramble.api.identity.AuthorConstants.MAX_PUBLIC_KEY_LENGTH; import static org.briarproject.bramble.api.record.Record.MAX_RECORD_PAYLOAD_BYTES; import static org.briarproject.bramble.test.TestUtils.getRandomId; import static org.briarproject.bramble.util.StringUtils.getRandomString; import static org.briarproject.briar.api.forum.ForumConstants.MAX_FORUM_POST_TEXT_LENGTH; +import static org.briarproject.briar.api.messaging.MessagingConstants.MAX_ATTACHMENTS_PER_MESSAGE; +import static org.briarproject.briar.api.messaging.MessagingConstants.MAX_CONTENT_TYPE_BYTES; import static org.briarproject.briar.api.messaging.MessagingConstants.MAX_PRIVATE_MESSAGE_TEXT_LENGTH; import static org.junit.Assert.assertTrue; @@ -43,18 +48,40 @@ public class MessageSizeIntegrationTest extends BriarTestCase { component.inject(this); } + @Test + public void testLegacyPrivateMessageFitsIntoRecord() throws Exception { + // Create a maximum-length private message + GroupId groupId = new GroupId(getRandomId()); + long timestamp = Long.MAX_VALUE; + String text = getRandomString(MAX_PRIVATE_MESSAGE_TEXT_LENGTH); + PrivateMessage message = privateMessageFactory + .createLegacyPrivateMessage(groupId, timestamp, text); + // Check the size of the serialised message + int length = message.getMessage().getRawLength(); + assertTrue(length > UniqueId.LENGTH + 8 + + MAX_PRIVATE_MESSAGE_TEXT_LENGTH); + assertTrue(length <= MAX_RECORD_PAYLOAD_BYTES); + } + @Test public void testPrivateMessageFitsIntoRecord() throws Exception { // Create a maximum-length private message GroupId groupId = new GroupId(getRandomId()); long timestamp = Long.MAX_VALUE; String text = getRandomString(MAX_PRIVATE_MESSAGE_TEXT_LENGTH); + // Create the maximum number of maximum-length attachment headers + List headers = new ArrayList<>(); + for (int i = 0; i < MAX_ATTACHMENTS_PER_MESSAGE; i++) { + headers.add(new AttachmentHeader(new MessageId(getRandomId()), + getRandomString(MAX_CONTENT_TYPE_BYTES))); + } PrivateMessage message = privateMessageFactory.createPrivateMessage( - groupId, timestamp, text, emptyList()); + groupId, timestamp, text, headers); // Check the size of the serialised message int length = message.getMessage().getRawLength(); assertTrue(length > UniqueId.LENGTH + 8 - + MAX_PRIVATE_MESSAGE_TEXT_LENGTH); + + MAX_PRIVATE_MESSAGE_TEXT_LENGTH + MAX_ATTACHMENTS_PER_MESSAGE + * (UniqueId.LENGTH + MAX_CONTENT_TYPE_BYTES)); assertTrue(length <= MAX_RECORD_PAYLOAD_BYTES); } diff --git a/briar-core/src/test/java/org/briarproject/briar/messaging/PrivateMessageValidatorTest.java b/briar-core/src/test/java/org/briarproject/briar/messaging/PrivateMessageValidatorTest.java index 9a279afdb..1f0503f2b 100644 --- a/briar-core/src/test/java/org/briarproject/briar/messaging/PrivateMessageValidatorTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/messaging/PrivateMessageValidatorTest.java @@ -1,83 +1,472 @@ package org.briarproject.briar.messaging; -import org.briarproject.bramble.api.FormatException; -import org.briarproject.bramble.api.client.BdfMessageContext; +import org.briarproject.bramble.api.UniqueId; import org.briarproject.bramble.api.data.BdfDictionary; +import org.briarproject.bramble.api.data.BdfEntry; import org.briarproject.bramble.api.data.BdfList; -import org.briarproject.bramble.test.ValidatorTestCase; +import org.briarproject.bramble.api.data.BdfReader; +import org.briarproject.bramble.api.data.BdfReaderFactory; +import org.briarproject.bramble.api.data.MetadataEncoder; +import org.briarproject.bramble.api.db.Metadata; +import org.briarproject.bramble.api.sync.Group; +import org.briarproject.bramble.api.sync.InvalidMessageException; +import org.briarproject.bramble.api.sync.Message; +import org.briarproject.bramble.api.sync.MessageContext; +import org.briarproject.bramble.api.sync.MessageId; +import org.briarproject.bramble.api.system.Clock; +import org.briarproject.bramble.test.BrambleMockTestCase; +import org.jmock.Expectations; import org.junit.Test; +import java.io.InputStream; + +import static org.briarproject.bramble.api.transport.TransportConstants.MAX_CLOCK_DIFFERENCE; +import static org.briarproject.bramble.test.TestUtils.getClientId; +import static org.briarproject.bramble.test.TestUtils.getGroup; +import static org.briarproject.bramble.test.TestUtils.getMessage; +import static org.briarproject.bramble.test.TestUtils.getRandomBytes; +import static org.briarproject.bramble.test.TestUtils.getRandomId; import static org.briarproject.bramble.util.StringUtils.getRandomString; +import static org.briarproject.briar.api.messaging.MessagingConstants.MAX_ATTACHMENTS_PER_MESSAGE; +import static org.briarproject.briar.api.messaging.MessagingConstants.MAX_CONTENT_TYPE_BYTES; import static org.briarproject.briar.api.messaging.MessagingConstants.MAX_PRIVATE_MESSAGE_TEXT_LENGTH; import static org.briarproject.briar.client.MessageTrackerConstants.MSG_KEY_READ; +import static org.briarproject.briar.messaging.MessageTypes.ATTACHMENT; +import static org.briarproject.briar.messaging.MessageTypes.PRIVATE_MESSAGE; +import static org.briarproject.briar.messaging.MessagingConstants.MSG_KEY_ATTACHMENT_HEADERS; +import static org.briarproject.briar.messaging.MessagingConstants.MSG_KEY_CONTENT_TYPE; +import static org.briarproject.briar.messaging.MessagingConstants.MSG_KEY_DESCRIPTOR_LENGTH; +import static org.briarproject.briar.messaging.MessagingConstants.MSG_KEY_HAS_TEXT; +import static org.briarproject.briar.messaging.MessagingConstants.MSG_KEY_LOCAL; +import static org.briarproject.briar.messaging.MessagingConstants.MSG_KEY_MSG_TYPE; +import static org.briarproject.briar.messaging.MessagingConstants.MSG_KEY_TIMESTAMP; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -public class PrivateMessageValidatorTest extends ValidatorTestCase { +public class PrivateMessageValidatorTest extends BrambleMockTestCase { - @Test(expected = FormatException.class) + private final BdfReaderFactory bdfReaderFactory = + context.mock(BdfReaderFactory.class); + private final MetadataEncoder metadataEncoder = + context.mock(MetadataEncoder.class); + private final Clock clock = context.mock(Clock.class); + private final BdfReader reader = context.mock(BdfReader.class); + + private final Group group = getGroup(getClientId(), 123); + private final Message message = getMessage(group.getId()); + private final long now = message.getTimestamp() + 1000; + private final String text = + getRandomString(MAX_PRIVATE_MESSAGE_TEXT_LENGTH); + private final BdfList attachmentHeader = getAttachmentHeader(); + private final MessageId attachmentId = new MessageId(getRandomId()); + private final String contentType = getRandomString(MAX_CONTENT_TYPE_BYTES); + private final BdfDictionary legacyMeta = BdfDictionary.of( + new BdfEntry(MSG_KEY_TIMESTAMP, message.getTimestamp()), + new BdfEntry(MSG_KEY_LOCAL, false), + new BdfEntry(MSG_KEY_READ, false) + ); + private final BdfDictionary noAttachmentsMeta = BdfDictionary.of( + new BdfEntry(MSG_KEY_TIMESTAMP, message.getTimestamp()), + new BdfEntry(MSG_KEY_LOCAL, false), + new BdfEntry(MSG_KEY_READ, false), + new BdfEntry(MSG_KEY_MSG_TYPE, PRIVATE_MESSAGE), + new BdfEntry(MSG_KEY_HAS_TEXT, true), + new BdfEntry(MSG_KEY_ATTACHMENT_HEADERS, new BdfList()) + ); + private final BdfDictionary noTextMeta = BdfDictionary.of( + new BdfEntry(MSG_KEY_TIMESTAMP, message.getTimestamp()), + new BdfEntry(MSG_KEY_LOCAL, false), + new BdfEntry(MSG_KEY_READ, false), + new BdfEntry(MSG_KEY_MSG_TYPE, PRIVATE_MESSAGE), + new BdfEntry(MSG_KEY_HAS_TEXT, false), + new BdfEntry(MSG_KEY_ATTACHMENT_HEADERS, + BdfList.of(attachmentHeader)) + ); + private final BdfDictionary attachmentMeta = BdfDictionary.of( + new BdfEntry(MSG_KEY_TIMESTAMP, message.getTimestamp()), + new BdfEntry(MSG_KEY_LOCAL, false), + new BdfEntry(MSG_KEY_MSG_TYPE, ATTACHMENT), + // Descriptor length is zero as the test doesn't read from the + // counting input stream + new BdfEntry(MSG_KEY_DESCRIPTOR_LENGTH, 0L), + new BdfEntry(MSG_KEY_CONTENT_TYPE, contentType) + ); + + private final PrivateMessageValidator validator = + new PrivateMessageValidator(bdfReaderFactory, metadataEncoder, + clock); + + @Test(expected = InvalidMessageException.class) + public void testRejectsFarFutureTimestamp() throws Exception { + expectCheckTimestamp(message.getTimestamp() - MAX_CLOCK_DIFFERENCE - 1); + + validator.validateMessage(message, group); + } + + @Test(expected = InvalidMessageException.class) public void testRejectsTooShortBody() throws Exception { - PrivateMessageValidator v = new PrivateMessageValidator(clientHelper, - metadataEncoder, clock); - v.validateMessage(message, group, new BdfList()); + expectCheckTimestamp(now); + expectParseList(new BdfList()); + + validator.validateMessage(message, group); } - @Test(expected = FormatException.class) - public void testRejectsTooLongBody() throws Exception { - PrivateMessageValidator v = new PrivateMessageValidator(clientHelper, - metadataEncoder, clock); - v.validateMessage(message, group, BdfList.of("", 123)); + @Test(expected = InvalidMessageException.class) + public void testRejectsTrailingDataForLegacyMessage() throws Exception { + expectCheckTimestamp(now); + expectParseList(BdfList.of(text)); + expectReadEof(false); + + validator.validateMessage(message, group); } - @Test(expected = FormatException.class) - public void testRejectsNullText() throws Exception { - PrivateMessageValidator v = new PrivateMessageValidator(clientHelper, - metadataEncoder, clock); - v.validateMessage(message, group, BdfList.of((String) null)); + @Test(expected = InvalidMessageException.class) + public void testRejectsNullTextForLegacyMessage() throws Exception { + testRejectsLegacyMessage(BdfList.of((String) null)); } - @Test(expected = FormatException.class) - public void testRejectsNonStringText() throws Exception { - PrivateMessageValidator v = new PrivateMessageValidator(clientHelper, - metadataEncoder, clock); - v.validateMessage(message, group, BdfList.of(123)); + @Test(expected = InvalidMessageException.class) + public void testRejectsNonStringTextForLegacyMessage() throws Exception { + testRejectsLegacyMessage(BdfList.of(123)); } - @Test(expected = FormatException.class) - public void testRejectsTooLongText() throws Exception { - PrivateMessageValidator v = new PrivateMessageValidator(clientHelper, - metadataEncoder, clock); + @Test(expected = InvalidMessageException.class) + public void testRejectsTooLongTextForLegacyMessage() throws Exception { String invalidText = getRandomString(MAX_PRIVATE_MESSAGE_TEXT_LENGTH + 1); - v.validateMessage(message, group, BdfList.of(invalidText)); + + testRejectsLegacyMessage(BdfList.of(invalidText)); } @Test - public void testAcceptsMaxLengthText() throws Exception { - PrivateMessageValidator v = new PrivateMessageValidator(clientHelper, - metadataEncoder, clock); - String text = getRandomString(MAX_PRIVATE_MESSAGE_TEXT_LENGTH); - BdfMessageContext messageContext = - v.validateMessage(message, group, BdfList.of(text)); - assertExpectedContext(messageContext); + public void testAcceptsMaxLengthTextForLegacyMessage() throws Exception { + testAcceptsLegacyMessage(BdfList.of(text)); } @Test - public void testAcceptsMinLengthText() throws Exception { - PrivateMessageValidator v = new PrivateMessageValidator(clientHelper, - metadataEncoder, clock); - BdfMessageContext messageContext = - v.validateMessage(message, group, BdfList.of("")); - assertExpectedContext(messageContext); + public void testAcceptsMinLengthTextForLegacyMessage() throws Exception { + testAcceptsLegacyMessage(BdfList.of("")); } - private void assertExpectedContext(BdfMessageContext messageContext) - throws FormatException { - BdfDictionary meta = messageContext.getDictionary(); - assertEquals(3, meta.size()); - assertEquals(timestamp, meta.getLong("timestamp").longValue()); - assertFalse(meta.getBoolean("local")); - assertFalse(meta.getBoolean(MSG_KEY_READ)); - assertEquals(0, messageContext.getDependencies().size()); + @Test(expected = InvalidMessageException.class) + public void testRejectsTrailingDataForPrivateMessage() throws Exception { + expectCheckTimestamp(now); + expectParseList(BdfList.of(PRIVATE_MESSAGE, text, new BdfList())); + expectReadEof(false); + + validator.validateMessage(message, group); + } + + @Test(expected = InvalidMessageException.class) + public void testRejectsTooShortBodyForPrivateMessage() throws Exception { + testRejectsPrivateMessage(BdfList.of(PRIVATE_MESSAGE, text)); + } + + @Test(expected = InvalidMessageException.class) + public void testRejectsTooLongBodyForPrivateMessage() throws Exception { + testRejectsPrivateMessage( + BdfList.of(PRIVATE_MESSAGE, text, new BdfList(), 123)); + } + + @Test(expected = InvalidMessageException.class) + public void testRejectsNullTextWithoutAttachmentsForPrivateMessage() + throws Exception { + testRejectsPrivateMessage( + BdfList.of(PRIVATE_MESSAGE, null, new BdfList())); + } + + @Test + public void testAcceptsNullTextWithAttachmentsForPrivateMessage() + throws Exception { + testAcceptsPrivateMessage(BdfList.of(PRIVATE_MESSAGE, null, + BdfList.of(attachmentHeader)), noTextMeta); + } + + @Test(expected = InvalidMessageException.class) + public void testRejectsNonStringTextForPrivateMessage() throws Exception { + testRejectsPrivateMessage( + BdfList.of(PRIVATE_MESSAGE, 123, new BdfList())); + } + + @Test(expected = InvalidMessageException.class) + public void testRejectsTooLongTextForPrivateMessage() throws Exception { + String invalidText = + getRandomString(MAX_PRIVATE_MESSAGE_TEXT_LENGTH + 1); + + testRejectsPrivateMessage( + BdfList.of(PRIVATE_MESSAGE, invalidText, new BdfList())); + } + + @Test + public void testAcceptsMaxLengthTextForPrivateMessage() throws Exception { + testAcceptsPrivateMessage(BdfList.of(PRIVATE_MESSAGE, text, + new BdfList()), noAttachmentsMeta); + } + + @Test + public void testAcceptsMinLengthTextForPrivateMessage() throws Exception { + testAcceptsPrivateMessage(BdfList.of(PRIVATE_MESSAGE, "", + new BdfList()), noAttachmentsMeta); + } + + @Test(expected = InvalidMessageException.class) + public void testRejectsNullAttachmentListForPrivateMessage() + throws Exception { + testRejectsPrivateMessage(BdfList.of(PRIVATE_MESSAGE, text, null)); + } + + @Test(expected = InvalidMessageException.class) + public void testRejectsNonListAttachmentListForPrivateMessage() + throws Exception { + testRejectsPrivateMessage(BdfList.of(PRIVATE_MESSAGE, text, 123)); + } + + @Test(expected = InvalidMessageException.class) + public void testRejectsTooLongAttachmentListForPrivateMessage() + throws Exception { + BdfList invalidList = new BdfList(); + for (int i = 0; i < MAX_ATTACHMENTS_PER_MESSAGE + 1; i++) { + invalidList.add(getAttachmentHeader()); + } + + testRejectsPrivateMessage( + BdfList.of(PRIVATE_MESSAGE, text, invalidList)); + } + + @Test + public void testAcceptsMaxLengthAttachmentListForPrivateMessage() + throws Exception { + BdfList attachmentList = new BdfList(); + for (int i = 0; i < MAX_ATTACHMENTS_PER_MESSAGE; i++) { + attachmentList.add(getAttachmentHeader()); + } + BdfDictionary maxAttachmentsMeta = new BdfDictionary(noAttachmentsMeta); + maxAttachmentsMeta.put(MSG_KEY_ATTACHMENT_HEADERS, attachmentList); + + testAcceptsPrivateMessage(BdfList.of(PRIVATE_MESSAGE, text, + attachmentList), maxAttachmentsMeta); + } + + @Test(expected = InvalidMessageException.class) + public void testRejectsNullAttachmentIdForPrivateMessage() + throws Exception { + BdfList invalidHeader = BdfList.of(null, contentType); + + testRejectsPrivateMessage(BdfList.of(PRIVATE_MESSAGE, text, + BdfList.of(invalidHeader))); + } + + @Test(expected = InvalidMessageException.class) + public void testRejectsNonRawAttachmentIdForPrivateMessage() + throws Exception { + BdfList invalidHeader = BdfList.of(123, contentType); + + testRejectsPrivateMessage(BdfList.of(PRIVATE_MESSAGE, text, + BdfList.of(invalidHeader))); + } + + @Test(expected = InvalidMessageException.class) + public void testRejectsTooShortAttachmentIdForPrivateMessage() + throws Exception { + BdfList invalidHeader = + BdfList.of(getRandomBytes(UniqueId.LENGTH - 1), contentType); + + testRejectsPrivateMessage(BdfList.of(PRIVATE_MESSAGE, text, + BdfList.of(invalidHeader))); + } + + @Test(expected = InvalidMessageException.class) + public void testRejectsTooLongAttachmentIdForPrivateMessage() + throws Exception { + BdfList invalidHeader = + BdfList.of(getRandomBytes(UniqueId.LENGTH + 1), contentType); + + testRejectsPrivateMessage(BdfList.of(PRIVATE_MESSAGE, text, + BdfList.of(invalidHeader))); + } + + + @Test(expected = InvalidMessageException.class) + public void testRejectsNullContentTypeForPrivateMessage() + throws Exception { + BdfList invalidHeader = BdfList.of(attachmentId, null); + + testRejectsPrivateMessage(BdfList.of(PRIVATE_MESSAGE, text, + BdfList.of(invalidHeader))); + } + + @Test(expected = InvalidMessageException.class) + public void testRejectsNonStringContentTypeForPrivateMessage() + throws Exception { + BdfList invalidHeader = BdfList.of(attachmentId, 123); + + testRejectsPrivateMessage(BdfList.of(PRIVATE_MESSAGE, text, + BdfList.of(invalidHeader))); + } + + @Test(expected = InvalidMessageException.class) + public void testRejectsTooShortContentTypeForPrivateMessage() + throws Exception { + BdfList invalidHeader = BdfList.of(attachmentId, ""); + + testRejectsPrivateMessage(BdfList.of(PRIVATE_MESSAGE, text, + BdfList.of(invalidHeader))); + } + + @Test(expected = InvalidMessageException.class) + public void testRejectsTooLongContentTypeForPrivateMessage() + throws Exception { + BdfList invalidHeader = BdfList.of(attachmentId, + getRandomString(MAX_CONTENT_TYPE_BYTES + 1)); + + testRejectsPrivateMessage(BdfList.of(PRIVATE_MESSAGE, text, + BdfList.of(invalidHeader))); + } + + @Test(expected = InvalidMessageException.class) + public void testRejectsTooShortDescriptorWithoutTrailingDataForAttachment() + throws Exception { + expectCheckTimestamp(now); + expectParseList(BdfList.of(ATTACHMENT)); + // Single-element list is interpreted as a legacy private message, so + // EOF is expected + expectReadEof(true); + + validator.validateMessage(message, group); + } + + @Test(expected = InvalidMessageException.class) + public void testRejectsTooShortDescriptorWithTrailingDataForAttachment() + throws Exception { + expectCheckTimestamp(now); + expectParseList(BdfList.of(ATTACHMENT)); + // Single-element list is interpreted as a legacy private message, so + // EOF is expected + expectReadEof(false); + + validator.validateMessage(message, group); + } + + @Test(expected = InvalidMessageException.class) + public void testRejectsTooLongDescriptorForAttachment() throws Exception { + testRejectsAttachment(BdfList.of(ATTACHMENT, contentType, 123)); + } + + @Test(expected = InvalidMessageException.class) + public void testRejectsNullContentTypeForAttachment() throws Exception { + testRejectsAttachment(BdfList.of(ATTACHMENT, null)); + } + + @Test(expected = InvalidMessageException.class) + public void testRejectsNonStringContentTypeForAttachment() + throws Exception { + testRejectsAttachment(BdfList.of(ATTACHMENT, 123)); + } + + @Test(expected = InvalidMessageException.class) + public void testRejectsTooShortContentTypeForAttachment() throws Exception { + testRejectsAttachment(BdfList.of(ATTACHMENT, "")); + } + + @Test(expected = InvalidMessageException.class) + public void testRejectsTooLongContentTypeForAttachment() throws Exception { + String invalidContentType = getRandomString(MAX_CONTENT_TYPE_BYTES + 1); + + testRejectsAttachment(BdfList.of(ATTACHMENT, invalidContentType)); + } + + @Test + public void testAcceptsValidDescriptorForAttachment() throws Exception { + expectCheckTimestamp(now); + expectParseList(BdfList.of(ATTACHMENT, contentType)); + expectEncodeMetadata(attachmentMeta); + + validator.validateMessage(message, group); + } + + @Test(expected = InvalidMessageException.class) + public void testRejectsUnknownMessageType() throws Exception { + expectCheckTimestamp(now); + expectParseList(BdfList.of(ATTACHMENT + 1, contentType)); + + validator.validateMessage(message, group); + } + + private void testRejectsLegacyMessage(BdfList body) throws Exception { + expectCheckTimestamp(now); + expectParseList(body); + expectReadEof(true); + + validator.validateMessage(message, group); + } + + private void testAcceptsLegacyMessage(BdfList body) throws Exception { + expectCheckTimestamp(now); + expectParseList(body); + expectReadEof(true); + expectEncodeMetadata(legacyMeta); + + MessageContext result = validator.validateMessage(message, group); + assertEquals(0, result.getDependencies().size()); + } + + private void testRejectsPrivateMessage(BdfList body) throws Exception { + expectCheckTimestamp(now); + expectParseList(body); + expectReadEof(true); + + validator.validateMessage(message, group); + } + + private void testAcceptsPrivateMessage(BdfList body, BdfDictionary meta) + throws Exception { + expectCheckTimestamp(now); + expectParseList(body); + expectReadEof(true); + expectEncodeMetadata(meta); + + MessageContext result = validator.validateMessage(message, group); + assertEquals(0, result.getDependencies().size()); + } + + private void testRejectsAttachment(BdfList descriptor) throws Exception { + expectCheckTimestamp(now); + expectParseList(descriptor); + + validator.validateMessage(message, group); + } + + private void expectCheckTimestamp(long now) { + context.checking(new Expectations() {{ + oneOf(clock).currentTimeMillis(); + will(returnValue(now)); + }}); + } + + private void expectParseList(BdfList body) throws Exception { + context.checking(new Expectations() {{ + oneOf(bdfReaderFactory).createReader(with(any(InputStream.class))); + will(returnValue(reader)); + oneOf(reader).readList(); + will(returnValue(body)); + }}); + } + + private void expectReadEof(boolean eof) throws Exception { + context.checking(new Expectations() {{ + oneOf(reader).eof(); + will(returnValue(eof)); + }}); + } + + private void expectEncodeMetadata(BdfDictionary meta) throws Exception { + context.checking(new Expectations() {{ + oneOf(metadataEncoder).encode(meta); + will(returnValue(new Metadata())); + }}); + } + + private BdfList getAttachmentHeader() { + return BdfList.of(new MessageId(getRandomId()), + getRandomString(MAX_CONTENT_TYPE_BYTES)); } } diff --git a/briar-core/src/test/java/org/briarproject/briar/messaging/SimplexMessagingIntegrationTest.java b/briar-core/src/test/java/org/briarproject/briar/messaging/SimplexMessagingIntegrationTest.java index 0b2f07309..50a00290a 100644 --- a/briar-core/src/test/java/org/briarproject/briar/messaging/SimplexMessagingIntegrationTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/messaging/SimplexMessagingIntegrationTest.java @@ -15,9 +15,11 @@ import org.briarproject.bramble.api.sync.event.MessageStateChangedEvent; import org.briarproject.bramble.test.TestDatabaseConfigModule; import org.briarproject.bramble.test.TestTransportConnectionReader; import org.briarproject.bramble.test.TestTransportConnectionWriter; +import org.briarproject.briar.api.messaging.AttachmentHeader; import org.briarproject.briar.api.messaging.MessagingManager; import org.briarproject.briar.api.messaging.PrivateMessage; import org.briarproject.briar.api.messaging.PrivateMessageFactory; +import org.briarproject.briar.api.messaging.event.AttachmentReceivedEvent; import org.briarproject.briar.api.messaging.event.PrivateMessageReceivedEvent; import org.briarproject.briar.test.BriarTestCase; import org.junit.After; @@ -27,9 +29,10 @@ import org.junit.Test; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.File; +import java.io.InputStream; import java.util.concurrent.CountDownLatch; -import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; import static java.util.concurrent.TimeUnit.MILLISECONDS; import static org.briarproject.bramble.api.sync.validation.MessageState.DELIVERED; import static org.briarproject.bramble.test.TestPluginConfigModule.SIMPLEX_TRANSPORT_ID; @@ -84,10 +87,12 @@ public class SimplexMessagingIntegrationTest extends BriarTestCase { read(bob, write(alice, bobId), 2); // Sync Bob's client versions and transport properties read(alice, write(bob, aliceId), 2); - // Sync the private message - read(bob, write(alice, bobId), 1); + // Sync the private message and the attachment + read(bob, write(alice, bobId), 2); // Bob should have received the private message assertTrue(listener.messageAdded); + // Bob should have received the attachment + assertTrue(listener.attachmentAdded); } private ContactId setUp(SimplexMessagingIntegrationTestComponent device, @@ -107,16 +112,20 @@ public class SimplexMessagingIntegrationTest extends BriarTestCase { private void sendMessage(SimplexMessagingIntegrationTestComponent device, ContactId contactId) throws Exception { - // Send Bob a message MessagingManager messagingManager = device.getMessagingManager(); GroupId groupId = messagingManager.getConversationId(contactId); + long timestamp = System.currentTimeMillis(); + InputStream in = new ByteArrayInputStream(new byte[] {0, 1, 2, 3}); + AttachmentHeader attachmentHeader = messagingManager.addLocalAttachment( + groupId, timestamp, "image/png", in); PrivateMessageFactory privateMessageFactory = device.getPrivateMessageFactory(); PrivateMessage message = privateMessageFactory.createPrivateMessage( - groupId, System.currentTimeMillis(), "Hi!", emptyList()); + groupId, timestamp, "Hi!", singletonList(attachmentHeader)); messagingManager.addLocalMessage(message); } + @SuppressWarnings("SameParameterValue") private void read(SimplexMessagingIntegrationTestComponent device, byte[] stream, int deliveries) throws Exception { // Listen for message deliveries @@ -187,10 +196,15 @@ public class SimplexMessagingIntegrationTest extends BriarTestCase { private static class PrivateMessageListener implements EventListener { private volatile boolean messageAdded = false; + private volatile boolean attachmentAdded = false; @Override public void eventOccurred(Event e) { - if (e instanceof PrivateMessageReceivedEvent) messageAdded = true; + if (e instanceof PrivateMessageReceivedEvent) { + messageAdded = true; + } else if (e instanceof AttachmentReceivedEvent) { + attachmentAdded = true; + } } } } diff --git a/briar-core/src/test/java/org/briarproject/briar/privategroup/invitation/AbstractProtocolEngineTest.java b/briar-core/src/test/java/org/briarproject/briar/privategroup/invitation/AbstractProtocolEngineTest.java index 5882233f0..262327aa2 100644 --- a/briar-core/src/test/java/org/briarproject/briar/privategroup/invitation/AbstractProtocolEngineTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/privategroup/invitation/AbstractProtocolEngineTest.java @@ -169,7 +169,8 @@ abstract class AbstractProtocolEngineTest extends BrambleMockTestCase { oneOf(messageEncoder).encodeMetadata(type, privateGroupId, message.getTimestamp(), true, true, visible, false, false); will(returnValue(meta)); - oneOf(clientHelper).addLocalMessage(txn, message, meta, true); + oneOf(clientHelper).addLocalMessage(txn, message, meta, true, + false); }}); } diff --git a/briar-headless/src/main/java/org/briarproject/briar/headless/event/OutputEvent.kt b/briar-headless/src/main/java/org/briarproject/briar/headless/event/OutputEvent.kt index 976749b40..0926bcf6c 100644 --- a/briar-headless/src/main/java/org/briarproject/briar/headless/event/OutputEvent.kt +++ b/briar-headless/src/main/java/org/briarproject/briar/headless/event/OutputEvent.kt @@ -20,7 +20,7 @@ internal class OutputEvent(val name: String, val data: JsonDict) { val type = "event" } -internal fun ConversationMessageReceivedEvent<*>.output(text: String): JsonDict { +internal fun ConversationMessageReceivedEvent<*>.output(text: String?): JsonDict { check(messageHeader is PrivateMessageHeader) return (messageHeader as PrivateMessageHeader).output(contactId, text) } diff --git a/briar-headless/src/main/java/org/briarproject/briar/headless/messaging/MessagingControllerImpl.kt b/briar-headless/src/main/java/org/briarproject/briar/headless/messaging/MessagingControllerImpl.kt index e3bb602cf..4d4742687 100644 --- a/briar-headless/src/main/java/org/briarproject/briar/headless/messaging/MessagingControllerImpl.kt +++ b/briar-headless/src/main/java/org/briarproject/briar/headless/messaging/MessagingControllerImpl.kt @@ -67,16 +67,16 @@ constructor( override fun write(ctx: Context): Context { val contact = getContact(ctx) - val message = ctx.getFromJson(objectMapper, "text") - if (utf8IsTooLong(message, MAX_PRIVATE_MESSAGE_TEXT_LENGTH)) + val text = ctx.getFromJson(objectMapper, "text") + if (utf8IsTooLong(text, MAX_PRIVATE_MESSAGE_TEXT_LENGTH)) throw BadRequestResponse("Message text is too long") val group = messagingManager.getContactGroup(contact) val now = clock.currentTimeMillis() - val m = privateMessageFactory.createPrivateMessage(group.id, now, message, emptyList()) + val m = privateMessageFactory.createPrivateMessage(group.id, now, text, emptyList()) messagingManager.addLocalMessage(m) - return ctx.json(m.output(contact.id, message)) + return ctx.json(m.output(contact.id, text)) } override fun eventOccurred(e: Event) { diff --git a/briar-headless/src/test/java/org/briarproject/briar/headless/messaging/MessagingControllerImplTest.kt b/briar-headless/src/test/java/org/briarproject/briar/headless/messaging/MessagingControllerImplTest.kt index d5ab17dc9..cf3ec733b 100644 --- a/briar-headless/src/test/java/org/briarproject/briar/headless/messaging/MessagingControllerImplTest.kt +++ b/briar-headless/src/test/java/org/briarproject/briar/headless/messaging/MessagingControllerImplTest.kt @@ -169,7 +169,12 @@ internal class MessagingControllerImplTest : ControllerTest() { val event = PrivateMessageReceivedEvent(header, contact.id) every { messagingManager.getMessageText(message.id) } returns text - every { webSocketController.sendEvent(EVENT_CONVERSATION_MESSAGE, event.output(text)) } just runs + every { + webSocketController.sendEvent( + EVENT_CONVERSATION_MESSAGE, + event.output(text) + ) + } just runs controller.eventOccurred(event) }