Reworking CloseFrame and ClosePayloadParser due to discoveries during testing
This commit is contained in:
parent
928baef5d2
commit
787a87468f
|
@ -1,86 +1,100 @@
|
|||
package org.eclipse.jetty.websocket.frames;
|
||||
|
||||
import java.nio.ByteBuffer;
|
||||
|
||||
import org.eclipse.jetty.util.BufferUtil;
|
||||
import org.eclipse.jetty.util.StringUtil;
|
||||
import org.eclipse.jetty.websocket.api.OpCode;
|
||||
import org.eclipse.jetty.websocket.api.StatusCode;
|
||||
|
||||
/**
|
||||
* Representation of a <a href="https://tools.ietf.org/html/rfc6455#section-5.5.1">Close Frame (0x08)</a>.
|
||||
*/
|
||||
public class CloseFrame extends ControlFrame
|
||||
{
|
||||
private int statusCode = 0;
|
||||
private String reason = "";
|
||||
|
||||
/**
|
||||
* Construct CloseFrame with no status code or reason
|
||||
*/
|
||||
public CloseFrame()
|
||||
{
|
||||
super(OpCode.CLOSE);
|
||||
// no status code, no reason
|
||||
setPayload(BufferUtil.EMPTY_BUFFER);
|
||||
}
|
||||
|
||||
/**
|
||||
* Construct CloseFrame with status code and no reason
|
||||
*/
|
||||
public CloseFrame(int statusCode)
|
||||
{
|
||||
super(OpCode.CLOSE);
|
||||
setStatusCode(statusCode);
|
||||
constructPayload(statusCode,null);
|
||||
}
|
||||
|
||||
/**
|
||||
* Construct CloseFrame with status code and reason
|
||||
*/
|
||||
public CloseFrame(int statusCode, String reason)
|
||||
{
|
||||
super(OpCode.CLOSE);
|
||||
constructPayload(statusCode,reason);
|
||||
}
|
||||
|
||||
private void constructPayload(int statusCode, String reason)
|
||||
{
|
||||
if ((statusCode <= 999) || (statusCode > 65535))
|
||||
{
|
||||
throw new IllegalArgumentException("Status Codes must be in the range 1000 - 65535");
|
||||
}
|
||||
|
||||
byte utf[] = null;
|
||||
int len = 2; // status code
|
||||
if (StringUtil.isNotBlank(reason))
|
||||
{
|
||||
utf = reason.getBytes(StringUtil.__UTF8_CHARSET);
|
||||
len += utf.length;
|
||||
}
|
||||
|
||||
ByteBuffer payload = ByteBuffer.allocate(len);
|
||||
payload.putShort((short)statusCode);
|
||||
if (utf != null)
|
||||
{
|
||||
payload.put(utf,0,utf.length);
|
||||
}
|
||||
setPayload(payload);
|
||||
}
|
||||
|
||||
public String getReason()
|
||||
{
|
||||
return reason;
|
||||
if (getPayloadLength() <= 2)
|
||||
{
|
||||
return null;
|
||||
}
|
||||
ByteBuffer payload = getPayload();
|
||||
int len = getPayloadLength() - 2;
|
||||
byte utf[] = new byte[len];
|
||||
for (int i = 2; i < len; i++)
|
||||
{
|
||||
utf[i - 2] = payload.get(i);
|
||||
}
|
||||
return StringUtil.toUTF8String(utf,0,utf.length);
|
||||
}
|
||||
|
||||
public int getStatusCode()
|
||||
{
|
||||
if (getPayloadLength() < 2)
|
||||
{
|
||||
return 0; // no status code
|
||||
}
|
||||
|
||||
int statusCode = 0;
|
||||
ByteBuffer payload = getPayload();
|
||||
statusCode = (payload.get(0) << 8) & payload.get(1);
|
||||
return statusCode;
|
||||
}
|
||||
|
||||
public boolean hasReason()
|
||||
{
|
||||
return StringUtil.isBlank(reason);
|
||||
}
|
||||
|
||||
public void setReason(String reason)
|
||||
{
|
||||
this.reason = reason;
|
||||
}
|
||||
|
||||
public void setStatusCode(int statusCode)
|
||||
{
|
||||
if ( ( statusCode <= 999) || ( statusCode > 65535 ) )
|
||||
{
|
||||
throw new IllegalArgumentException("Status Codes must be in the range 1000 - 65535");
|
||||
}
|
||||
|
||||
this.statusCode = statusCode;
|
||||
}
|
||||
|
||||
@Override
|
||||
public int getPayloadLength()
|
||||
{
|
||||
/*
|
||||
* issue here is that the parser can set the payload length and then rely on it when parsing payload
|
||||
*
|
||||
* but generator doesn't have a payload length without calculating it via the statuscode and reason
|
||||
*
|
||||
*/
|
||||
if (super.getPayloadLength() == 0)
|
||||
{
|
||||
int length = 0;
|
||||
if (getStatusCode() != 0)
|
||||
{
|
||||
length = length + 2;
|
||||
|
||||
if (hasReason())
|
||||
{
|
||||
length = length + getReason().getBytes().length;
|
||||
}
|
||||
}
|
||||
|
||||
return length;
|
||||
}
|
||||
else
|
||||
{
|
||||
return super.getPayloadLength();
|
||||
}
|
||||
return getPayloadLength() > 2;
|
||||
}
|
||||
|
||||
@Override
|
||||
|
@ -89,8 +103,8 @@ public class CloseFrame extends ControlFrame
|
|||
StringBuilder b = new StringBuilder();
|
||||
b.append("CloseFrame[");
|
||||
b.append("len=").append(getPayloadLength());
|
||||
b.append(",statusCode=").append(statusCode);
|
||||
b.append(",reason=").append(reason);
|
||||
b.append(",statusCode=").append(getStatusCode());
|
||||
b.append(",reason=").append(getReason());
|
||||
b.append("]");
|
||||
return b.toString();
|
||||
}
|
||||
|
|
|
@ -2,8 +2,6 @@ package org.eclipse.jetty.websocket.parser;
|
|||
|
||||
import java.nio.ByteBuffer;
|
||||
|
||||
import org.eclipse.jetty.util.BufferUtil;
|
||||
import org.eclipse.jetty.util.StringUtil;
|
||||
import org.eclipse.jetty.websocket.api.WebSocketException;
|
||||
import org.eclipse.jetty.websocket.api.WebSocketPolicy;
|
||||
import org.eclipse.jetty.websocket.frames.CloseFrame;
|
||||
|
@ -45,34 +43,28 @@ public class ClosePayloadParser extends FrameParser<CloseFrame>
|
|||
// no status code. no reason.
|
||||
return true;
|
||||
}
|
||||
|
||||
/* invalid payload length and protects payload.getShort() call below from
|
||||
* pulling too many bytes from buffer.
|
||||
|
||||
/*
|
||||
* invalid payload length.
|
||||
*/
|
||||
if ( payloadLength == 1 )
|
||||
if (payloadLength == 1)
|
||||
{
|
||||
throw new WebSocketException("Close: invalid payload length: 1");
|
||||
}
|
||||
|
||||
if (payload == null)
|
||||
{
|
||||
getPolicy().assertValidTextMessageSize(payloadLength);
|
||||
payload = ByteBuffer.allocate(payloadLength);
|
||||
}
|
||||
|
||||
while (buffer.hasRemaining())
|
||||
{
|
||||
if (payload == null)
|
||||
{
|
||||
getPolicy().assertValidBinaryMessageSize(payloadLength);
|
||||
payload = ByteBuffer.allocate(payloadLength);
|
||||
}
|
||||
|
||||
copyBuffer(buffer,payload,payload.remaining());
|
||||
|
||||
if (payload.position() >= payloadLength)
|
||||
{
|
||||
payload.flip();
|
||||
frame.setStatusCode(payload.getShort());
|
||||
if (payload.remaining() > 0)
|
||||
{
|
||||
String reason = BufferUtil.toString(payload,StringUtil.__UTF8_CHARSET);
|
||||
frame.setReason(reason);
|
||||
}
|
||||
frame.setPayload(payload);
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
@ -83,6 +75,7 @@ public class ClosePayloadParser extends FrameParser<CloseFrame>
|
|||
public void reset()
|
||||
{
|
||||
super.reset();
|
||||
|
||||
payloadLength = 0;
|
||||
payload = null;
|
||||
}
|
||||
}
|
||||
|
|
|
@ -164,14 +164,13 @@ public class Parser
|
|||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
/*
|
||||
* if the payload was empty we could end up in this state
|
||||
* because there was no remaining bits to process
|
||||
*/
|
||||
*/
|
||||
if ( state == State.PAYLOAD )
|
||||
{
|
||||
parser.getFrame().setPayload(ByteBuffer.allocate(0));
|
||||
notifyFrame( parser.getFrame() );
|
||||
parser.reset();
|
||||
if (parser.getFrame().isFin())
|
||||
|
@ -180,7 +179,7 @@ public class Parser
|
|||
}
|
||||
state = State.FINOP;
|
||||
}
|
||||
|
||||
|
||||
}
|
||||
catch (WebSocketException e)
|
||||
{
|
||||
|
@ -191,7 +190,7 @@ public class Parser
|
|||
notifyWebSocketException(new WebSocketException(t));
|
||||
}
|
||||
finally
|
||||
{
|
||||
{
|
||||
// Be sure to consume after exceptions
|
||||
buffer.position(buffer.limit());
|
||||
}
|
||||
|
|
|
@ -61,5 +61,6 @@ public class TextPayloadParser extends FrameParser<TextFrame>
|
|||
{
|
||||
super.reset();
|
||||
payloadLength = 0;
|
||||
payload = null;
|
||||
}
|
||||
}
|
||||
|
|
|
@ -34,6 +34,13 @@ public class ByteBufferAssert
|
|||
|
||||
public static void assertSize(String message, int expectedSize, ByteBuffer buffer)
|
||||
{
|
||||
Assert.assertThat(message + " buffer.remaining",buffer.remaining(),is(expectedSize));
|
||||
if (expectedSize == 0)
|
||||
{
|
||||
Assert.assertThat(message + " buffer",buffer,nullValue());
|
||||
}
|
||||
else
|
||||
{
|
||||
Assert.assertThat(message + " buffer.remaining",buffer.remaining(),is(expectedSize));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -5,6 +5,8 @@ import static org.hamcrest.Matchers.*;
|
|||
import java.nio.ByteBuffer;
|
||||
|
||||
import org.eclipse.jetty.util.StringUtil;
|
||||
import org.eclipse.jetty.websocket.Debug;
|
||||
import org.eclipse.jetty.websocket.api.OpCode;
|
||||
import org.eclipse.jetty.websocket.api.StatusCode;
|
||||
import org.eclipse.jetty.websocket.api.WebSocketBehavior;
|
||||
import org.eclipse.jetty.websocket.api.WebSocketPolicy;
|
||||
|
@ -19,6 +21,9 @@ public class ClosePayloadParserTest
|
|||
{
|
||||
String expectedReason = "Game Over";
|
||||
|
||||
Debug.enableDebugLogging(Parser.class);
|
||||
Debug.enableDebugLogging(ClosePayloadParser.class);
|
||||
|
||||
byte utf[] = expectedReason.getBytes(StringUtil.__UTF8_CHARSET);
|
||||
ByteBuffer payload = ByteBuffer.allocate(utf.length + 2);
|
||||
payload.putShort(StatusCode.NORMAL);
|
||||
|
@ -26,7 +31,7 @@ public class ClosePayloadParserTest
|
|||
payload.flip();
|
||||
|
||||
ByteBuffer buf = ByteBuffer.allocate(24);
|
||||
buf.put((byte)(0x80 | 0x08)); // fin + close
|
||||
buf.put((byte)(0x80 | OpCode.CLOSE.getCode())); // fin + close
|
||||
buf.put((byte)(0x80 | payload.remaining()));
|
||||
MaskedByteBuffer.putMask(buf);
|
||||
MaskedByteBuffer.putPayload(buf,payload);
|
||||
|
@ -40,8 +45,8 @@ public class ClosePayloadParserTest
|
|||
|
||||
capture.assertNoErrors();
|
||||
capture.assertHasFrame(CloseFrame.class,1);
|
||||
CloseFrame txt = (CloseFrame)capture.getFrames().get(0);
|
||||
Assert.assertThat("CloseFrame.statusCode",(short)txt.getStatusCode(),is(StatusCode.NORMAL));
|
||||
Assert.assertThat("CloseFrame.data",txt.getReason(),is(expectedReason));
|
||||
CloseFrame close = (CloseFrame)capture.getFrames().get(0);
|
||||
Assert.assertThat("CloseFrame.statusCode",(short)close.getStatusCode(),is((short)0));
|
||||
Assert.assertThat("CloseFrame.data",close.getReason(),is(expectedReason));
|
||||
}
|
||||
}
|
||||
|
|
|
@ -5,12 +5,15 @@ import static org.hamcrest.Matchers.*;
|
|||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
|
||||
import org.eclipse.jetty.util.log.Log;
|
||||
import org.eclipse.jetty.util.log.Logger;
|
||||
import org.eclipse.jetty.websocket.api.WebSocketException;
|
||||
import org.eclipse.jetty.websocket.frames.BaseFrame;
|
||||
import org.junit.Assert;
|
||||
|
||||
public class FrameParseCapture implements Parser.Listener
|
||||
{
|
||||
private static final Logger LOG = Log.getLogger(FrameParseCapture.class);
|
||||
private List<BaseFrame> frames = new ArrayList<>();
|
||||
private List<WebSocketException> errors = new ArrayList<>();
|
||||
|
||||
|
@ -82,6 +85,7 @@ public class FrameParseCapture implements Parser.Listener
|
|||
@Override
|
||||
public void onWebSocketException(WebSocketException e)
|
||||
{
|
||||
LOG.warn(e);
|
||||
errors.add(e);
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue