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/18 21:33:55 UTC
svn commit: r1686300 - in /tomcat/trunk:
java/org/apache/coyote/http2/Http2Parser.java
java/org/apache/coyote/http2/LocalStrings.properties
test/org/apache/coyote/http2/TestHttp2Section_6_1.java
Author: markt
Date: Thu Jun 18 19:33:54 2015
New Revision: 1686300
URL: http://svn.apache.org/r1686300
Log:
Implement optional padding validation
Modified:
tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java
tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties
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=1686300&r1=1686299&r2=1686300&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java Thu Jun 18 19:33:54 2015
@@ -86,7 +86,7 @@ class Http2Parser {
try {
validateFrame(expected, frameType, streamId, flags, payloadSize);
} catch (StreamException se) {
- swallow(streamId, payloadSize);
+ swallow(streamId, payloadSize, false);
throw se;
}
@@ -160,7 +160,7 @@ class Http2Parser {
ByteBuffer dest = output.getInputByteBuffer(streamId, dataLength);
if (dest == null) {
- swallow(streamId, dataLength);
+ swallow(streamId, dataLength, false);
if (endOfStream) {
output.receiveEndOfStream(streamId);
}
@@ -174,7 +174,7 @@ class Http2Parser {
}
}
if (padLength > 0) {
- swallow(streamId, padLength);
+ swallow(streamId, padLength, true);
output.swallowedPadding(streamId, padLength);
}
}
@@ -189,7 +189,7 @@ class Http2Parser {
try {
hpackDecoder.setHeaderEmitter(output.headersStart(streamId));
} catch (StreamException se) {
- swallow(streamId, payloadSize);
+ swallow(streamId, payloadSize, false);
throw se;
}
@@ -224,7 +224,7 @@ class Http2Parser {
readHeaderBlock(payloadSize, endOfHeaders);
- swallow(streamId, padLength);
+ swallow(streamId, padLength, true);
if (endOfHeaders) {
output.headersEnd(streamId);
@@ -405,12 +405,18 @@ class Http2Parser {
private void readUnknownFrame(int streamId, FrameType frameType, int flags, int payloadSize)
throws IOException {
- swallow(streamId, payloadSize);
+ try {
+ swallow(streamId, payloadSize, false);
+ } catch (ConnectionException e) {
+ // Will never happen because swallow() is called with mustBeZero set
+ // to false
+ }
output.swallowed(streamId, frameType, flags, payloadSize);
}
- private void swallow(int streamId, int len) throws IOException {
+ private void swallow(int streamId, int len, boolean mustBeZero)
+ throws IOException, ConnectionException {
if (log.isDebugEnabled()) {
log.debug(sm.getString("http2Parser.swallow.debug", connectionId,
Integer.toString(streamId), Integer.toString(len)));
@@ -423,6 +429,17 @@ class Http2Parser {
while (read < len) {
int thisTime = Math.min(buffer.length, len - read);
input.fill(true, buffer, 0, thisTime);
+ if (mustBeZero) {
+ // Validate the padding is zero since receiving non-zero padding
+ // is a strong indication of either a faulty client or a server
+ // side bug.
+ for (int i = 0; i < thisTime; i++) {
+ if (buffer[i] != 0) {
+ throw new ConnectionException(sm.getString("http2Parser.nonZeroPadding",
+ connectionId, Integer.toString(streamId)), Http2Error.PROTOCOL_ERROR);
+ }
+ }
+ }
read += thisTime;
}
}
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=1686300&r1=1686299&r2=1686300&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties Thu Jun 18 19:33:54 2015
@@ -38,6 +38,7 @@ hpackhuffman.huffmanEncodedHpackValueDid
http2Parser.headers.wrongFrameType=Connection [{0}], headers in progress for stream [{1}] but a frame of type [{2}] was received
http2Parser.headers.wrongStream=Connection [{0}], headers in progress for stream [{1}] but a frame for stream [{2}] was received
+http2Parser.nonZeroPadding=Connection [{0}], Stream [{1}], Non-zero padding received
http2Parser.payloadTooBig=The payload is [{0}] bytes long but the maximum frame size is [{1}]
http2Parser.preface.invalid=Invalid connection preface [{0}] presented
http2Parser.preface.io=Unable to read connection preface
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=1686300&r1=1686299&r2=1686300&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 19:33:54 2015
@@ -72,5 +72,25 @@ public class TestHttp2Section_6_1 extend
+ "3-EndOfStream\n", trace);
}
+
+ @Test
+ public void testDataFrameWithNonZeroPadding() throws Exception {
+ http2Connect();
+
+ byte[] padding = new byte[8];
+ padding[4] = 0x01;
+
+ sendSimplePostRequest(3, padding);
+ parser.readFrame(true);
+ // May see Window updates depending on timing
+ while (output.getTrace().contains("WindowSize")) {
+ output.clearTrace();
+ parser.readFrame(true);
+ }
+
+ String trace = output.getTrace();
+ Assert.assertTrue(trace, trace.startsWith("0-Goaway-[3]-[1]-["));
+ }
+
// TODO: Remainder if section 6.1 tests
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org