Frame reordering window.

api/net/sf/briar/api/transport/TransportConstants.java

@@ -26,4 +26,7 @@ public interface TransportConstants {
 
 	/** The size of the connection reordering window. */
 	static final int CONNECTION_WINDOW_SIZE = 32;
+
+	/** The size of the frame reordering window. */
+	static final int FRAME_WINDOW_SIZE = 32;
 }

components/net/sf/briar/transport/ConnectionWindowImpl.java

@@ -83,12 +83,12 @@ class ConnectionWindowImpl implements ConnectionWindow {
 	}
 
 	// Returns the lowest value contained in a window with the given centre
-	private long getBottom(long centre) {
+	private static long getBottom(long centre) {
 		return Math.max(0, centre - CONNECTION_WINDOW_SIZE / 2);
 	}
 
 	// Returns the highest value contained in a window with the given centre
-	private long getTop(long centre) {
+	private static long getTop(long centre) {
 		return Math.min(ByteUtils.MAX_32_BIT_UNSIGNED,
 				centre + CONNECTION_WINDOW_SIZE / 2 - 1);
 	}

components/net/sf/briar/transport/FrameWindow.java

@@ -0,0 +1,22 @@
+package net.sf.briar.transport;
+
+interface FrameWindow {
+
+	/** Returns true if the given number is in the window. */
+	boolean contains(long frameNumber);
+
+	/**
+	 * Advances the window so it is centred on the given number, unless the
+	 * centre is already greater than the number. Returns false if the window
+	 * is unmodifiable.
+	 */
+	boolean advance(long frameNumber);
+
+	/**
+	 * Removes the given number from the window and, if the number is greater
+	 * than or equal to the window's centre, advances the centre to one greater
+	 * than the given number. Returns false if the given number is not in the
+	 * window or the window is unmodifiable.
+	 */
+	boolean remove(long frameNumber);
+}

components/net/sf/briar/transport/FrameWindowImpl.java

@@ -0,0 +1,63 @@
+package net.sf.briar.transport;
+
+import static net.sf.briar.api.transport.TransportConstants.FRAME_WINDOW_SIZE;
+import static net.sf.briar.util.ByteUtils.MAX_32_BIT_UNSIGNED;
+
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Iterator;
+
+class FrameWindowImpl implements FrameWindow {
+
+	private final Collection<Long> window;
+
+	private long centre;
+
+	FrameWindowImpl() {
+		window = new HashSet<Long>();
+		for(long l = 0; l < FRAME_WINDOW_SIZE / 2; l++) window.add(l);
+		centre = 0;
+	}
+
+	public boolean contains(long frameNumber) {
+		if(frameNumber < 0 || frameNumber > MAX_32_BIT_UNSIGNED)
+			throw new IllegalArgumentException();
+		return window.contains(frameNumber);
+	}
+
+	public boolean advance(long frameNumber) {
+		if(frameNumber < 0 || frameNumber > MAX_32_BIT_UNSIGNED)
+			throw new IllegalArgumentException();
+		if(frameNumber <= centre) return true;
+		// Remove values that have passed out of the window
+		long newBottom = getBottom(frameNumber);
+		Iterator<Long> it = window.iterator();
+		while(it.hasNext()) if(it.next() < newBottom) it.remove();
+		// Add values that have passed into the window
+		long fillFrom = Math.max(newBottom, getTop(centre) + 1);
+		long newTop = getTop(frameNumber);
+		for(long l = fillFrom; l <= newTop; l++) window.add(l);
+		centre = frameNumber;
+		return true;
+	}
+
+	public boolean remove(long frameNumber) {
+		if(frameNumber < 0 || frameNumber > MAX_32_BIT_UNSIGNED)
+			throw new IllegalArgumentException();
+		if(!window.remove(frameNumber)) return false;
+		if(frameNumber >= centre && frameNumber < MAX_32_BIT_UNSIGNED)
+			advance(frameNumber + 1);
+		return true;
+	}
+
+	// Returns the lowest value contained in a window with the given centre
+	private static long getBottom(long centre) {
+		return Math.max(0, centre - FRAME_WINDOW_SIZE / 2);
+	}
+
+	// Returns the highest value contained in a window with the given centre
+	private static long getTop(long centre) {
+		return Math.min(MAX_32_BIT_UNSIGNED,
+				centre + FRAME_WINDOW_SIZE / 2 - 1);
+	}
+}

components/net/sf/briar/transport/HeaderEncoder.java

@@ -22,9 +22,8 @@ class HeaderEncoder {
 		ByteUtils.writeUint16(padding, header, 6);
 	}
 
-	static boolean validateHeader(byte[] header, long frameNumber) {
+	static boolean validateHeader(byte[] header) {
 		if(header.length < FRAME_HEADER_LENGTH) return false;
-		if(ByteUtils.readUint32(header, 0) != frameNumber) return false;
 		int payload = ByteUtils.readUint16(header, 4);
 		int padding = ByteUtils.readUint16(header, 6);
 		int frameLength = FRAME_HEADER_LENGTH + payload + padding + MAC_LENGTH;

components/net/sf/briar/transport/IncomingAuthenticationLayer.java

@@ -1,7 +1,6 @@
 package net.sf.briar.transport;
 
 import java.io.IOException;
-import java.util.Collection;
 
 interface IncomingAuthenticationLayer {
 
@@ -14,6 +13,6 @@ interface IncomingAuthenticationLayer {
 	 * @throws InvalidDataException if a recoverable error occurs. The caller
 	 * may choose whether to retry the read or close the connection.
 	 */
-	boolean readFrame(Frame f, Collection<Long> window) throws IOException,
+	boolean readFrame(Frame f, FrameWindow window) throws IOException,
 	InvalidDataException;
 }

components/net/sf/briar/transport/IncomingAuthenticationLayerImpl.java

@@ -5,7 +5,6 @@ import static net.sf.briar.api.transport.TransportConstants.MAC_LENGTH;
 
 import java.io.IOException;
 import java.security.InvalidKeyException;
-import java.util.Collection;
 
 import javax.crypto.Mac;
 
@@ -31,15 +30,13 @@ class IncomingAuthenticationLayerImpl implements IncomingAuthenticationLayer {
 			throw new IllegalArgumentException();
 	}
 
-	public boolean readFrame(Frame f, Collection<Long> window)
+	public boolean readFrame(Frame f, FrameWindow window) throws IOException,
-	throws IOException, InvalidDataException {
+	InvalidDataException {
 		// Read a frame
 		if(!in.readFrame(f, window)) return false;
 		// Check that the length is legal
 		byte[] buf = f.getBuffer();
-		long frameNumber = HeaderEncoder.getFrameNumber(buf);
+		if(!HeaderEncoder.validateHeader(buf)) throw new InvalidDataException();
-		if(!HeaderEncoder.validateHeader(buf, frameNumber))
-			throw new InvalidDataException();
 		// Check that the payload and padding lengths are correct
 		int payload = HeaderEncoder.getPayloadLength(buf);
 		int padding = HeaderEncoder.getPaddingLength(buf);
@@ -58,7 +55,6 @@ class IncomingAuthenticationLayerImpl implements IncomingAuthenticationLayer {
 			if(expectedMac[i] != buf[macStart + i])
 				throw new InvalidDataException();
 		}
-		frameNumber++;
 		return true;
 	}
 }

components/net/sf/briar/transport/IncomingErrorCorrectionLayer.java

@@ -1,7 +1,6 @@
 package net.sf.briar.transport;
 
 import java.io.IOException;
-import java.util.Collection;
 
 interface IncomingErrorCorrectionLayer {
 
@@ -14,6 +13,6 @@ interface IncomingErrorCorrectionLayer {
 	 * @throws InvalidDataException if a recoverable error occurs. The caller
 	 * may choose whether to retry the read or close the connection.
 	 */
-	boolean readFrame(Frame f, Collection<Long> window) throws IOException,
+	boolean readFrame(Frame f, FrameWindow window) throws IOException,
 	InvalidDataException;
 }

components/net/sf/briar/transport/NullFrameWindow.java

@@ -0,0 +1,20 @@
+package net.sf.briar.transport;
+
+class NullFrameWindow implements FrameWindow {
+
+	private long centre = 0L;
+
+	public boolean contains(long frameNumber) {
+		return frameNumber == centre;
+	}
+
+	public boolean advance(long frameNumber) {
+		return frameNumber == centre;
+	}
+
+	public boolean remove(long frameNumber) {
+		if(frameNumber != centre) return false;
+		centre++;
+		return true;
+	}
+}

components/net/sf/briar/transport/NullIncomingErrorCorrectionLayer.java

@@ -1,7 +1,6 @@
 package net.sf.briar.transport;
 
 import java.io.IOException;
-import java.util.Collection;
 
 import net.sf.briar.api.transport.Segment;
 
@@ -15,7 +14,7 @@ class NullIncomingErrorCorrectionLayer implements IncomingErrorCorrectionLayer {
 		segment = new SegmentImpl();
 	}
 
-	public boolean readFrame(Frame f, Collection<Long> window)
+	public boolean readFrame(Frame f, FrameWindow window)
 	throws IOException, InvalidDataException {
 		while(true) {
 			if(!in.readSegment(segment)) return false;

components/net/sf/briar/transport/NullIncomingReliabilityLayer.java

@@ -1,24 +1,22 @@
 package net.sf.briar.transport;
 
 import java.io.IOException;
-import java.util.Collection;
-import java.util.Collections;
 
 class NullIncomingReliabilityLayer implements IncomingReliabilityLayer {
 
 	private final IncomingAuthenticationLayer in;
-
+	private final FrameWindow window;
-	private long frameNumber = 0L;
 
 	NullIncomingReliabilityLayer(IncomingAuthenticationLayer in) {
 		this.in = in;
+		window = new NullFrameWindow();
 	}
 
 	public boolean readFrame(Frame f) throws IOException, InvalidDataException {
-		// Frames must be read in order
-		Collection<Long> window = Collections.singleton(frameNumber);
 		if(!in.readFrame(f, window)) return false;
-		frameNumber++;
+		long frameNumber = HeaderEncoder.getFrameNumber(f.getBuffer());
+		if(!window.remove(frameNumber) && window.advance(frameNumber))
+				throw new RuntimeException();
 		return true;
 	}
 }

test/build.xml

@@ -56,6 +56,7 @@
 			<test name='net.sf.briar.transport.ConnectionWriterImplTest'/>
 			<test name='net.sf.briar.transport.ConnectionWriterTest'/>
 			<test name='net.sf.briar.transport.FrameReadWriteTest'/>
+			<test name='net.sf.briar.transport.FrameWindowImplTest'/>
 			<test name='net.sf.briar.transport.IncomingEncryptionLayerImplTest'/>
 			<test name='net.sf.briar.transport.IncomingSegmentedEncryptionLayerTest'/>
 			<test name='net.sf.briar.transport.OutgoingEncryptionLayerImplTest'/>

test/net/sf/briar/transport/FrameWindowImplTest.java

@@ -0,0 +1,89 @@
+package net.sf.briar.transport;
+
+import net.sf.briar.BriarTestCase;
+import static net.sf.briar.api.transport.TransportConstants.FRAME_WINDOW_SIZE;
+import static net.sf.briar.util.ByteUtils.MAX_32_BIT_UNSIGNED;
+
+import org.junit.Test;
+
+public class FrameWindowImplTest extends BriarTestCase {
+
+	@Test
+	public void testWindowSliding() {
+		FrameWindow w = new FrameWindowImpl();
+		for(int i = 0; i < 100; i++) {
+			assertTrue(w.contains(i));
+			w.remove(i);
+			assertFalse(w.contains(i));
+		}
+	}
+
+	@Test
+	public void testWindowJumping() {
+		FrameWindow w = new FrameWindowImpl();
+		for(int i = 0; i < 100; i += 13) {
+			assertTrue(w.contains(i));
+			w.remove(i);
+			assertFalse(w.contains(i));
+		}
+	}
+
+	@Test
+	public void testWindowUpperLimit() {
+		FrameWindow w = new FrameWindowImpl();
+		// Centre is 0, highest value in window is 15
+		for(int i = 0; i < 16; i++) assertTrue(w.contains(i));
+		assertFalse(w.remove(16));
+		assertTrue(w.remove(15));
+		assertFalse(w.remove(15));
+		// Centre is 16, highest value in window is 31
+		for(int i = 0; i < 32; i++) assertEquals(i != 15, w.contains(i));
+		assertFalse(w.remove(32));
+		assertTrue(w.remove(31));
+		// Values greater than 2^32 - 1 should never be allowed
+		assertTrue(w.advance(MAX_32_BIT_UNSIGNED - 1));
+		assertTrue(w.remove(MAX_32_BIT_UNSIGNED - 1));
+		assertTrue(w.remove(MAX_32_BIT_UNSIGNED));
+		try {
+			w.remove(MAX_32_BIT_UNSIGNED + 1);
+			fail();
+		} catch(IllegalArgumentException expected) {}
+	}
+
+	@Test
+	public void testAdvance() {
+		FrameWindow w = new FrameWindowImpl();
+		long centre = 0;
+		for(int i = 0; i < 10; i++) {
+			long bottom = Math.max(0, centre - FRAME_WINDOW_SIZE / 2);
+			long top = Math.min(MAX_32_BIT_UNSIGNED,
+					centre + FRAME_WINDOW_SIZE / 2 - 1);
+			if(bottom > 0) assertFalse(w.contains(bottom - 1));
+			for(long j = bottom; j <= top; j++) assertTrue(w.contains(j));
+			if(top < MAX_32_BIT_UNSIGNED) assertFalse(w.contains(top + 1));
+			centre += 12345;
+			assertTrue(w.advance(centre));
+		}
+	}
+
+	@Test
+	public void testWindowLowerLimit() {
+		FrameWindow w = new FrameWindowImpl();
+		// Centre is 0, negative values should never be allowed
+		try {
+			w.remove(-1);
+			fail();
+		} catch(IllegalArgumentException expected) {}
+		// Slide the window twice
+		assertTrue(w.remove(15));
+		assertTrue(w.remove(16));
+		// Centre is 17, lowest value in window is 1
+		assertFalse(w.remove(0));
+		assertTrue(w.remove(1));
+		// Slide the window
+		assertTrue(w.remove(25));
+		// Centre is 26, lowest value in window is 10
+		assertFalse(w.remove(9));
+		assertTrue(w.remove(10));
+	}
+}