Removed signatures from headers and bundles, since the transport's

authentication will make them redundant.

test/build.xml

@@ -20,6 +20,7 @@
 			<test name='net.sf.briar.i18n.FontManagerTest'/>
 			<test name='net.sf.briar.i18n.I18nTest'/>
 			<test name='net.sf.briar.invitation.InvitationWorkerTest'/>
+			<test name='net.sf.briar.protocol.BundleReaderImplTest'/>
 			<test name='net.sf.briar.protocol.BundleReadWriteTest'/>
 			<test name='net.sf.briar.protocol.ConsumersTest'/>
 			<test name='net.sf.briar.protocol.SigningDigestingOutputStreamTest'/>

test/net/sf/briar/protocol/BundleReadWriteTest.java

@@ -3,8 +3,6 @@ package net.sf.briar.protocol;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
-import java.io.RandomAccessFile;
-import java.security.GeneralSecurityException;
 import java.security.KeyFactory;
 import java.security.KeyPair;
 import java.security.KeyPairGenerator;
@@ -66,25 +64,23 @@ public class BundleReadWriteTest extends TestCase {
 	private final String nick = "Foo Bar";
 	private final String messageBody = "This is the message body! Wooooooo!";
 
-	private final ReaderFactory rf;
-	private final WriterFactory wf;
-
-	private final KeyPair keyPair;
-	private final Signature sig, sig1;
-	private final MessageDigest dig, dig1;
+	private final ReaderFactory readerFactory;
+	private final WriterFactory writerFactory;
+	private final Signature signature;
+	private final MessageDigest messageDigest, batchDigest;
 	private final KeyParser keyParser;
 	private final Message message;
 
 	public BundleReadWriteTest() throws Exception {
 		super();
+		// Inject the reader and writer factories, since they belong to
+		// a different component
 		Injector i = Guice.createInjector(new SerialModule());
-		rf = i.getInstance(ReaderFactory.class);
-		wf = i.getInstance(WriterFactory.class);
-		keyPair = KeyPairGenerator.getInstance(KEY_PAIR_ALGO).generateKeyPair();
-		sig = Signature.getInstance(SIGNATURE_ALGO);
-		sig1 = Signature.getInstance(SIGNATURE_ALGO);
-		dig = MessageDigest.getInstance(DIGEST_ALGO);
-		dig1 = MessageDigest.getInstance(DIGEST_ALGO);
+		readerFactory = i.getInstance(ReaderFactory.class);
+		writerFactory = i.getInstance(WriterFactory.class);
+		signature = Signature.getInstance(SIGNATURE_ALGO);
+		messageDigest = MessageDigest.getInstance(DIGEST_ALGO);
+		batchDigest = MessageDigest.getInstance(DIGEST_ALGO);
 		final KeyFactory keyFactory = KeyFactory.getInstance(KEY_PAIR_ALGO);
 		keyParser = new KeyParser() {
 			public PublicKey parsePublicKey(byte[] encodedKey)
@@ -93,8 +89,13 @@ public class BundleReadWriteTest extends TestCase {
 				return keyFactory.generatePublic(e);
 			}
 		};
-		assertEquals(dig.getDigestLength(), UniqueId.LENGTH);
-		MessageEncoder messageEncoder = new MessageEncoderImpl(sig, dig, wf);
+		assertEquals(messageDigest.getDigestLength(), UniqueId.LENGTH);
+		assertEquals(batchDigest.getDigestLength(), UniqueId.LENGTH);
+		// Create and encode a test message
+		MessageEncoder messageEncoder = new MessageEncoderImpl(signature,
+				messageDigest, writerFactory);
+		KeyPair keyPair =
+			KeyPairGenerator.getInstance(KEY_PAIR_ALGO).generateKeyPair();
 		message = messageEncoder.encodeMessage(MessageId.NONE, sub, nick,
 				keyPair, messageBody.getBytes("UTF-8"));
 	}
@@ -107,8 +108,8 @@ public class BundleReadWriteTest extends TestCase {
 	@Test
 	public void testWriteBundle() throws Exception {
 		FileOutputStream out = new FileOutputStream(bundle);
-		BundleWriter w = new BundleWriterImpl(out, wf, keyPair.getPrivate(),
-				sig, dig, capacity);
+		BundleWriter w = new BundleWriterImpl(out, writerFactory, batchDigest,
+				capacity);
 		Raw messageRaw = new RawByteArray(message.getBytes());
 
 		w.addHeader(acks, subs, transports);
@@ -125,12 +126,12 @@ public class BundleReadWriteTest extends TestCase {
 		testWriteBundle();
 
 		FileInputStream in = new FileInputStream(bundle);
-		Reader reader = rf.createReader(in);
-		MessageReader messageReader = new MessageReader(keyParser, sig1, dig1);
-		HeaderReader headerReader = new HeaderReader(keyPair.getPublic(), sig,
-				new HeaderFactoryImpl());
-		BatchReader batchReader = new BatchReader(keyPair.getPublic(), sig, dig,
-				messageReader, new BatchFactoryImpl());
+		Reader reader = readerFactory.createReader(in);
+		MessageReader messageReader =
+			new MessageReader(keyParser, signature, messageDigest);
+		HeaderReader headerReader = new HeaderReader(new HeaderFactoryImpl());
+		BatchReader batchReader = new BatchReader(batchDigest, messageReader,
+				new BatchFactoryImpl());
 		BundleReader r = new BundleReaderImpl(reader, headerReader,
 				batchReader);
 
@@ -153,40 +154,6 @@ public class BundleReadWriteTest extends TestCase {
 		r.finish();
 	}
 
-
-	@Test
-	public void testModifyingBundleBreaksSignature() throws Exception {
-
-		testWriteBundle();
-
-		RandomAccessFile f = new RandomAccessFile(bundle, "rw");
-		f.seek(bundle.length() - 100);
-		byte b = f.readByte();
-		f.seek(bundle.length() - 100);
-		f.writeByte(b + 1);
-		f.close();
-
-		FileInputStream in = new FileInputStream(bundle);
-		Reader reader = rf.createReader(in);
-		MessageReader messageReader = new MessageReader(keyParser, sig1, dig1);
-		HeaderReader headerReader = new HeaderReader(keyPair.getPublic(), sig,
-				new HeaderFactoryImpl());
-		BatchReader batchReader = new BatchReader(keyPair.getPublic(), sig, dig,
-				messageReader, new BatchFactoryImpl());
-		BundleReader r = new BundleReaderImpl(reader, headerReader,
-				batchReader);
-
-		Header h = r.getHeader();
-		assertEquals(acks, h.getAcks());
-		assertEquals(subs, h.getSubscriptions());
-		assertEquals(transports, h.getTransports());
-		try {
-			r.getNextBatch();
-			assertTrue(false);
-		} catch(GeneralSecurityException expected) {}
-		r.finish();
-	}
-
 	@After
 	public void tearDown() {
 		TestUtils.deleteTestDirectory(testDir);

test/net/sf/briar/protocol/BundleReaderImplTest.java

@@ -1,5 +1,270 @@
 package net.sf.briar.protocol;
 
-public class BundleReaderImplTest {
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.security.GeneralSecurityException;
+import java.util.Collections;
+import java.util.Map;
+import java.util.Set;
 
+import junit.framework.TestCase;
+import net.sf.briar.api.protocol.Batch;
+import net.sf.briar.api.protocol.BatchId;
+import net.sf.briar.api.protocol.GroupId;
+import net.sf.briar.api.protocol.Header;
+import net.sf.briar.api.protocol.Message;
+import net.sf.briar.api.protocol.Tags;
+import net.sf.briar.api.serial.FormatException;
+import net.sf.briar.api.serial.ObjectReader;
+import net.sf.briar.api.serial.Reader;
+import net.sf.briar.api.serial.ReaderFactory;
+import net.sf.briar.api.serial.Writer;
+import net.sf.briar.api.serial.WriterFactory;
+import net.sf.briar.serial.SerialModule;
+
+import org.junit.Test;
+
+import com.google.inject.Guice;
+import com.google.inject.Injector;
+
+public class BundleReaderImplTest extends TestCase {
+
+	private final ReaderFactory readerFactory;
+	private final WriterFactory writerFactory;
+
+	public BundleReaderImplTest() {
+		Injector i = Guice.createInjector(new SerialModule());
+		readerFactory = i.getInstance(ReaderFactory.class);
+		writerFactory = i.getInstance(WriterFactory.class);
+	}
+
+	@Test
+	public void testEmptyBundleThrowsFormatException() throws Exception {
+		ByteArrayInputStream in = new ByteArrayInputStream(new byte[] {});
+		Reader r = readerFactory.createReader(in);
+		BundleReaderImpl b = new BundleReaderImpl(r, new TestHeaderReader(),
+				new TestBatchReader());
+
+		try {
+			b.getHeader();
+			assertTrue(false);
+		} catch(FormatException expected) {}
+	}
+
+	@Test
+	public void testReadingBatchBeforeHeaderThrowsIllegalStateException()
+	throws Exception {
+		ByteArrayInputStream in = new ByteArrayInputStream(createValidBundle());
+		Reader r = readerFactory.createReader(in);
+		BundleReaderImpl b = new BundleReaderImpl(r, new TestHeaderReader(),
+				new TestBatchReader());
+
+		try {
+			b.getNextBatch();
+			assertTrue(false);
+		} catch(IllegalStateException expected) {}
+	}
+
+	@Test
+	public void testMissingHeaderThrowsFormatException() throws Exception {
+		// Create a headless bundle
+		ByteArrayOutputStream out = new ByteArrayOutputStream();
+		Writer w = writerFactory.createWriter(out);
+		w.writeListStart();
+		w.writeUserDefinedTag(Tags.BATCH);
+		w.writeList(Collections.emptyList());
+		w.writeListEnd();
+		w.close();
+		byte[] headless = out.toByteArray();
+		// Try to read a header from the headless bundle
+		ByteArrayInputStream in = new ByteArrayInputStream(headless);
+		Reader r = readerFactory.createReader(in);
+		BundleReaderImpl b = new BundleReaderImpl(r, new TestHeaderReader(),
+				new TestBatchReader());
+
+		try {
+			b.getHeader();
+			assertTrue(false);
+		} catch(FormatException expected) {}
+	}
+
+	@Test
+	public void testMissingBatchListThrowsFormatException() throws Exception {
+		// Create a header-only bundle
+		ByteArrayOutputStream out = new ByteArrayOutputStream();
+		Writer w = writerFactory.createWriter(out);
+		w.writeUserDefinedTag(Tags.HEADER);
+		w.writeList(Collections.emptyList()); // Acks
+		w.writeList(Collections.emptyList()); // Subs
+		w.writeMap(Collections.emptyMap()); // Transports
+		w.writeInt64(System.currentTimeMillis()); // Timestamp
+		w.close();
+		byte[] headerOnly = out.toByteArray();
+		// Try to read a header from the header-only bundle
+		ByteArrayInputStream in = new ByteArrayInputStream(headerOnly);
+		final Reader r = readerFactory.createReader(in);
+		BundleReaderImpl b = new BundleReaderImpl(r, new TestHeaderReader(),
+				new TestBatchReader());
+
+		try {
+			b.getHeader();
+			assertTrue(false);
+		} catch(FormatException expected) {}
+	}
+
+	@Test
+	public void testEmptyBatchListIsAcceptable() throws Exception {
+		// Create a bundle with no batches
+		ByteArrayOutputStream out = new ByteArrayOutputStream();
+		Writer w = writerFactory.createWriter(out);
+		w.writeUserDefinedTag(Tags.HEADER);
+		w.writeList(Collections.emptyList()); // Acks
+		w.writeList(Collections.emptyList()); // Subs
+		w.writeMap(Collections.emptyMap()); // Transports
+		w.writeInt64(System.currentTimeMillis()); // Timestamp
+		w.writeListStart();
+		w.writeListEnd();
+		w.close();
+		byte[] batchless = out.toByteArray();
+		// It should be possible to read the header and null
+		ByteArrayInputStream in = new ByteArrayInputStream(batchless);
+		final Reader r = readerFactory.createReader(in);
+		BundleReaderImpl b = new BundleReaderImpl(r, new TestHeaderReader(),
+				new TestBatchReader());
+
+		assertNotNull(b.getHeader());
+		assertNull(b.getNextBatch());
+	}
+
+	@Test
+	public void testValidBundle() throws Exception {
+		// It should be possible to read the header, a batch, and null
+		ByteArrayInputStream in = new ByteArrayInputStream(createValidBundle());
+		final Reader r = readerFactory.createReader(in);
+		BundleReaderImpl b = new BundleReaderImpl(r, new TestHeaderReader(),
+				new TestBatchReader());
+
+		assertNotNull(b.getHeader());
+		assertNotNull(b.getNextBatch());
+		assertNull(b.getNextBatch());
+	}
+
+	@Test
+	public void testReadingBatchAfterNullThrowsIllegalStateException()
+	throws Exception {
+		// Trying to read another batch after null should not succeed
+		ByteArrayInputStream in = new ByteArrayInputStream(createValidBundle());
+		final Reader r = readerFactory.createReader(in);
+		BundleReaderImpl b = new BundleReaderImpl(r, new TestHeaderReader(),
+				new TestBatchReader());
+
+		assertNotNull(b.getHeader());
+		assertNotNull(b.getNextBatch());
+		assertNull(b.getNextBatch());
+		try {
+			b.getNextBatch();
+			assertTrue(false);
+		} catch(IllegalStateException expected) {}
+	}
+
+	@Test
+	public void testReadingHeaderTwiceThrowsIllegalStateException()
+	throws Exception {
+		// Trying to read the header twice should not succeed
+		ByteArrayInputStream in = new ByteArrayInputStream(createValidBundle());
+		final Reader r = readerFactory.createReader(in);
+		BundleReaderImpl b = new BundleReaderImpl(r, new TestHeaderReader(),
+				new TestBatchReader());
+
+		assertNotNull(b.getHeader());
+		try {
+			b.getHeader();
+			assertTrue(false);
+		} catch(IllegalStateException expected) {}
+	}
+
+	@Test
+	public void testReadingHeaderAfterBatchThrowsIllegalStateException()
+	throws Exception {
+		// Trying to read the header after a batch should not succeed
+		ByteArrayInputStream in = new ByteArrayInputStream(createValidBundle());
+		final Reader r = readerFactory.createReader(in);
+		BundleReaderImpl b = new BundleReaderImpl(r, new TestHeaderReader(),
+				new TestBatchReader());
+
+		assertNotNull(b.getHeader());
+		assertNotNull(b.getNextBatch());
+		try {
+			b.getHeader();
+			assertTrue(false);
+		} catch(IllegalStateException expected) {}
+	}
+
+	private byte[] createValidBundle() throws IOException {
+		ByteArrayOutputStream out = new ByteArrayOutputStream();
+		Writer w = writerFactory.createWriter(out);
+		w.writeUserDefinedTag(Tags.HEADER);
+		w.writeList(Collections.emptyList()); // Acks
+		w.writeList(Collections.emptyList()); // Subs
+		w.writeMap(Collections.emptyMap()); // Transports
+		w.writeInt64(System.currentTimeMillis()); // Timestamp
+		w.writeListStart();
+		w.writeUserDefinedTag(Tags.BATCH);
+		w.writeList(Collections.emptyList()); // Messages
+		w.writeListEnd();
+		w.close();
+		return out.toByteArray();
+	}
+
+	private static class TestHeaderReader implements ObjectReader<Header> {
+
+		public Header readObject(Reader r) throws IOException,
+		GeneralSecurityException {
+			r.readList();
+			r.readList();
+			r.readMap();
+			r.readInt64();
+			return new TestHeader();
+		}
+	}
+
+	private static class TestHeader implements Header {
+
+		public Set<BatchId> getAcks() {
+			return null;
+		}
+
+		public Set<GroupId> getSubscriptions() {
+			return null;
+		}
+
+		public Map<String, String> getTransports() {
+			return null;
+		}
+
+		public long getTimestamp() {
+			return 0;
+		}
+	}
+
+	private static class TestBatchReader implements ObjectReader<Batch> {
+
+		public Batch readObject(Reader r) throws IOException,
+		GeneralSecurityException {
+			r.readList();
+			return new TestBatch();
+		}
+	}
+
+	private static class TestBatch implements Batch {
+
+		public BatchId getId() {
+			return null;
+		}
+
+		public Iterable<Message> getMessages() {
+			return null;
+		}
+	}
 }