Fixing various Parser bugs identified by test cases

+ Mask bytes were read backwards during slow parse
+ Bad continuation frame detection added
+ Bad fragmented data frame detection added
+ Correcting WebSocketFrame copy constructor and FIN bit
This commit is contained in:
Joakim Erdfelt 2012-08-02 13:02:54 -07:00
parent 01c40b5290
commit 000657ddb5
14 changed files with 269 additions and 105 deletions

View File

@ -29,7 +29,7 @@ public class FragmentExtension extends Extension
private int maxLength = -1;
@Override
public <C> void output(C context, Callback<C> callback, WebSocketFrame frame) throws IOException
public <C> void output(C context, Callback<C> callback, final WebSocketFrame frame) throws IOException
{
if (frame.isControlFrame())
{

View File

@ -49,6 +49,7 @@ public class Parser
private int cursor = 0;
// Frame
private WebSocketFrame frame;
private WebSocketFrame priorDataFrame;
private byte lastDataOpcode;
// payload specific
private ByteBuffer payload;
@ -177,8 +178,15 @@ public class Parser
{
LOG.debug("{} Parsed Frame: {}",policy.getBehavior(),frame);
notifyFrame(frame);
if (frame.isDataFrame() && frame.isFin())
{
priorDataFrame = null;
}
else
{
priorDataFrame = frame;
}
}
}
catch (WebSocketException e)
{
@ -267,20 +275,35 @@ public class Parser
throw new ProtocolException("RSV3 not allowed to be set");
}
if (OpCode.isControlFrame(opcode) && !fin)
{
throw new ProtocolException("Fragmented Control Frame [" + OpCode.name(opcode) + "]");
}
boolean isContinuation = false;
if (opcode == OpCode.CONTINUATION)
if (OpCode.isControlFrame(opcode))
{
if (frame == null)
// control frame validation
if (!fin)
{
throw new ProtocolException("Fragment continuation frame without prior !FIN");
throw new ProtocolException("Fragmented Control Frame [" + OpCode.name(opcode) + "]");
}
}
else if (opcode == OpCode.CONTINUATION)
{
isContinuation = true;
// continuation validation
if (priorDataFrame == null)
{
throw new ProtocolException("CONTINUATION frame without prior !FIN");
}
// Be careful to use the original opcode
opcode = lastDataOpcode;
}
else if (OpCode.isDataFrame(opcode))
{
// data validation
if ((priorDataFrame != null) && (!priorDataFrame.isFin()))
{
throw new ProtocolException("Unexpected " + OpCode.name(opcode) + " frame, was expecting CONTINUATION");
}
}
// base framing flags
frame = new WebSocketFrame();
@ -289,6 +312,7 @@ public class Parser
frame.setRsv2(rsv2);
frame.setRsv3(rsv3);
frame.setOpCode(opcode);
frame.setContinuation(isContinuation);
if (frame.isDataFrame())
{
@ -435,7 +459,7 @@ public class Parser
* the payload buffer
* @return true if payload is done reading, false if incomplete
*/
public boolean parsePayload(ByteBuffer buffer)
private boolean parsePayload(ByteBuffer buffer)
{
if (payloadLength == 0)
{

View File

@ -113,6 +113,7 @@ public class WebSocketFrame implements Frame
*/
public WebSocketFrame(byte opcode)
{
reset();
this.opcode = opcode;
}
@ -126,7 +127,7 @@ public class WebSocketFrame implements Frame
*/
public WebSocketFrame(WebSocketFrame copy)
{
fin = copy.rsv1;
fin = copy.fin;
rsv1 = copy.rsv2;
rsv2 = copy.rsv2;
rsv3 = copy.rsv3;

View File

@ -232,7 +232,7 @@ public class DeflateFrameExtensionTest
Assert.assertThat(prefix + ".opcode",actual.getOpCode(),is(OpCode.TEXT));
Assert.assertThat(prefix + ".fin",actual.isFin(),is(true));
Assert.assertThat(prefix + ".rsv1",actual.isRsv1(),is(true));
Assert.assertThat(prefix + ".rsv1",actual.isRsv1(),is(false)); // RSV1 should be unset at this point
Assert.assertThat(prefix + ".rsv2",actual.isRsv2(),is(false));
Assert.assertThat(prefix + ".rsv3",actual.isRsv3(),is(false));

View File

@ -88,7 +88,7 @@ public class FragmentExtensionTest
quote.add("a single experiment can prove me wrong.");
quote.add("-- Albert Einstein");
// Manually compress frame and pass into extension
// Manually create frame and pass into extension
for (String q : quote)
{
WebSocketFrame frame = WebSocketFrame.text(q);

View File

@ -16,28 +16,14 @@ import org.junit.Test;
public class GeneratorTest
{
private void parsePartial(Parser parser, ByteBuffer buf, int numBytes)
{
int len = Math.min(numBytes,buf.remaining());
byte arr[] = new byte[len];
buf.get(arr,0,len);
parser.parse(ByteBuffer.wrap(arr));
}
/**
* Prevent regression of masking of many packets.
*/
@Test
public void testManyMasked()
{
byte[] MASK =
{ 0x11, 0x22, 0x33, 0x44 };
int pingCount = 10;
// the generator
Generator generator = new UnitGenerator();
// Prepare frames
List<WebSocketFrame> send = new ArrayList<>();
for (int i = 0; i < pingCount; i++)
@ -47,38 +33,15 @@ public class GeneratorTest
}
send.add(new CloseInfo(StatusCode.NORMAL).asFrame());
// Generate into single bytebuffer
int buflen = 0;
for (WebSocketFrame f : send)
{
buflen += f.getPayloadLength() + Generator.OVERHEAD;
}
ByteBuffer completeBuf = ByteBuffer.allocate(buflen);
BufferUtil.clearToFill(completeBuf);
// Generate frames
for (WebSocketFrame f : send)
{
f.setMask(MASK); // make sure we have mask set
ByteBuffer slice = f.getPayload().slice();
BufferUtil.put(generator.generate(f),completeBuf);
f.setPayload(slice);
}
BufferUtil.flipToFlush(completeBuf,0);
ByteBuffer completeBuf = UnitGenerator.generate(send);
// Parse complete buffer (5 bytes at a time)
WebSocketPolicy policy = WebSocketPolicy.newServerPolicy();
Parser parser = new Parser(policy);
UnitParser parser = new UnitParser();
IncomingFramesCapture capture = new IncomingFramesCapture();
parser.setIncomingFramesHandler(capture);
int segmentSize = 5;
while (completeBuf.remaining() > 0)
{
parsePartial(parser,completeBuf,segmentSize);
}
capture.dump();
parser.parseSlowly(completeBuf,segmentSize);
// Assert validity of frame
int frameCount = send.size();

View File

@ -18,15 +18,151 @@ package org.eclipse.jetty.websocket.protocol;
import static org.hamcrest.Matchers.*;
import java.nio.ByteBuffer;
import java.util.ArrayList;
import java.util.List;
import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.websocket.api.StatusCode;
import org.eclipse.jetty.websocket.api.WebSocketBehavior;
import org.eclipse.jetty.websocket.api.WebSocketPolicy;
import org.eclipse.jetty.websocket.protocol.Parser;
import org.junit.Assert;
import org.junit.Test;
public class ParserTest
{
/**
* Similar to the server side 5.15 testcase. A normal 2 fragment text text message, followed by another continuation.
*/
@Test
public void testParseCase5_15()
{
List<WebSocketFrame> send = new ArrayList<>();
send.add(new WebSocketFrame(OpCode.TEXT).setPayload("fragment1").setFin(false));
send.add(new WebSocketFrame(OpCode.CONTINUATION).setPayload("fragment2").setFin(true));
send.add(new WebSocketFrame(OpCode.CONTINUATION).setPayload("fragment3").setFin(false)); // bad frame
send.add(new WebSocketFrame(OpCode.TEXT).setPayload("fragment4").setFin(true));
send.add(new CloseInfo(StatusCode.NORMAL).asFrame());
ByteBuffer completeBuf = UnitGenerator.generate(send);
UnitParser parser = new UnitParser();
IncomingFramesCapture capture = new IncomingFramesCapture();
parser.setIncomingFramesHandler(capture);
parser.parse(completeBuf);
capture.assertErrorCount(1);
capture.assertHasFrame(OpCode.TEXT,2);
}
/**
* Similar to the server side 5.18 testcase. Text message fragmented as 2 frames, both as opcode=TEXT
*/
@Test
public void testParseCase5_18()
{
List<WebSocketFrame> send = new ArrayList<>();
send.add(new WebSocketFrame(OpCode.TEXT).setPayload("fragment1").setFin(false));
send.add(new WebSocketFrame(OpCode.TEXT).setPayload("fragment2").setFin(true)); // bad frame, must be continuation
send.add(new CloseInfo(StatusCode.NORMAL).asFrame());
ByteBuffer completeBuf = UnitGenerator.generate(send);
UnitParser parser = new UnitParser();
IncomingFramesCapture capture = new IncomingFramesCapture();
parser.setIncomingFramesHandler(capture);
parser.parse(completeBuf);
capture.assertErrorCount(1);
capture.assertHasFrame(OpCode.TEXT,1); // fragment 1
}
/**
* Similar to the server side 5.19 testcase.
* text message, send in 5 frames/fragments, with 2 pings in the mix.
*/
@Test
public void testParseCase5_19()
{
List<WebSocketFrame> send = new ArrayList<>();
send.add(new WebSocketFrame(OpCode.TEXT).setPayload("f1").setFin(false));
send.add(new WebSocketFrame(OpCode.CONTINUATION).setPayload(",f2").setFin(false));
send.add(new WebSocketFrame(OpCode.PING).setPayload("pong-1"));
send.add(new WebSocketFrame(OpCode.CONTINUATION).setPayload(",f3").setFin(false));
send.add(new WebSocketFrame(OpCode.CONTINUATION).setPayload(",f4").setFin(false));
send.add(new WebSocketFrame(OpCode.PING).setPayload("pong-2"));
send.add(new WebSocketFrame(OpCode.CONTINUATION).setPayload(",f5").setFin(true));
send.add(new CloseInfo(StatusCode.NORMAL).asFrame());
ByteBuffer completeBuf = UnitGenerator.generate(send);
UnitParser parser = new UnitParser();
IncomingFramesCapture capture = new IncomingFramesCapture();
parser.setIncomingFramesHandler(capture);
parser.parse(completeBuf);
capture.assertErrorCount(0);
capture.assertHasFrame(OpCode.TEXT,5);
capture.assertHasFrame(OpCode.CLOSE,1);
capture.assertHasFrame(OpCode.PING,2);
}
/**
* Similar to the server side 5.6 testcase. pong, then text, then close frames.
*/
@Test
public void testParseCase5_6()
{
List<WebSocketFrame> send = new ArrayList<>();
send.add(WebSocketFrame.pong().setPayload("ping"));
send.add(WebSocketFrame.text("hello, world"));
send.add(new CloseInfo(StatusCode.NORMAL).asFrame());
ByteBuffer completeBuf = UnitGenerator.generate(send);
UnitParser parser = new UnitParser();
IncomingFramesCapture capture = new IncomingFramesCapture();
parser.setIncomingFramesHandler(capture);
parser.parse(completeBuf);
capture.assertErrorCount(0);
capture.assertHasFrame(OpCode.TEXT,1);
capture.assertHasFrame(OpCode.CLOSE,1);
capture.assertHasFrame(OpCode.PONG,1);
}
/**
* Similar to the server side 6.2.3 testcase. Lots of small 1 byte UTF8 Text frames, representing 1 overall text message.
*/
@Test
public void testParseCase6_2_3()
{
String utf8 = "Hello-\uC2B5@\uC39F\uC3A4\uC3BC\uC3A0\uC3A1-UTF-8!!";
byte msg[] = StringUtil.getUtf8Bytes(utf8);
List<WebSocketFrame> send = new ArrayList<>();
int len = msg.length;
byte opcode = OpCode.TEXT;
byte mini[];
for (int i = 0; i < len; i++)
{
WebSocketFrame frame = new WebSocketFrame(opcode);
mini = new byte[1];
mini[0] = msg[i];
frame.setPayload(mini);
boolean isLast = (i >= (len - 1));
frame.setFin(isLast);
send.add(frame);
opcode = OpCode.CONTINUATION;
}
send.add(new CloseInfo(StatusCode.NORMAL).asFrame());
ByteBuffer completeBuf = UnitGenerator.generate(send);
UnitParser parser = new UnitParser();
IncomingFramesCapture capture = new IncomingFramesCapture();
parser.setIncomingFramesHandler(capture);
parser.parse(completeBuf);
capture.assertErrorCount(0);
capture.assertHasFrame(OpCode.TEXT,len);
capture.assertHasFrame(OpCode.CLOSE,1);
}
@Test
public void testParseNothing()
{

View File

@ -15,7 +15,11 @@
//========================================================================
package org.eclipse.jetty.websocket.protocol;
import java.nio.ByteBuffer;
import java.util.List;
import org.eclipse.jetty.io.StandardByteBufferPool;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.websocket.api.WebSocketPolicy;
/**
@ -23,6 +27,37 @@ import org.eclipse.jetty.websocket.api.WebSocketPolicy;
*/
public class UnitGenerator extends Generator
{
public static ByteBuffer generate(List<WebSocketFrame> frames)
{
// Create non-symmetrical mask (shows mask bytes order issues)
byte[] MASK =
{ 0x11, 0x22, 0x33, 0x44 };
// the generator
Generator generator = new UnitGenerator();
// Generate into single bytebuffer
int buflen = 0;
for (WebSocketFrame f : frames)
{
buflen += f.getPayloadLength() + Generator.OVERHEAD;
}
ByteBuffer completeBuf = ByteBuffer.allocate(buflen);
BufferUtil.clearToFill(completeBuf);
// Generate frames
for (WebSocketFrame f : frames)
{
f.setMask(MASK); // make sure we have mask set
ByteBuffer slice = f.getPayload().slice();
BufferUtil.put(generator.generate(f),completeBuf);
f.setPayload(slice);
}
BufferUtil.flipToFlush(completeBuf,0);
return completeBuf;
}
public UnitGenerator()
{
super(WebSocketPolicy.newServerPolicy(),new StandardByteBufferPool());

View File

@ -0,0 +1,29 @@
package org.eclipse.jetty.websocket.protocol;
import java.nio.ByteBuffer;
import org.eclipse.jetty.websocket.api.WebSocketPolicy;
public class UnitParser extends Parser
{
public UnitParser()
{
super(WebSocketPolicy.newServerPolicy());
}
private void parsePartial(ByteBuffer buf, int numBytes)
{
int len = Math.min(numBytes,buf.remaining());
byte arr[] = new byte[len];
buf.get(arr,0,len);
this.parse(ByteBuffer.wrap(arr));
}
public void parseSlowly(ByteBuffer buf, int segmentSize)
{
while (buf.remaining() > 0)
{
parsePartial(buf,segmentSize);
}
}
}

View File

@ -26,7 +26,6 @@ import org.eclipse.jetty.websocket.server.helper.IncomingFramesCapture;
import org.junit.AfterClass;
import org.junit.Assert;
import org.junit.BeforeClass;
import org.junit.Ignore;
import org.junit.Test;
public class DeflateExtensionTest
@ -47,7 +46,6 @@ public class DeflateExtensionTest
}
@Test
@Ignore /* FIXME */
public void testDeflateFrameExtension() throws Exception
{
BlockheadClient client = new BlockheadClient(server.getServerUri());

View File

@ -23,7 +23,6 @@ import org.eclipse.jetty.websocket.api.StatusCode;
import org.eclipse.jetty.websocket.protocol.CloseInfo;
import org.eclipse.jetty.websocket.protocol.WebSocketFrame;
import org.eclipse.jetty.websocket.server.ab.Fuzzer.SendMode;
import org.junit.Ignore;
import org.junit.Test;
public class TestABCase1 extends AbstractABCase
@ -184,7 +183,6 @@ public class TestABCase1 extends AbstractABCase
* Echo 65535 byte TEXT message (uses medium 2 byte payload length)
*/
@Test
@Ignore /* FIXME */
public void testCase1_1_6() throws Exception
{
byte payload[] = new byte[65535];
@ -216,7 +214,6 @@ public class TestABCase1 extends AbstractABCase
* Echo 65536 byte TEXT message (uses large 8 byte payload length)
*/
@Test
@Ignore /* FIXME */
public void testCase1_1_7() throws Exception
{
byte payload[] = new byte[65536];
@ -252,7 +249,6 @@ public class TestABCase1 extends AbstractABCase
* This is done to test the parsing together of the frame on the server side.
*/
@Test
@Ignore /* FIXME */
public void testCase1_1_8() throws Exception
{
byte payload[] = new byte[65536];
@ -438,7 +434,6 @@ public class TestABCase1 extends AbstractABCase
* Echo 65535 byte BINARY message (uses medium 2 byte payload length)
*/
@Test
@Ignore /* FIXME */
public void testCase1_2_6() throws Exception
{
byte payload[] = new byte[65535];
@ -470,7 +465,6 @@ public class TestABCase1 extends AbstractABCase
* Echo 65536 byte BINARY message (uses large 8 byte payload length)
*/
@Test
@Ignore /* FIXME */
public void testCase1_2_7() throws Exception
{
byte payload[] = new byte[65536];
@ -506,7 +500,6 @@ public class TestABCase1 extends AbstractABCase
* This is done to test the parsing together of the frame on the server side.
*/
@Test
@Ignore /* FIXME */
public void testCase1_2_8() throws Exception
{
byte payload[] = new byte[65536];

View File

@ -26,7 +26,6 @@ import org.eclipse.jetty.websocket.api.StatusCode;
import org.eclipse.jetty.websocket.protocol.CloseInfo;
import org.eclipse.jetty.websocket.protocol.OpCode;
import org.eclipse.jetty.websocket.protocol.WebSocketFrame;
import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
@ -242,7 +241,6 @@ public class TestABCase5 extends AbstractABCase
* Send text fragmented properly in 2 frames, then continuation!fin, then text unfragmented.
*/
@Test
@Ignore /* FIXME */
public void testCase5_15() throws Exception
{
List<WebSocketFrame> send = new ArrayList<>();
@ -253,7 +251,7 @@ public class TestABCase5 extends AbstractABCase
send.add(new CloseInfo(StatusCode.NORMAL).asFrame());
List<WebSocketFrame> expect = new ArrayList<>();
send.add(WebSocketFrame.text("fragment1fragment2"));
expect.add(WebSocketFrame.text("fragment1fragment2"));
expect.add(new CloseInfo(StatusCode.PROTOCOL).asFrame());
Fuzzer fuzzer = new Fuzzer(this);
@ -338,7 +336,6 @@ public class TestABCase5 extends AbstractABCase
* text message fragmented in 2 frames, both frames as opcode=TEXT
*/
@Test
@Ignore /* FIXME */
public void testCase5_18() throws Exception
{
List<WebSocketFrame> send = new ArrayList<>();

View File

@ -37,6 +37,27 @@ import org.junit.runner.RunWith;
@RunWith(AdvancedRunner.class)
public class TestABCase6 extends AbstractABCase
{
/**
* Split a message byte array into a series of fragments (frames + continuations) of 1 byte message contents each.
*/
protected void fragmentText(List<WebSocketFrame> frames, byte msg[])
{
int len = msg.length;
byte opcode = OpCode.TEXT;
byte mini[];
for (int i = 0; i < len; i++)
{
WebSocketFrame frame = new WebSocketFrame(opcode);
mini = new byte[1];
mini[0] = msg[i];
frame.setPayload(mini);
boolean isLast = (i >= (len - 1));
frame.setFin(isLast);
frames.add(frame);
opcode = OpCode.CONTINUATION;
}
}
/**
* text message, 1 frame, 0 length
*/
@ -198,18 +219,7 @@ public class TestABCase6 extends AbstractABCase
byte msg[] = StringUtil.getUtf8Bytes(utf8);
List<WebSocketFrame> send = new ArrayList<>();
int len = msg.length;
byte opcode = OpCode.TEXT;
byte mini[];
for (int i = 0; i < len; i++)
{
WebSocketFrame frame = new WebSocketFrame(opcode);
mini = new byte[1];
mini[0] = msg[i];
frame.setPayload(mini);
frame.setFin(!(i < (len - 1)));
send.add(frame);
}
fragmentText(send,msg);
send.add(new CloseInfo(StatusCode.NORMAL).asFrame());
List<WebSocketFrame> expect = new ArrayList<>();
@ -239,18 +249,7 @@ public class TestABCase6 extends AbstractABCase
byte msg[] = Hex.asByteArray("CEBAE1BDB9CF83CEBCCEB5");
List<WebSocketFrame> send = new ArrayList<>();
int len = msg.length;
byte opcode = OpCode.TEXT;
byte mini[];
for (int i = 0; i < len; i++)
{
WebSocketFrame frame = new WebSocketFrame(opcode);
mini = new byte[1];
mini[0] = msg[i];
frame.setPayload(mini);
frame.setFin(!(i < (len - 1)));
send.add(frame);
}
fragmentText(send,msg);
send.add(new CloseInfo(StatusCode.NORMAL).asFrame());
List<WebSocketFrame> expect = new ArrayList<>();
@ -309,18 +308,7 @@ public class TestABCase6 extends AbstractABCase
byte invalid[] = Hex.asByteArray("CEBAE1BDB9CF83CEBCCEB5EDA080656469746564");
List<WebSocketFrame> send = new ArrayList<>();
int len = invalid.length;
byte opcode = OpCode.TEXT;
byte mini[];
for (int i = 0; i < len; i++)
{
WebSocketFrame frame = new WebSocketFrame(opcode);
mini = new byte[1];
mini[0] = invalid[i];
frame.setPayload(mini);
frame.setFin(!(i < (len - 1)));
send.add(frame);
}
fragmentText(send,invalid);
send.add(new CloseInfo(StatusCode.NORMAL).asFrame());
List<WebSocketFrame> expect = new ArrayList<>();

View File

@ -14,5 +14,5 @@ org.eclipse.jetty.websocket.server.helper.RFCSocket.LEVEL=OFF
# org.eclipse.jetty.websocket.extensions.LEVEL=DEBUG
# org.eclipse.jetty.websocket.protocol.Generator.LEVEL=INFO
# org.eclipse.jetty.websocket.protocol.Parser.LEVEL=DEBUG
# org.eclipse.jetty.websocket.server.ab.LEVEL=DEBUG
# org.eclipse.jetty.websocket.server.blockhead.LEVEL=DEBUG
org.eclipse.jetty.websocket.server.ab.LEVEL=DEBUG
org.eclipse.jetty.websocket.server.blockhead.LEVEL=DEBUG