Upgrade messaging client to support attachments.

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();
 	}
 

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);

briar-core/src/main/java/org/briarproject/briar/introduction/AbstractProtocolEngine.java

@@ -140,7 +140,7 @@ abstract class AbstractProtocolEngine<S extends Session>
 				.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<S extends Session>
 	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) {

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;
+	}
+}

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;
+}

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";
+}

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<AttachmentHeader> 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<AttachmentHeader> 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<AttachmentHeader> parseAttachmentHeaders(BdfDictionary meta)
+			throws FormatException {
+		BdfList attachmentHeaders = meta.getList(MSG_KEY_ATTACHMENT_HEADERS);
+		int length = attachmentHeaders.size();
+		List<AttachmentHeader> 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;
 	}
 
 }

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;

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<AttachmentHeader> 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<AttachmentHeader> 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);
+	}
 }

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);
+	}
 }

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());

briar-core/src/main/java/org/briarproject/briar/privategroup/invitation/AbstractProtocolEngine.java

@@ -93,7 +93,7 @@ abstract class AbstractProtocolEngine<S extends Session>
 	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<S extends Session>
 				.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);
 		}

briar-core/src/main/java/org/briarproject/briar/sharing/ProtocolEngineImpl.java

@@ -567,7 +567,7 @@ abstract class ProtocolEngineImpl<S extends Shareable>
 				.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<S extends Shareable>
 			@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) {

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<Contact> contacts, int numBlogPosts)
 			throws DbException {
 		for (int i = 0; i < numBlogPosts; i++) {

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);

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())

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();
+	}
+}

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<AttachmentHeader> 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);
 	}
 

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));
 	}
 }

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;
+			}
 		}
 	}
 }

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);
 		}});
 	}