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
