From a49238ceba125c890ba22345af24978d9655b923 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Tue, 24 Jul 2012 13:21:35 -0700 Subject: [PATCH] Removing FragmentExtension's minFragments config. + The minFragments configuration appears to work on the message, not the fragments, so its was removed as inappropriate for the level of the protocol the FragmentExtension works on. --- .../fragment/FragmentExtension.java | 68 ++++----------- .../extensions/FragmentExtensionTest.java | 87 +++++++++++++++++-- 2 files changed, 100 insertions(+), 55 deletions(-) diff --git a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/extensions/fragment/FragmentExtension.java b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/extensions/fragment/FragmentExtension.java index 1175d3bdbe8..d8a18a613cc 100644 --- a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/extensions/fragment/FragmentExtension.java +++ b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/extensions/fragment/FragmentExtension.java @@ -26,8 +26,6 @@ import org.eclipse.jetty.websocket.protocol.WebSocketFrame; public class FragmentExtension extends Extension { private int maxLength = -1; - private int minFragments = 1; - @Override public void output(C context, Callback callback, WebSocketFrame frame) @@ -39,7 +37,6 @@ public class FragmentExtension extends Extension return; } - int fragments = 1; int length = frame.getPayloadLength(); OpCode opcode = frame.getOpCode(); // original opcode @@ -47,63 +44,35 @@ public class FragmentExtension extends Extension int originalLimit = payload.limit(); int currentPosition = payload.position(); - // break apart payload based on maxLength rules - if (maxLength > 0) + if (maxLength <= 0) { - while (length > maxLength) - { - fragments++; - - WebSocketFrame frag = new WebSocketFrame(frame); - frag.setOpCode(opcode); - frag.setFin(false); // always false here - payload.position(currentPosition); - payload.limit(Math.min(payload.position() + maxLength,originalLimit)); - frag.setPayload(payload); - - nextOutputNoCallback(frag); - - length -= maxLength; - opcode = OpCode.CONTINUATION; - currentPosition = payload.limit(); - } - - // write remaining - WebSocketFrame frag = new WebSocketFrame(frame); - frag.setOpCode(opcode); - frag.setFin(frame.isFin()); // use original fin - payload.position(currentPosition); - payload.limit(originalLimit); - frag.setPayload(payload); - - nextOutput(context,callback,frag); + // output original frame + nextOutput(context,callback,frame); return; } - // break apart payload based on minimum # of fragments - if (fragments < minFragments) + // break apart payload based on maxLength rules + while (length > maxLength) { - int fragmentsLeft = (minFragments - fragments); - int fragLength = length / fragmentsLeft; // equal sized fragments + WebSocketFrame frag = new WebSocketFrame(frame); + frag.setOpCode(opcode); + frag.setFin(false); // always false here + payload.position(currentPosition); + payload.limit(Math.min(payload.position() + maxLength,originalLimit)); + frag.setPayload(payload); - while (fragments < minFragments) - { - fragments++; + nextOutputNoCallback(frag); - WebSocketFrame frag = new WebSocketFrame(frame); - frag.setOpCode(opcode); - frag.setFin(false); - frag.setPayload(payload); - - nextOutputNoCallback(frag); - length -= fragLength; - opcode = OpCode.CONTINUATION; - } + length -= maxLength; + opcode = OpCode.CONTINUATION; + currentPosition = payload.limit(); } - // output whatever is left + // write remaining WebSocketFrame frag = new WebSocketFrame(frame); frag.setOpCode(opcode); + frag.setFin(frame.isFin()); // use original fin + payload.position(currentPosition); payload.limit(originalLimit); frag.setPayload(payload); @@ -116,6 +85,5 @@ public class FragmentExtension extends Extension super.setConfig(config); maxLength = config.getParameter("maxLength",maxLength); - minFragments = config.getParameter("minFragments",minFragments); } } diff --git a/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/extensions/FragmentExtensionTest.java b/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/extensions/FragmentExtensionTest.java index 5dcdce61287..e44fbad5a05 100644 --- a/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/extensions/FragmentExtensionTest.java +++ b/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/extensions/FragmentExtensionTest.java @@ -76,7 +76,7 @@ public class FragmentExtensionTest FragmentExtension ext = new FragmentExtension(); ext.setBufferPool(new StandardByteBufferPool()); ext.setPolicy(WebSocketPolicy.newServerPolicy()); - ExtensionConfig config = ExtensionConfig.parse("fragment;maxLength=4;minFragments=7"); + ExtensionConfig config = ExtensionConfig.parse("fragment;maxLength=4"); ext.setConfig(config); ext.setNextIncomingFrames(capture); @@ -127,7 +127,7 @@ public class FragmentExtensionTest FragmentExtension ext = new FragmentExtension(); ext.setBufferPool(new StandardByteBufferPool()); ext.setPolicy(WebSocketPolicy.newServerPolicy()); - ExtensionConfig config = ExtensionConfig.parse("fragment;maxLength=4;minFragments=7"); + ExtensionConfig config = ExtensionConfig.parse("fragment;maxLength=4"); ext.setConfig(config); ext.setNextIncomingFrames(capture); @@ -152,10 +152,10 @@ public class FragmentExtensionTest } /** - * Verify that outgoing text frames are compressed. + * Verify that outgoing text frames are fragmented by the maxLength configuration. */ @Test - public void testOutgoingFrames() + public void testOutgoingFramesByMaxLength() { OutgoingFramesCapture capture = new OutgoingFramesCapture(); @@ -233,6 +233,83 @@ public class FragmentExtensionTest } } + /** + * Verify that outgoing text frames are fragmented by default configuration + */ + @Test + public void testOutgoingFramesDefaultConfig() + { + OutgoingFramesCapture capture = new OutgoingFramesCapture(); + + FragmentExtension ext = new FragmentExtension(); + ext.setBufferPool(new StandardByteBufferPool()); + ext.setPolicy(WebSocketPolicy.newServerPolicy()); + ExtensionConfig config = ExtensionConfig.parse("fragment"); + ext.setConfig(config); + + ext.setNextOutgoingFrames(capture); + + // Quote + List quote = new ArrayList<>(); + quote.add("No amount of experimentation can ever prove me right;"); + quote.add("a single experiment can prove me wrong."); + quote.add("-- Albert Einstein"); + + // Write quote as separate frames + List> callbacks = new ArrayList<>(); + for (String section : quote) + { + WebSocketFrame frame = WebSocketFrame.text(section); + FutureCallback callback = new FutureCallback<>(); + ext.output("Q" + (callbacks.size()),callback,frame); + callbacks.add(callback); + } + + // Expected Frames + ExpectedWrites expectedWrites = new ExpectedWrites(); + expectedWrites.add("Q0",callbacks.get(0)).frame = new WebSocketFrame(OpCode.TEXT).setPayload("No amount of experimentation can ever prove me right;"); + expectedWrites.add("Q1",callbacks.get(1)).frame = new WebSocketFrame(OpCode.TEXT).setPayload("a single experiment can prove me wrong."); + expectedWrites.add("Q2",callbacks.get(2)).frame = new WebSocketFrame(OpCode.TEXT).setPayload("-- Albert Einstein"); + + // capture.dump(); + + int len = expectedWrites.size(); + capture.assertFrameCount(len); + + String prefix; + LinkedList> writes = capture.getWrites(); + for (int i = 0; i < len; i++) + { + prefix = "Write[" + i + "]"; + Write actualWrite = writes.get(i); + Write expectedWrite = expectedWrites.get(i); + + if (expectedWrite.context != null) + { + // Validate callbacks have original settings + Assert.assertThat(prefix + ".context",(String)actualWrite.context,is(expectedWrite.context)); + Assert.assertSame(prefix + ".callback",expectedWrite.callback,actualWrite.callback); + } + + // Validate Frame + WebSocketFrame actualFrame = actualWrite.frame; + WebSocketFrame expectedFrame = expectedWrite.frame; + prefix += ".frame"; + Assert.assertThat(prefix + ".opcode",actualFrame.getOpCode(),is(expectedFrame.getOpCode())); + Assert.assertThat(prefix + ".fin",actualFrame.isFin(),is(expectedFrame.isFin())); + Assert.assertThat(prefix + ".rsv1",actualFrame.isRsv1(),is(expectedFrame.isRsv1())); + Assert.assertThat(prefix + ".rsv2",actualFrame.isRsv2(),is(expectedFrame.isRsv2())); + Assert.assertThat(prefix + ".rsv3",actualFrame.isRsv3(),is(expectedFrame.isRsv3())); + + // Validate Payload + ByteBuffer expectedData = expectedFrame.getPayload().slice(); + ByteBuffer actualData = actualFrame.getPayload().slice(); + + Assert.assertThat(prefix + ".payloadLength",actualData.remaining(),is(expectedData.remaining())); + ByteBufferAssert.assertEquals(prefix + ".payload",expectedData,actualData); + } + } + /** * Outgoing PING (Control Frame) should pass through extension unmodified */ @@ -244,7 +321,7 @@ public class FragmentExtensionTest FragmentExtension ext = new FragmentExtension(); ext.setBufferPool(new StandardByteBufferPool()); ext.setPolicy(WebSocketPolicy.newServerPolicy()); - ExtensionConfig config = ExtensionConfig.parse("fragment;maxLength=4;minFragments=7"); + ExtensionConfig config = ExtensionConfig.parse("fragment;maxLength=4"); ext.setConfig(config); ext.setNextOutgoingFrames(capture);