476023 - Incorrect trimming of WebSocket close reason

+ Fixed CloseStatus.trimMaxReasonLength() to perform trim correctly
+ Deprecated CloseStatus.trimMaxReasonLength() because it creates
  to many objects
+ Removed use of CloseStatus.trimMaxReasonLength() in implementation
  code
+ Improved CloseInfo ...
  + tracks reason via utf8 byte array (not String object)
  + trims utf8 byte array as-needed
+ All non-jsr implementations use CloseInfo logic
This commit is contained in:
Joakim Erdfelt 2015-08-27 11:12:51 -07:00
parent 54e3d0a2e8
commit ef067b2624
4 changed files with 70 additions and 54 deletions

View File

@ -18,33 +18,38 @@
package org.eclipse.jetty.websocket.api;
import java.nio.charset.StandardCharsets;
public class CloseStatus
{
private static final int MAX_CONTROL_PAYLOAD = 125;
private static final int MAX_REASON_PHRASE = MAX_CONTROL_PAYLOAD - 2;
public static final int MAX_REASON_PHRASE = MAX_CONTROL_PAYLOAD - 2;
/**
* Convenience method for trimming a long reason phrase at the maximum reason phrase length.
* Convenience method for trimming a long reason phrase at the maximum reason phrase length of 123 UTF-8 bytes (per WebSocket spec).
*
* @param reason
* the proposed reason phrase
* @return the reason phrase (trimmed if needed)
* @deprecated use of this method is strongly discouraged, as it creates too many new objects that are just thrown away to accomplish its goals.
*/
@Deprecated
public static String trimMaxReasonLength(String reason)
{
if (reason == null)
{
return null;
}
byte[] reasonBytes = reason.getBytes(StandardCharsets.UTF_8);
if (reasonBytes.length > MAX_REASON_PHRASE)
{
byte[] trimmed = new byte[MAX_REASON_PHRASE];
System.arraycopy(reasonBytes,0,trimmed,0,MAX_REASON_PHRASE);
return new String(trimmed,StandardCharsets.UTF_8);
}
if (reason.length() > MAX_REASON_PHRASE)
{
return reason.substring(0,MAX_REASON_PHRASE);
}
else
{
return reason;
}
return reason;
}
private int code;

View File

@ -19,14 +19,13 @@
package org.eclipse.jetty.websocket.common;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.util.Utf8Appendable.NotUtf8Exception;
import org.eclipse.jetty.util.Utf8StringBuilder;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
import org.eclipse.jetty.util.Utf8Appendable.NotUtf8Exception;
import org.eclipse.jetty.websocket.api.BadPayloadException;
import org.eclipse.jetty.websocket.api.CloseStatus;
import org.eclipse.jetty.websocket.api.ProtocolException;
import org.eclipse.jetty.websocket.api.StatusCode;
import org.eclipse.jetty.websocket.api.extensions.Frame;
@ -34,19 +33,23 @@ import org.eclipse.jetty.websocket.common.frames.CloseFrame;
public class CloseInfo
{
private static final Logger LOG = Log.getLogger(CloseInfo.class);
private int statusCode;
private String reason;
private byte[] reasonBytes;
public CloseInfo()
{
this(StatusCode.NO_CODE,null);
}
/**
* Parse the Close Frame payload.
*
* @param payload the raw close frame payload.
* @param validate true if payload should be validated per WebSocket spec.
*/
public CloseInfo(ByteBuffer payload, boolean validate)
{
this.statusCode = StatusCode.NO_CODE;
this.reason = null;
if ((payload == null) || (payload.remaining() == 0))
{
@ -77,34 +80,24 @@ public class CloseInfo
if (data.remaining() > 0)
{
// Reason
try
// Reason (trimmed to max reason size)
int len = Math.min(data.remaining(), CloseStatus.MAX_REASON_PHRASE);
reasonBytes = new byte[len];
data.get(reasonBytes,0,len);
// Spec Requirement : throw BadPayloadException on invalid UTF8
if(validate)
{
Utf8StringBuilder utf = new Utf8StringBuilder();
utf.append(data);
reason = utf.toString();
}
catch (NotUtf8Exception e)
{
if (validate)
try
{
Utf8StringBuilder utf = new Utf8StringBuilder();
// if this throws, we know we have bad UTF8
utf.append(reasonBytes,0,reasonBytes.length);
}
catch (NotUtf8Exception e)
{
throw new BadPayloadException("Invalid Close Reason",e);
}
else
{
LOG.warn(e);
}
}
catch (RuntimeException e)
{
if (validate)
{
throw new ProtocolException("Invalid Close Reason",e);
}
else
{
LOG.warn(e);
}
}
}
}
@ -125,10 +118,28 @@ public class CloseInfo
this(statusCode,null);
}
/**
* Create a CloseInfo, trimming the reason to {@link CloseStatus#MAX_REASON_PHRASE} UTF-8 bytes if needed.
*
* @param statusCode the status code
* @param reason the raw reason code
*/
public CloseInfo(int statusCode, String reason)
{
this.statusCode = statusCode;
this.reason = reason;
if (reason != null)
{
byte[] utf8Bytes = reason.getBytes(StandardCharsets.UTF_8);
if (utf8Bytes.length > CloseStatus.MAX_REASON_PHRASE)
{
this.reasonBytes = new byte[CloseStatus.MAX_REASON_PHRASE];
System.arraycopy(utf8Bytes,0,this.reasonBytes,0,CloseStatus.MAX_REASON_PHRASE);
}
else
{
this.reasonBytes = utf8Bytes;
}
}
}
private ByteBuffer asByteBuffer()
@ -140,11 +151,10 @@ public class CloseInfo
}
int len = 2; // status code
byte utf[] = null;
if (StringUtil.isNotBlank(reason))
boolean hasReason = (this.reasonBytes != null) && (this.reasonBytes.length > 0);
if (hasReason)
{
utf = StringUtil.getUtf8Bytes(reason);
len += utf.length;
len += this.reasonBytes.length;
}
ByteBuffer buf = BufferUtil.allocate(len);
@ -152,9 +162,9 @@ public class CloseInfo
buf.put((byte)((statusCode >>> 8) & 0xFF));
buf.put((byte)((statusCode >>> 0) & 0xFF));
if (utf != null)
if (hasReason)
{
buf.put(utf,0,utf.length);
buf.put(this.reasonBytes,0,this.reasonBytes.length);
}
BufferUtil.flipToFlush(buf,0);
@ -178,7 +188,11 @@ public class CloseInfo
public String getReason()
{
return reason;
if (this.reasonBytes == null)
{
return null;
}
return new String(this.reasonBytes,StandardCharsets.UTF_8);
}
public int getStatusCode()
@ -199,6 +213,6 @@ public class CloseInfo
@Override
public String toString()
{
return String.format("CloseInfo[code=%d,reason=%s]",statusCode,reason);
return String.format("CloseInfo[code=%d,reason=%s]",statusCode,getReason());
}
}

View File

@ -105,7 +105,7 @@ public class WebSocketSession extends ContainerLifeCycle implements Session, Inc
@Override
public void close(int statusCode, String reason)
{
connection.close(statusCode,CloseStatus.trimMaxReasonLength(reason));
connection.close(statusCode,reason);
}
/**

View File

@ -27,7 +27,6 @@ import java.util.concurrent.atomic.AtomicReference;
import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
import org.eclipse.jetty.websocket.api.CloseStatus;
import org.eclipse.jetty.websocket.api.StatusCode;
import org.eclipse.jetty.websocket.common.CloseInfo;
import org.eclipse.jetty.websocket.common.ConnectionState;
@ -458,7 +457,6 @@ public class IOState
}
}
reason = CloseStatus.trimMaxReasonLength(reason);
CloseInfo close = new CloseInfo(StatusCode.ABNORMAL,reason);
finalClose.compareAndSet(null,close);
@ -509,7 +507,6 @@ public class IOState
}
}
reason = CloseStatus.trimMaxReasonLength(reason);
CloseInfo close = new CloseInfo(StatusCode.ABNORMAL,reason);
finalClose.compareAndSet(null,close);