You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2015/06/04 22:07:57 UTC
svn commit: r1683622 - in /tomcat/trunk:
java/org/apache/coyote/http2/Http2Parser.java
java/org/apache/coyote/http2/Http2UpgradeHandler.java
java/org/apache/coyote/http2/LocalStrings.properties
test/org/apache/coyote/http2/Http2TestBase.java
Author: markt
Date: Thu Jun 4 20:07:56 2015
New Revision: 1683622
URL: http://svn.apache.org/r1683622
Log:
Switch to using new Enum for frame types
Modified:
tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java
tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java
tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties
tomcat/trunk/test/org/apache/coyote/http2/Http2TestBase.java
Modified: tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java?rev=1683622&r1=1683621&r2=1683622&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java Thu Jun 4 20:07:56 2015
@@ -33,16 +33,6 @@ class Http2Parser {
static final byte[] CLIENT_PREFACE_START =
"PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n".getBytes(StandardCharsets.ISO_8859_1);
- private static final int FRAME_TYPE_DATA = 0;
- private static final int FRAME_TYPE_HEADERS = 1;
- private static final int FRAME_TYPE_PRIORITY = 2;
- private static final int FRAME_TYPE_SETTINGS = 4;
- private static final int FRAME_TYPE_PUSH_PROMISE = 5;
- private static final int FRAME_TYPE_PING = 6;
- private static final int FRAME_TYPE_GOAWAY = 7;
- private static final int FRAME_TYPE_WINDOW_UPDATE = 8;
- private static final int FRAME_TYPE_CONTINUATION = 9;
-
private final String connectionId;
private final Input input;
private final Output output;
@@ -80,48 +70,50 @@ class Http2Parser {
return readFrame(block, null);
}
- private boolean readFrame(boolean block, Integer expected) throws IOException {
+ private boolean readFrame(boolean block, FrameType expected) throws IOException {
if (!input.fill(block, frameHeaderBuffer)) {
return false;
}
int payloadSize = ByteUtil.getThreeBytes(frameHeaderBuffer, 0);
- int frameType = ByteUtil.getOneByte(frameHeaderBuffer, 3);
+ FrameType frameType = FrameType.valueOf(ByteUtil.getOneByte(frameHeaderBuffer, 3));
int flags = ByteUtil.getOneByte(frameHeaderBuffer, 4);
int streamId = ByteUtil.get31Bits(frameHeaderBuffer, 5);
validateFrame(expected, frameType, streamId, flags, payloadSize);
switch (frameType) {
- case FRAME_TYPE_DATA:
+ case DATA:
readDataFrame(streamId, flags, payloadSize);
break;
- case FRAME_TYPE_HEADERS:
+ case HEADERS:
readHeadersFrame(streamId, flags, payloadSize);
break;
- case FRAME_TYPE_PRIORITY:
- readPriorityFrame(streamId, flags, payloadSize);
+ case PRIORITY:
+ readPriorityFrame(streamId);
+ break;
+ case RST:
+ readRstFrame(streamId, payloadSize);
break;
- case FRAME_TYPE_SETTINGS:
- readSettingsFrame(streamId, flags, payloadSize);
+ case SETTINGS:
+ readSettingsFrame(flags, payloadSize);
break;
- case FRAME_TYPE_PUSH_PROMISE:
+ case PUSH_PROMISE:
readPushPromiseFrame(streamId, flags, payloadSize);
break;
- case FRAME_TYPE_PING:
- readPingFrame(streamId, flags, payloadSize);
+ case PING:
+ readPingFrame(flags);
break;
- case FRAME_TYPE_GOAWAY:
- readGoawayFrame(streamId, flags, payloadSize);
+ case GOAWAY:
+ readGoawayFrame(payloadSize);
break;
- case FRAME_TYPE_WINDOW_UPDATE:
- readWindowUpdateFrame(streamId, flags, payloadSize);
+ case WINDOW_UPDATE:
+ readWindowUpdateFrame(streamId);
break;
- case FRAME_TYPE_CONTINUATION:
+ case CONTINUATION:
readContinuationFrame(streamId, flags, payloadSize);
break;
- // TODO: Missing types
- default:
+ case UNKNOWN:
readUnknownFrame(streamId, frameType, flags, payloadSize);
}
@@ -130,12 +122,6 @@ class Http2Parser {
private void readDataFrame(int streamId, int flags, int payloadSize) throws IOException {
- // Validate the stream
- if (streamId == 0) {
- throw new Http2Exception(sm.getString("http2Parser.processFrameData.invalidStream"),
- 0, ErrorCode.PROTOCOL_ERROR);
- }
-
// Process the Stream
int padLength = 0;
@@ -168,12 +154,6 @@ class Http2Parser {
private void readHeadersFrame(int streamId, int flags, int payloadSize) throws IOException {
- // Validate the stream
- if (streamId == 0) {
- throw new Http2Exception(sm.getString("http2Parser.processFrameHeaders.invalidStream"),
- 0, ErrorCode.PROTOCOL_ERROR);
- }
-
if (hpackDecoder == null) {
hpackDecoder = output.getHpackDecoder();
}
@@ -228,17 +208,7 @@ class Http2Parser {
}
- private void readPriorityFrame(int streamId, int flags, int payloadSize) throws IOException {
- // Validate the frame
- if (streamId == 0) {
- throw new Http2Exception(sm.getString("http2Parser.processFramePriority.invalidStream"),
- 0, ErrorCode.PROTOCOL_ERROR);
- }
- if (payloadSize != 5) {
- throw new Http2Exception(sm.getString("http2Parser.processFramePriority.invalidPayloadSize",
- Integer.toString(payloadSize)), streamId, ErrorCode.FRAME_SIZE_ERROR);
- }
-
+ private void readPriorityFrame(int streamId) throws IOException {
byte[] payload = new byte[5];
input.fill(true, payload);
@@ -250,16 +220,13 @@ class Http2Parser {
}
- private void readSettingsFrame(int streamId, int flags, int payloadSize) throws IOException {
- // Validate the frame
- if (streamId != 0) {
- throw new Http2Exception(sm.getString("http2Parser.processFrameSettings.invalidStream",
- Integer.toString(streamId)), 0, ErrorCode.FRAME_SIZE_ERROR);
- }
- if (payloadSize % 6 != 0) {
- throw new Http2Exception(sm.getString("http2Parser.processFrameSettings.invalidPayloadSize",
- Integer.toString(payloadSize)), 0, ErrorCode.FRAME_SIZE_ERROR);
- }
+ private void readRstFrame(int streamId, int payloadSize) throws IOException {
+ // TODO: Process this
+ swallow(payloadSize);
+ }
+
+
+ private void readSettingsFrame(int flags, int payloadSize) throws IOException {
boolean ack = Flags.isAck(flags);
if (payloadSize > 0 && ack) {
throw new Http2Exception(sm.getString("http2Parser.processFrameSettings.ackWithNonZeroPayload"),
@@ -286,17 +253,8 @@ class Http2Parser {
}
- private void readPingFrame(int streamId, int flags, int payloadSize)
- throws IOException {
- // Validate the frame
- if (streamId != 0) {
- throw new Http2Exception(sm.getString("http2Parser.processFramePing.invalidStream",
- Integer.toString(streamId)), 0, ErrorCode.FRAME_SIZE_ERROR);
- }
- if (payloadSize != 8) {
- throw new Http2Exception(sm.getString("http2Parser.processFramePing.invalidPayloadSize",
- Integer.toString(payloadSize)), 0, ErrorCode.FRAME_SIZE_ERROR);
- }
+ private void readPingFrame(int flags) throws IOException {
+
if (Flags.isAck(flags)) {
// Read the payload
byte[] payload = new byte[8];
@@ -308,18 +266,7 @@ class Http2Parser {
}
- private void readGoawayFrame(int streamId, int flags, int payloadSize)
- throws IOException {
- // Validate the frame
- if (streamId != 0) {
- throw new Http2Exception(sm.getString("http2Parser.processFrameGoaway.invalidStream",
- Integer.toString(streamId)), 0, ErrorCode.PROTOCOL_ERROR);
- }
-
- if (payloadSize < 8) {
- throw new Http2Exception(sm.getString("http2Parser.processFrameGoaway.payloadTooSmall",
- connectionId, Integer.toString(payloadSize)), 0, ErrorCode.FRAME_SIZE_ERROR);
- }
+ private void readGoawayFrame(int payloadSize) throws IOException {
byte[] payload = new byte[payloadSize];
input.fill(true, payload);
@@ -333,14 +280,8 @@ class Http2Parser {
output.goaway(lastStreamId, errorCode, debugData);
}
- private void readWindowUpdateFrame(int streamId, int flags, int payloadSize)
+ private void readWindowUpdateFrame(int streamId)
throws IOException {
- // Validate the frame
- if (payloadSize != 4) {
- // Use stream 0 since this is always a connection error
- throw new Http2Exception(sm.getString("http2Parser.processFrameWindowUpdate.invalidPayloadSize",
- Integer.toString(payloadSize)), 0, ErrorCode.FRAME_SIZE_ERROR);
- }
byte[] payload = new byte[4];
input.fill(true, payload);
@@ -363,12 +304,6 @@ class Http2Parser {
private void readContinuationFrame(int streamId, int flags, int payloadSize)
throws IOException {
- if (streamId == 0) {
- throw new Http2Exception(sm.getString(
- "http2Parser.processFrameContinuation.invalidStream", connectionId),
- 0, ErrorCode.PROTOCOL_ERROR);
- }
-
if (headersCurrentStream == -1) {
// No headers to continue
throw new Http2Exception(sm.getString(
@@ -417,7 +352,7 @@ class Http2Parser {
}
- private void readUnknownFrame(int streamId, int frameType, int flags, int payloadSize)
+ private void readUnknownFrame(int streamId, FrameType frameType, int flags, int payloadSize)
throws IOException {
output.swallow(streamId, frameType, flags, payloadSize);
swallow(payloadSize);
@@ -438,7 +373,15 @@ class Http2Parser {
}
- private void validateFrame(Integer expected, int frameType, int streamId, int flags,
+ /*
+ * Implementation note:
+ * Validation applicable to all incoming frames should be implemented here.
+ * Frame type specific validation should be performed in the appropriate
+ * readXxxFrame() method.
+ * For validation applicable to some but not all frame types, use your
+ * judgement.
+ */
+ private void validateFrame(FrameType expected, FrameType frameType, int streamId, int flags,
int payloadSize) throws Http2Exception {
if (log.isDebugEnabled()) {
@@ -447,10 +390,9 @@ class Http2Parser {
Integer.toString(payloadSize)));
}
- if (expected != null && frameType != expected.intValue()) {
+ if (expected != null && frameType != expected) {
throw new Http2Exception(sm.getString("http2Parser.processFrame.unexpectedType",
- expected, Integer.toString(frameType)),
- streamId, ErrorCode.PROTOCOL_ERROR);
+ expected, frameType), streamId, ErrorCode.PROTOCOL_ERROR);
}
if (payloadSize > maxPayloadSize) {
@@ -465,13 +407,15 @@ class Http2Parser {
connectionId, Integer.toString(headersCurrentStream),
Integer.toString(streamId)), streamId, ErrorCode.COMPRESSION_ERROR);
}
- if (frameType != FRAME_TYPE_CONTINUATION) {
+ if (frameType != FrameType.CONTINUATION) {
throw new Http2Exception(sm.getString("http2Parser.headers.wrongFrameType",
connectionId, Integer.toString(headersCurrentStream),
- Integer.toString(frameType)), streamId, ErrorCode.COMPRESSION_ERROR);
+ frameType), streamId, ErrorCode.COMPRESSION_ERROR);
}
}
+ frameType.checkStream(connectionId, streamId);
+ frameType.checkPayloadSize(connectionId, streamId, payloadSize);
}
@@ -506,7 +450,7 @@ class Http2Parser {
}
// Must always be followed by a settings frame
- readFrame(true, Integer.valueOf(FRAME_TYPE_SETTINGS));
+ readFrame(true, FrameType.SETTINGS);
readPreface = true;
return true;
@@ -588,6 +532,6 @@ class Http2Parser {
void incrementWindowSize(int streamId, int increment) throws Http2Exception;
// Testing
- void swallow(int streamId, int frameType, int flags, int size) throws IOException;
+ void swallow(int streamId, FrameType frameType, int flags, int size) throws IOException;
}
}
Modified: tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java?rev=1683622&r1=1683621&r2=1683622&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java Thu Jun 4 20:07:56 2015
@@ -84,10 +84,6 @@ public class Http2UpgradeHandler extends
private static final int FLAG_END_OF_STREAM = 1;
private static final int FLAG_END_OF_HEADERS = 4;
- private static final int FRAME_TYPE_DATA = 0;
- private static final int FRAME_TYPE_HEADERS = 1;
- private static final int FRAME_TYPE_CONTINUATION = 9;
-
private static final byte[] PING_ACK = { 0x00, 0x00, 0x08, 0x06, 0x01, 0x00, 0x00, 0x00, 0x00 };
private static final byte[] SETTINGS_EMPTY = { 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00 };
@@ -392,12 +388,12 @@ public class Http2UpgradeHandler extends
target.flip();
ByteUtil.setThreeBytes(header, 0, target.limit());
if (first) {
- header[3] = FRAME_TYPE_HEADERS;
+ header[3] = FrameType.HEADERS.getIdByte();
if (stream.getOutputBuffer().hasNoBody()) {
header[4] = FLAG_END_OF_STREAM;
}
} else {
- header[3] = FRAME_TYPE_CONTINUATION;
+ header[3] = FrameType.CONTINUATION.getIdByte();
}
if (state == State.COMPLETE) {
header[4] += FLAG_END_OF_HEADERS;
@@ -430,7 +426,7 @@ public class Http2UpgradeHandler extends
synchronized (socketWrapper) {
byte[] header = new byte[9];
ByteUtil.setThreeBytes(header, 0, len);
- header[3] = FRAME_TYPE_DATA;
+ header[3] = FrameType.DATA.getIdByte();
if (stream.getOutputBuffer().isFinished()) {
header[4] = FLAG_END_OF_STREAM;
}
@@ -460,7 +456,7 @@ public class Http2UpgradeHandler extends
if (windowSize < 1 || backLogSize > 0) {
// Has this stream been granted an allocation
int[] value = backLogStreams.remove(stream);
- if (value[1] > 0) {
+ if (value != null && value[1] > 0) {
result = value[1];
value[0] = 0;
value[1] = 1;
@@ -811,7 +807,7 @@ public class Http2UpgradeHandler extends
@Override
- public void swallow(int streamId, int frameType, int flags, int size) throws IOException {
+ public void swallow(int streamId, FrameType frameType, int flags, int size) throws IOException {
swallow(size);
}
}
Modified: tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties?rev=1683622&r1=1683621&r2=1683622&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties Thu Jun 4 20:07:56 2015
@@ -26,6 +26,10 @@ connectionSettings.maxFrameSizeInvalid=T
connectionSettings.unknown=An unknown setting with identifier [{0}] and value [{1}] was ignored
connectionSettings.windowSizeTooBig=The requested window size of [{0}] is bigger than the maximum permitted value of [{1}]
+frameType.checkPayloadSize=Connection [{0}], Stream [{1}], Frame type [{2}], Payload size of [{3}] is not valid for this frame type
+frameType.checkStream.invalidForZero=Connection [{0}], Stream [0], received a [{1}] frame which is not valid for stream zero
+frameType.checkStream.invalidForNonZero=Connection [{0}], Stream [{1}], received a [{2}] frame which is only valid for stream zero
+
hpack.integerEncodedOverTooManyOctets=HPACK variable length integer encoded over too many octets, max is {0}
hpackdecoder.zeroNotValidHeaderTableIndex=Zero is not a valid header table index
@@ -39,21 +43,14 @@ http2Parser.preface.invalid=Invalid conn
http2Parser.preface.io=Unable to read connection preface
http2Parser.processFrame=Connection [{0}], Stream [{1}], Flags [{2}], Payload size [{3}]
http2Parser.processFrame.unexpectedType=Expected frame type [{0}] but received frame type [{1}]
-http2Parser.processFrameContinuation.invalidStream=Connection [{0}], Continuation frame received for stream [0]
http2Parser.processFrameContinuation.notExpected=Connection [{0}], Continuation frame received for stream [{1}] when no headers were in progress
-http2Parser.processFrameData.invalidStream=Data frame received for stream [0]
-http2Parser.processFrameGoaway.invalidStream=Goaway frame received for stream [{0}]
http2Parser.processFrameGoaway.payloadTooSmall=Connection [{0}]: Goaway payload size was [{1}] which is less than the minimum 8
-http2Parser.processFrameHeaders.invalidStream=Headers frame received for stream [0]
http2Parser.processFrameHeaders.decodingFailed=There was an error during the HPACK decoding of HTTP headers
http2Parser.processFrameHeaders.decodingDataLeft=Data left over after HPACK decoding - it should have been consumed
http2Parser.processFramePing.invalidPayloadSize=Settings frame received with an invalid payload size of [{0}] (should be 8)
-http2Parser.processFramePing.invalidStream=Ping frame received for stream [{0}]
http2Parser.processFramePriority.invalidPayloadSize=Priority frame received with an invalid payload size of [{0}] (should be 5)
-http2Parser.processFramePriority.invalidStream=Priority frame received for stream [0]
http2Parser.processFrameSettings.ackWithNonZeroPayload=Settings frame received with the ACK flag set and payload present
http2Parser.processFrameSettings.invalidPayloadSize=Settings frame received with a payload size of [{0}] which is not a multiple of 6
-http2Parser.processFrameSettings.invalidStream=Settings frame received for stream [{0}]
http2Parser.processFrameWindowUpdate.debug=Connection [{0}], Stream [{1}], Window size increment [{2}]
http2Parser.processFrameWindowUpdate.invalidIncrement=Window update frame received with an invalid increment size of [0]
http2Parser.processFrameWindowUpdate.invalidPayloadSize=Window update frame received with an invalid payload size of [{0}]
Modified: tomcat/trunk/test/org/apache/coyote/http2/Http2TestBase.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/coyote/http2/Http2TestBase.java?rev=1683622&r1=1683621&r2=1683622&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/coyote/http2/Http2TestBase.java (original)
+++ tomcat/trunk/test/org/apache/coyote/http2/Http2TestBase.java Thu Jun 4 20:07:56 2015
@@ -527,7 +527,7 @@ public abstract class Http2TestBase exte
@Override
- public void swallow(int streamId, int frameType, int flags, int size) {
+ public void swallow(int streamId, FrameType frameType, int flags, int size) {
trace.append(streamId);
trace.append(",");
trace.append(frameType);
@@ -538,6 +538,7 @@ public abstract class Http2TestBase exte
trace.append("\n");
}
+
public void clearTrace() {
trace = new StringBuffer();
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org