Author: markt
Date: Thu Jun 18 09:43:06 2015
New Revision: 1686156
URL: http://svn.apache.org/r1686156
Log:
Add unit tests for data frames with padding including support for simple POST
requests.
Fix errors in parsing of padded data frames.
Make parser responsible for swallowing unwanted data rather than the code using
the parser and rename output methods to make this clearer
More debug logging
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
tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_6_1.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=1686156&r1=1686155&r2=1686156&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java Thu Jun 18
09:43:06 2015
@@ -86,7 +86,7 @@ class Http2Parser {
try {
validateFrame(expected, frameType, streamId, flags, payloadSize);
} catch (StreamException se) {
- swallow(payloadSize);
+ swallow(streamId, payloadSize);
throw se;
}
@@ -136,28 +136,47 @@ class Http2Parser {
boolean endOfStream = Flags.isEndOfStream(flags);
+ int dataLength;
if (Flags.hasPadding(flags)) {
byte[] b = new byte[1];
input.fill(true, b);
padLength = b[0] & 0xFF;
+ // +1 is for the padding length byte we just read above
+ dataLength = payloadSize - (padLength + 1);
+ } else {
+ dataLength = payloadSize;
}
- ByteBuffer dest = output.getInputByteBuffer(streamId, payloadSize);
+ if (log.isDebugEnabled()) {
+ String padding;
+ if (Flags.hasPadding(flags)) {
+ padding = Integer.toString(padLength);
+ } else {
+ padding = "none";
+ }
+ log.debug(sm.getString("http2Parser.processFrameData.lengths",
connectionId,
+ Integer.toString(streamId), Integer.toString(dataLength),
padding));
+ }
+
+ ByteBuffer dest = output.getInputByteBuffer(streamId, dataLength);
if (dest == null) {
- swallow(payloadSize);
+ swallow(streamId, dataLength);
if (endOfStream) {
output.receiveEndOfStream(streamId);
}
} else {
synchronized (dest) {
- input.fill(true, dest, payloadSize);
+ input.fill(true, dest, dataLength);
if (endOfStream) {
output.receiveEndOfStream(streamId);
}
dest.notifyAll();
}
}
- swallow(padLength);
+ if (padLength > 0) {
+ swallow(streamId, padLength);
+ output.swallowedPadding(streamId, padLength);
+ }
}
@@ -170,7 +189,7 @@ class Http2Parser {
try {
hpackDecoder.setHeaderEmitter(output.headersStart(streamId));
} catch (StreamException se) {
- swallow(payloadSize);
+ swallow(streamId, payloadSize);
throw se;
}
@@ -205,7 +224,7 @@ class Http2Parser {
readHeaderBlock(payloadSize, endOfHeaders);
- swallow(padLength);
+ swallow(streamId, padLength);
if (endOfHeaders) {
output.headersEnd(streamId);
@@ -386,11 +405,16 @@ class Http2Parser {
private void readUnknownFrame(int streamId, FrameType frameType, int
flags, int payloadSize)
throws IOException {
- output.swallow(streamId, frameType, flags, payloadSize);
+ swallow(streamId, payloadSize);
+ output.swallowed(streamId, frameType, flags, payloadSize);
}
- private void swallow(int len) throws IOException {
+ private void swallow(int streamId, int len) throws IOException {
+ if (log.isDebugEnabled()) {
+ log.debug(sm.getString("http2Parser.swallow.debug", connectionId,
+ Integer.toString(streamId), Integer.toString(len)));
+ }
if (len == 0) {
return;
}
@@ -527,6 +551,7 @@ class Http2Parser {
// Data frames
ByteBuffer getInputByteBuffer(int streamId, int payloadSize) throws
Http2Exception;
void receiveEndOfStream(int streamId) throws ConnectionException;
+ void swallowedPadding(int streamId, int paddingLength) throws
ConnectionException, IOException;
// Header frames
HeaderEmitter headersStart(int streamId) throws Http2Exception;
@@ -554,6 +579,6 @@ class Http2Parser {
void incrementWindowSize(int streamId, int increment) throws
Http2Exception;
// Testing
- void swallow(int streamId, FrameType frameType, int flags, int size)
throws IOException;
+ void swallowed(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=1686156&r1=1686155&r2=1686156&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java
(original)
+++ tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java Thu Jun
18 09:43:06 2015
@@ -812,6 +812,15 @@ public class Http2UpgradeHandler extends
@Override
+ public void swallowedPadding(int streamId, int paddingLength) throws
+ ConnectionException, IOException {
+ Stream stream = getStream(streamId, true);
+ // +1 is for the payload byte used to define the padding length
+ writeWindowUpdate(stream, paddingLength + 1);
+ }
+
+
+ @Override
public HeaderEmitter headersStart(int streamId) throws Http2Exception {
Stream stream = getStream(streamId, false);
if (stream == null) {
@@ -942,7 +951,8 @@ public class Http2UpgradeHandler extends
@Override
- public void swallow(int streamId, FrameType frameType, int flags, int
size) throws IOException {
- swallow(size);
+ public void swallowed(int streamId, FrameType frameType, int flags, int
size)
+ throws IOException {
+ // NO-OP.
}
}
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=1686156&r1=1686155&r2=1686156&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties Thu Jun
18 09:43:06 2015
@@ -44,6 +44,7 @@ http2Parser.preface.io=Unable to read co
http2Parser.processFrame=Connection [{0}], Stream [{1}], Frame type [{2}],
Flags [{3}], Payload size [{4}]
http2Parser.processFrame.unexpectedType=Expected frame type [{0}] but received
frame type [{1}]
http2Parser.processFrameContinuation.notExpected=Connection [{0}],
Continuation frame received for stream [{1}] when no headers were in progress
+http2Parser.processFrameData.lengths=Connection [{0}], Stream [{1}], Data
length, [{2}], Padding length [{3}]
http2Parser.processFrameGoaway.payloadTooSmall=Connection [{0}]: Goaway
payload size was [{1}] which is less than the minimum 8
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
@@ -56,6 +57,7 @@ http2Parser.processFrameSettings.invalid
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}]
+http2Parser.swallow.debug=Connection [{0}], Stream [{1}], Swallowed [{2}] bytes
stream.header.debug=Connection [{0}], Stream [{1}], HTTP header [{2}], Value
[{3}]
stream.reprioritisation.debug=Connection [{0}], Stream [{1}], Exclusive [{2}],
Parent [{3}], Weight [{4}]
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=1686156&r1=1686155&r2=1686156&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/coyote/http2/Http2TestBase.java (original)
+++ tomcat/trunk/test/org/apache/coyote/http2/Http2TestBase.java Thu Jun 18
09:43:06 2015
@@ -193,21 +193,21 @@ public abstract class Http2TestBase exte
}
- protected void sendSimplePostRequest(int streamId) throws IOException {
+ protected void sendSimplePostRequest(int streamId, byte[] padding) throws
IOException {
byte[] headersFrameHeader = new byte[9];
ByteBuffer headersPayload = ByteBuffer.allocate(128);
byte[] dataFrameHeader = new byte[9];
ByteBuffer dataPayload = ByteBuffer.allocate(128);
buildPostRequest(headersFrameHeader, headersPayload,
- dataFrameHeader, dataPayload, streamId);
+ dataFrameHeader, dataPayload, padding, streamId);
writeFrame(headersFrameHeader, headersPayload);
writeFrame(dataFrameHeader, dataPayload);
}
protected void buildPostRequest(byte[] headersFrameHeader, ByteBuffer
headersPayload,
- byte[] dataFrameHeader, ByteBuffer dataPayload, int streamId) {
+ byte[] dataFrameHeader, ByteBuffer dataPayload, byte[] padding,
int streamId) {
MimeHeaders headers = new MimeHeaders();
headers.addValue(":method").setString("POST");
headers.addValue(":path").setString("/simple");
@@ -225,16 +225,30 @@ public abstract class Http2TestBase exte
ByteUtil.set31Bits(headersFrameHeader, 5, streamId);
// Data
+ if (padding != null) {
+ dataPayload.put((byte) (padding.length & 0xFF));
+ dataPayload.limit(dataPayload.capacity() - padding.length);
+ }
+
while (dataPayload.hasRemaining()) {
dataPayload.put((byte) 'x');
}
+ if (padding != null && padding.length > 0) {
+ dataPayload.limit(dataPayload.capacity());
+ dataPayload.put(padding);
+ }
+
dataPayload.flip();
// Size
ByteUtil.setThreeBytes(dataFrameHeader, 0, dataPayload.limit());
// Data is type 0
- // End of stream
- dataFrameHeader[4] = 0x01;
+ // Flags: End of stream 1, Padding 8
+ if (padding == null) {
+ dataFrameHeader[4] = 0x01;
+ } else {
+ dataFrameHeader[4] = 0x09;
+ }
ByteUtil.set31Bits(dataFrameHeader, 5, streamId);
}
@@ -254,7 +268,12 @@ public abstract class Http2TestBase exte
}
- protected void readSimplePostResponse() throws Http2Exception, IOException
{
+ protected void readSimplePostResponse(boolean padding) throws
Http2Exception, IOException {
+ if (padding) {
+ // Window updates for padding
+ parser.readFrame(true);
+ parser.readFrame(true);
+ }
// Connection window update after reading request body
parser.readFrame(true);
// Stream window update after reading request body
@@ -675,7 +694,7 @@ public abstract class Http2TestBase exte
@Override
- public void swallow(int streamId, FrameType frameType, int flags, int
size) {
+ public void swallowed(int streamId, FrameType frameType, int flags,
int size) {
trace.append(streamId);
trace.append(",");
trace.append(frameType);
@@ -687,6 +706,15 @@ public abstract class Http2TestBase exte
}
+ @Override
+ public void swallowedPadding(int streamId, int paddingLength) {
+ trace.append(streamId);
+ trace.append("-SwallowedPadding-[");
+ trace.append(paddingLength);
+ trace.append("]\n");
+ }
+
+
public void clearTrace() {
trace = new StringBuffer();
}
Modified: tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_6_1.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_6_1.java?rev=1686156&r1=1686155&r2=1686156&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_6_1.java
(original)
+++ tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_6_1.java Thu Jun
18 09:43:06 2015
@@ -32,8 +32,8 @@ public class TestHttp2Section_6_1 extend
public void testDataFrame() throws Exception {
http2Connect();
- sendSimplePostRequest(3);
- readSimplePostResponse();
+ sendSimplePostRequest(3, null);
+ readSimplePostResponse(false);
Assert.assertEquals("0-WindowSize-[128]\n"
+ "3-WindowSize-[128]\n"
@@ -45,5 +45,32 @@ public class TestHttp2Section_6_1 extend
}
+ @Test
+ public void testDataFrameWithPadding() throws Exception {
+ http2Connect();
+
+ byte[] padding = new byte[8];
+
+ sendSimplePostRequest(3, padding);
+ readSimplePostResponse(true);
+
+
+ // The window update for the padding could occur anywhere since it
+ // happens on a different thead to the response.
+ String trace = output.getTrace();
+ String paddingWindowUpdate = "0-WindowSize-[9]\n3-WindowSize-[9]\n";
+
+ Assert.assertTrue(trace, trace.contains(paddingWindowUpdate));
+ trace = trace.replace(paddingWindowUpdate, "");
+
+ Assert.assertEquals("0-WindowSize-[119]\n"
+ + "3-WindowSize-[119]\n"
+ + "3-HeadersStart\n"
+ + "3-Header-[:status]-[200]\n"
+ + "3-HeadersEnd\n"
+ + "3-Body-119\n"
+ + "3-EndOfStream\n", trace);
+ }
+
// TODO: Remainder if section 6.1 tests
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
