diff --git a/components/net/sf/briar/transport/FrameWindow.java b/components/net/sf/briar/transport/FrameWindow.java index dcb1d75a9e81b78df78113940e8ab8c61355b2aa..85e162b1f1cc90c909dfe91ead44e7b933606953 100644 --- a/components/net/sf/briar/transport/FrameWindow.java +++ b/components/net/sf/briar/transport/FrameWindow.java @@ -2,21 +2,15 @@ package net.sf.briar.transport; interface FrameWindow { + /** Returns true if the given number is too high to fit in the window. */ + boolean isTooHigh(long frameNumber); + /** 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. + * Removes the given number from the window and advances the window. + * Returns false if the given number is not in the window. */ boolean remove(long frameNumber); } diff --git a/components/net/sf/briar/transport/FrameWindowImpl.java b/components/net/sf/briar/transport/FrameWindowImpl.java index 3348ce73f4fe18a16d86fb9ab3922638b1f16fb8..ddf3e71cf65793dca4c97199dba7bcbfc09afd9f 100644 --- a/components/net/sf/briar/transport/FrameWindowImpl.java +++ b/components/net/sf/briar/transport/FrameWindowImpl.java @@ -5,59 +5,57 @@ 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; + private long base; FrameWindowImpl() { window = new HashSet<Long>(); - for(long l = 0; l < FRAME_WINDOW_SIZE / 2; l++) window.add(l); - centre = 0; + fill(0, FRAME_WINDOW_SIZE); + base = 0; } - public boolean contains(long frameNumber) { + public boolean isTooHigh(long frameNumber) { if(frameNumber < 0 || frameNumber > MAX_32_BIT_UNSIGNED) throw new IllegalArgumentException(); - return window.contains(frameNumber); + return frameNumber >= base + FRAME_WINDOW_SIZE; } - public boolean advance(long frameNumber) { + public boolean contains(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; + return window.contains(frameNumber); } 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); + if(frameNumber == base) { + // Find the new base + if(window.isEmpty()) { + base += FRAME_WINDOW_SIZE; + fill(base, base + FRAME_WINDOW_SIZE); + } else { + for(long l = base; l < base + FRAME_WINDOW_SIZE; l++) { + if(window.contains(l)) { + fill(base + FRAME_WINDOW_SIZE, l + FRAME_WINDOW_SIZE); + base = l; + break; + } + } + } + } 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); + private void fill(long from, long to) { + for(long l = from; l < to; l++) { + if(l <= MAX_32_BIT_UNSIGNED) window.add(l); + else return; + } } } diff --git a/components/net/sf/briar/transport/NullFrameWindow.java b/components/net/sf/briar/transport/NullFrameWindow.java index c8560de86161b2ecca76a325d2b00255d6bff3c0..c56a78a37dea818d2c0f01109d39d52c378f4b3f 100644 --- a/components/net/sf/briar/transport/NullFrameWindow.java +++ b/components/net/sf/briar/transport/NullFrameWindow.java @@ -1,20 +1,28 @@ package net.sf.briar.transport; +import static net.sf.briar.util.ByteUtils.MAX_32_BIT_UNSIGNED; + class NullFrameWindow implements FrameWindow { - private long centre = 0L; + private long base = 0L; - public boolean contains(long frameNumber) { - return frameNumber == centre; + public boolean isTooHigh(long frameNumber) { + if(frameNumber < 0 || frameNumber > MAX_32_BIT_UNSIGNED) + throw new IllegalArgumentException(); + return frameNumber != base; } - public boolean advance(long frameNumber) { - return frameNumber == centre; + public boolean contains(long frameNumber) { + if(frameNumber < 0 || frameNumber > MAX_32_BIT_UNSIGNED) + throw new IllegalArgumentException(); + return frameNumber == base; } public boolean remove(long frameNumber) { - if(frameNumber != centre) return false; - centre++; + if(frameNumber < 0 || frameNumber > MAX_32_BIT_UNSIGNED) + throw new IllegalArgumentException(); + if(frameNumber != base) return false; + base++; return true; } } diff --git a/components/net/sf/briar/transport/NullIncomingReliabilityLayer.java b/components/net/sf/briar/transport/NullIncomingReliabilityLayer.java index 3ecd84502c6bd9081f2767c1bfa7a774144903b5..25ddb59d24846c50ac99adc6c5d7b93b07c5dfcc 100644 --- a/components/net/sf/briar/transport/NullIncomingReliabilityLayer.java +++ b/components/net/sf/briar/transport/NullIncomingReliabilityLayer.java @@ -15,8 +15,7 @@ class NullIncomingReliabilityLayer implements IncomingReliabilityLayer { public boolean readFrame(Frame f) throws IOException, InvalidDataException { if(!in.readFrame(f, window)) return false; long frameNumber = HeaderEncoder.getFrameNumber(f.getBuffer()); - if(!window.remove(frameNumber) && window.advance(frameNumber)) - throw new RuntimeException(); + if(!window.remove(frameNumber)) throw new IllegalStateException(); return true; } } diff --git a/test/net/sf/briar/transport/FrameWindowImplTest.java b/test/net/sf/briar/transport/FrameWindowImplTest.java index 1cc08245701af5501d48d0df6d78d4db22cb7df9..fc8913befca114f27139d541c02111513e5a8eb5 100644 --- a/test/net/sf/briar/transport/FrameWindowImplTest.java +++ b/test/net/sf/briar/transport/FrameWindowImplTest.java @@ -1,8 +1,7 @@ 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 net.sf.briar.BriarTestCase; import org.junit.Test; @@ -13,77 +12,71 @@ public class FrameWindowImplTest extends BriarTestCase { FrameWindow w = new FrameWindowImpl(); for(int i = 0; i < 100; i++) { assertTrue(w.contains(i)); - w.remove(i); + assertTrue(w.remove(i)); assertFalse(w.contains(i)); } + for(int i = 100; i < 100 + FRAME_WINDOW_SIZE; i++) { + assertTrue(w.contains(i)); + assertFalse(w.isTooHigh(i)); + } + assertFalse(w.contains(100 + FRAME_WINDOW_SIZE)); + assertTrue(w.isTooHigh(100 + FRAME_WINDOW_SIZE)); } @Test public void testWindowJumping() { FrameWindow w = new FrameWindowImpl(); - for(int i = 0; i < 100; i += 13) { + // Base of the window is 0 + for(int i = 0; i < FRAME_WINDOW_SIZE; i++) assertTrue(w.contains(i)); + assertFalse(w.contains(FRAME_WINDOW_SIZE)); + assertFalse(w.isTooHigh(FRAME_WINDOW_SIZE - 1)); + assertTrue(w.isTooHigh(FRAME_WINDOW_SIZE)); + // Remove all numbers except 0 and 5 + for(int i = 1; i < 5; i++) assertTrue(w.remove(i)); + for(int i = 6; i < FRAME_WINDOW_SIZE; i++) assertTrue(w.remove(i)); + // Base of the window should still be 0 + assertTrue(w.contains(0)); + for(int i = 1; i < 5; i++) assertFalse(w.contains(i)); + assertTrue(w.contains(5)); + for(int i = 6; i < FRAME_WINDOW_SIZE; i++) assertFalse(w.contains(i)); + assertFalse(w.contains(FRAME_WINDOW_SIZE)); + assertFalse(w.isTooHigh(FRAME_WINDOW_SIZE - 1)); + assertTrue(w.isTooHigh(FRAME_WINDOW_SIZE)); + // Remove 0 + assertTrue(w.remove(0)); + // Base of the window should now be 5 + for(int i = 0; i < 5; i++) assertFalse(w.contains(i)); + assertTrue(w.contains(5)); + for(int i = 6; i < FRAME_WINDOW_SIZE; i++) assertFalse(w.contains(i)); + for(int i = FRAME_WINDOW_SIZE; i < FRAME_WINDOW_SIZE + 5; i++) { assertTrue(w.contains(i)); - w.remove(i); + } + assertFalse(w.contains(FRAME_WINDOW_SIZE + 5)); + assertFalse(w.isTooHigh(FRAME_WINDOW_SIZE + 4)); + assertTrue(w.isTooHigh(FRAME_WINDOW_SIZE + 5)); + // Remove all numbers except 5 + for(int i = FRAME_WINDOW_SIZE; i < FRAME_WINDOW_SIZE + 5; i++) { + assertTrue(w.remove(i)); + } + // Base of the window should still be 5 + assertTrue(w.contains(5)); + for(int i = 6; i < FRAME_WINDOW_SIZE + 5; 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)); + assertFalse(w.contains(FRAME_WINDOW_SIZE + 5)); + assertFalse(w.isTooHigh(FRAME_WINDOW_SIZE + 4)); + assertTrue(w.isTooHigh(FRAME_WINDOW_SIZE + 5)); + // Remove 5 + assertTrue(w.remove(5)); + // Base of the window should now be FRAME_WINDOW_SIZE + 5 + for(int i = 0; i < FRAME_WINDOW_SIZE + 5; i++) { + assertFalse(w.contains(i)); } - } - - @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)); + for(int i = FRAME_WINDOW_SIZE + 5; i < FRAME_WINDOW_SIZE * 2 + 5; i++) { + assertTrue(w.contains(i)); + } + assertFalse(w.contains(FRAME_WINDOW_SIZE * 2 + 5)); + assertFalse(w.isTooHigh(FRAME_WINDOW_SIZE * 2 + 4)); + assertTrue(w.isTooHigh(FRAME_WINDOW_SIZE * 2 + 5)); } } \ No newline at end of file