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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org