Author: markt Date: Fri Mar 24 22:10:45 2017 New Revision: 1788550 URL: http://svn.apache.org/viewvc?rev=1788550&view=rev Log: Streams that contain duplicate pseudo headers are invalid. Found with the h2spec tool written by Moto Ishizawa.
Modified: tomcat/trunk/java/org/apache/coyote/http2/HpackDecoder.java tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties tomcat/trunk/java/org/apache/coyote/http2/Stream.java tomcat/trunk/webapps/docs/changelog.xml Modified: tomcat/trunk/java/org/apache/coyote/http2/HpackDecoder.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/HpackDecoder.java?rev=1788550&r1=1788549&r2=1788550&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http2/HpackDecoder.java (original) +++ tomcat/trunk/java/org/apache/coyote/http2/HpackDecoder.java Fri Mar 24 22:10:45 2017 @@ -342,8 +342,10 @@ public class HpackDecoder { * * @param name Header name * @param value Header value + * @throws HpackException If a header is received that is not compliant + * with the HTTP/2 specification */ - void emitHeader(String name, String value); + void emitHeader(String name, String value) throws HpackException; /** * Are the headers pass to the recipient so far valid? The decoder needs @@ -384,7 +386,7 @@ public class HpackDecoder { } - private void emitHeader(String name, String value) { + private void emitHeader(String name, String value) throws HpackException { // Header names are forced to lower case if ("cookie".equals(name)) { // Only count the cookie header once since HTTP/2 splits it into 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=1788550&r1=1788549&r2=1788550&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties (original) +++ tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties Fri Mar 24 22:10:45 2017 @@ -74,6 +74,7 @@ pingManager.roundTripTime=Connection [{0 stream.closed=Connection [{0}], Stream [{1}], Unable to write to stream once it has been closed stream.header.debug=Connection [{0}], Stream [{1}], HTTP header [{2}], Value [{3}] +stream.header.duplicate=Connection [{0}], Stream [{1}], received multiple [{3}] headers stream.header.unexpectedPseudoHeader=Connection [{0}], Stream [{1}], Pseudo header [{2}] received after a regular header stream.header.unknownPseudoHeader=Connection [{0}], Stream [{1}], Unknown pseudo header [{2}] received stream.notWritable=Connection [{0}], Stream [{1}], This stream is not writable Modified: tomcat/trunk/java/org/apache/coyote/http2/Stream.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/Stream.java?rev=1788550&r1=1788549&r2=1788550&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http2/Stream.java (original) +++ tomcat/trunk/java/org/apache/coyote/http2/Stream.java Fri Mar 24 22:10:45 2017 @@ -220,7 +220,7 @@ class Stream extends AbstractStream impl @Override - public final void emitHeader(String name, String value) { + public final void emitHeader(String name, String value) throws HpackException { if (log.isDebugEnabled()) { log.debug(sm.getString("stream.header.debug", getConnectionId(), getIdentifier(), name, value)); @@ -247,34 +247,56 @@ class Stream extends AbstractStream impl switch(name) { case ":method": { - coyoteRequest.method().setString(value); + if (coyoteRequest.method().isNull()) { + coyoteRequest.method().setString(value); + } else { + throw new HpackException(sm.getString("stream.header.duplicate", + getConnectionId(), getIdentifier(), ":method" )); + } break; } case ":scheme": { - coyoteRequest.scheme().setString(value); + if (coyoteRequest.scheme().isNull()) { + coyoteRequest.scheme().setString(value); + } else { + throw new HpackException(sm.getString("stream.header.duplicate", + getConnectionId(), getIdentifier(), ":scheme" )); + } break; } case ":path": { - int queryStart = value.indexOf('?'); - if (queryStart == -1) { - coyoteRequest.requestURI().setString(value); - coyoteRequest.decodedURI().setString(coyoteRequest.getURLDecoder().convert(value, false)); + if (coyoteRequest.requestURI().isNull()) { + int queryStart = value.indexOf('?'); + if (queryStart == -1) { + coyoteRequest.requestURI().setString(value); + coyoteRequest.decodedURI().setString( + coyoteRequest.getURLDecoder().convert(value, false)); + } else { + String uri = value.substring(0, queryStart); + String query = value.substring(queryStart + 1); + coyoteRequest.requestURI().setString(uri); + coyoteRequest.decodedURI().setString( + coyoteRequest.getURLDecoder().convert(uri, false)); + coyoteRequest.queryString().setString(query); + } } else { - String uri = value.substring(0, queryStart); - String query = value.substring(queryStart + 1); - coyoteRequest.requestURI().setString(uri); - coyoteRequest.decodedURI().setString(coyoteRequest.getURLDecoder().convert(uri, false)); - coyoteRequest.queryString().setString(query); + throw new HpackException(sm.getString("stream.header.duplicate", + getConnectionId(), getIdentifier(), ":path" )); } break; } case ":authority": { - int i = value.lastIndexOf(':'); - if (i > -1) { - coyoteRequest.serverName().setString(value.substring(0, i)); - coyoteRequest.setServerPort(Integer.parseInt(value.substring(i + 1))); + if (coyoteRequest.serverName().isNull()) { + int i = value.lastIndexOf(':'); + if (i > -1) { + coyoteRequest.serverName().setString(value.substring(0, i)); + coyoteRequest.setServerPort(Integer.parseInt(value.substring(i + 1))); + } else { + coyoteRequest.serverName().setString(value); + } } else { - coyoteRequest.serverName().setString(value); + throw new HpackException(sm.getString("stream.header.duplicate", + getConnectionId(), getIdentifier(), ":authority" )); } break; } Modified: tomcat/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1788550&r1=1788549&r2=1788550&view=diff ============================================================================== --- tomcat/trunk/webapps/docs/changelog.xml (original) +++ tomcat/trunk/webapps/docs/changelog.xml Fri Mar 24 22:10:45 2017 @@ -133,6 +133,10 @@ reported by the h2spec tool written by Moto Ishizawa. (markt) </fix> <fix> + Improve HTTP/2 specification compliance by fixing some test failures + reported by the h2spec tool written by Moto Ishizawa. (markt) + </fix> + <fix> <bug>60918</bug>: Fix sendfile processing error that could lead to subsequent requests experiencing an <code>IllegalStateException</code>. (markt) --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org