Merge branch 'transactional-attachment-reader' into 'master'

Transactional version of AttachmentReader#getAttachment()

See merge request briar/briar!1570

briar-api/src/main/java/org/briarproject/briar/api/attachment/AttachmentReader.java

@@ -2,6 +2,7 @@ package org.briarproject.briar.api.attachment;
 
 import org.briarproject.bramble.api.db.DbException;
 import org.briarproject.bramble.api.db.NoSuchMessageException;
+import org.briarproject.bramble.api.db.Transaction;
 
 public interface AttachmentReader {
 
@@ -17,4 +18,17 @@ public interface AttachmentReader {
 	 */
 	Attachment getAttachment(AttachmentHeader h) throws DbException;
 
+	/**
+	 * Returns the attachment with the given attachment header.
+	 *
+	 * @throws NoSuchMessageException If the header refers to a message in
+	 * a different group from the one specified in the header, to a message
+	 * that is not an attachment, or to an attachment that does not have the
+	 * expected content type. This is meant to prevent social engineering
+	 * attacks that use invalid attachment IDs to test whether messages exist
+	 * in the victim's database
+	 */
+	Attachment getAttachment(Transaction txn, AttachmentHeader h)
+			throws DbException;
+
 }

briar-core/src/main/java/org/briarproject/briar/attachment/AttachmentReaderImpl.java

@@ -5,6 +5,8 @@ import org.briarproject.bramble.api.client.ClientHelper;
 import org.briarproject.bramble.api.data.BdfDictionary;
 import org.briarproject.bramble.api.db.DbException;
 import org.briarproject.bramble.api.db.NoSuchMessageException;
+import org.briarproject.bramble.api.db.Transaction;
+import org.briarproject.bramble.api.db.TransactionManager;
 import org.briarproject.bramble.api.sync.Message;
 import org.briarproject.bramble.api.sync.MessageId;
 import org.briarproject.briar.api.attachment.Attachment;
@@ -21,18 +23,27 @@ import static org.briarproject.briar.api.attachment.MediaConstants.MSG_KEY_DESCR
 
 public class AttachmentReaderImpl implements AttachmentReader {
 
+	private final TransactionManager db;
 	private final ClientHelper clientHelper;
 
 	@Inject
-	public AttachmentReaderImpl(ClientHelper clientHelper) {
+	public AttachmentReaderImpl(TransactionManager db,
+			ClientHelper clientHelper) {
+		this.db = db;
 		this.clientHelper = clientHelper;
 	}
 
 	@Override
 	public Attachment getAttachment(AttachmentHeader h) throws DbException {
+		return db.transactionWithResult(true, txn -> getAttachment(txn, h));
+	}
+
+	@Override
+	public Attachment getAttachment(Transaction txn, AttachmentHeader h)
+			throws DbException {
 		// TODO: Support large messages
 		MessageId m = h.getMessageId();
-		Message message = clientHelper.getMessage(m);
+		Message message = clientHelper.getMessage(txn, m);
 		// Check that the message is in the expected group, to prevent it from
 		// being loaded in the context of a different group
 		if (!message.getGroupId().equals(h.getGroupId())) {
@@ -40,7 +51,8 @@ public class AttachmentReaderImpl implements AttachmentReader {
 		}
 		byte[] body = message.getBody();
 		try {
-			BdfDictionary meta = clientHelper.getMessageMetadataAsDictionary(m);
+			BdfDictionary meta =
+					clientHelper.getMessageMetadataAsDictionary(txn, m);
 			String contentType = meta.getString(MSG_KEY_CONTENT_TYPE);
 			if (!contentType.equals(h.getContentType()))
 				throw new NoSuchMessageException();

briar-core/src/test/java/org/briarproject/briar/attachment/AttachmentReaderImplTest.java

@@ -3,13 +3,16 @@ package org.briarproject.briar.attachment;
 import org.briarproject.bramble.api.client.ClientHelper;
 import org.briarproject.bramble.api.data.BdfDictionary;
 import org.briarproject.bramble.api.data.BdfEntry;
+import org.briarproject.bramble.api.db.DatabaseComponent;
 import org.briarproject.bramble.api.db.NoSuchMessageException;
+import org.briarproject.bramble.api.db.Transaction;
+import org.briarproject.bramble.api.db.TransactionManager;
 import org.briarproject.bramble.api.sync.GroupId;
 import org.briarproject.bramble.api.sync.Message;
 import org.briarproject.bramble.test.BrambleMockTestCase;
+import org.briarproject.bramble.test.DbExpectations;
 import org.briarproject.briar.api.attachment.Attachment;
 import org.briarproject.briar.api.attachment.AttachmentHeader;
-import org.jmock.Expectations;
 import org.junit.Test;
 
 import java.io.ByteArrayOutputStream;
@@ -25,6 +28,7 @@ import static org.junit.Assert.assertArrayEquals;
 
 public class AttachmentReaderImplTest extends BrambleMockTestCase {
 
+	private final TransactionManager db = context.mock(DatabaseComponent.class);
 	private final ClientHelper clientHelper = context.mock(ClientHelper.class);
 
 	private final GroupId groupId = new GroupId(getRandomId());
@@ -34,7 +38,7 @@ public class AttachmentReaderImplTest extends BrambleMockTestCase {
 			message.getId(), contentType);
 
 	private final AttachmentReaderImpl attachmentReader =
-			new AttachmentReaderImpl(clientHelper);
+			new AttachmentReaderImpl(db, clientHelper);
 
 	@Test(expected = NoSuchMessageException.class)
 	public void testWrongGroup() throws Exception {
@@ -42,8 +46,11 @@ public class AttachmentReaderImplTest extends BrambleMockTestCase {
 		AttachmentHeader wrongGroup = new AttachmentHeader(wrongGroupId,
 				message.getId(), contentType);
 
-		context.checking(new Expectations() {{
-			oneOf(clientHelper).getMessage(message.getId());
+		Transaction txn = new Transaction(null, true);
+
+		context.checking(new DbExpectations() {{
+			oneOf(db).transactionWithResult(with(true), withDbCallable(txn));
+			oneOf(clientHelper).getMessage(txn, message.getId());
 			will(returnValue(message));
 		}});
 
@@ -74,10 +81,14 @@ public class AttachmentReaderImplTest extends BrambleMockTestCase {
 	}
 
 	private void testInvalidMetadata(BdfDictionary meta) throws Exception {
-		context.checking(new Expectations() {{
-			oneOf(clientHelper).getMessage(message.getId());
+		Transaction txn = new Transaction(null, true);
+
+		context.checking(new DbExpectations() {{
+			oneOf(db).transactionWithResult(with(true), withDbCallable(txn));
+			oneOf(clientHelper).getMessage(txn, message.getId());
 			will(returnValue(message));
-			oneOf(clientHelper).getMessageMetadataAsDictionary(message.getId());
+			oneOf(clientHelper)
+					.getMessageMetadataAsDictionary(txn, message.getId());
 			will(returnValue(meta));
 		}});
 
@@ -95,10 +106,14 @@ public class AttachmentReaderImplTest extends BrambleMockTestCase {
 		byte[] expectedData = new byte[body.length - descriptorLength];
 		arraycopy(body, descriptorLength, expectedData, 0, expectedData.length);
 
-		context.checking(new Expectations() {{
-			oneOf(clientHelper).getMessage(message.getId());
+		Transaction txn = new Transaction(null, true);
+
+		context.checking(new DbExpectations() {{
+			oneOf(db).transactionWithResult(with(true), withDbCallable(txn));
+			oneOf(clientHelper).getMessage(txn, message.getId());
 			will(returnValue(message));
-			oneOf(clientHelper).getMessageMetadataAsDictionary(message.getId());
+			oneOf(clientHelper)
+					.getMessageMetadataAsDictionary(txn, message.getId());
 			will(returnValue(meta));
 		}});