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 2016/10/20 10:07:49 UTC

svn commit: r1765794 - in /tomcat/trunk: java/org/apache/coyote/http2/Constants.java java/org/apache/coyote/http2/Http2Parser.java webapps/docs/changelog.xml

Author: markt
Date: Thu Oct 20 10:07:49 2016
New Revision: 1765794

URL: http://svn.apache.org/viewvc?rev=1765794&view=rev
Log:
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=60232

The header read buffer needs to be at least the size of the largest
header.

Modified:
    tomcat/trunk/java/org/apache/coyote/http2/Constants.java
    tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java
    tomcat/trunk/webapps/docs/changelog.xml

Modified: tomcat/trunk/java/org/apache/coyote/http2/Constants.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/Constants.java?rev=1765794&r1=1765793&r2=1765794&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/Constants.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/Constants.java Thu Oct 20 10:07:49 2016
@@ -20,4 +20,7 @@ public class Constants {
 
     // Prioritisation
     public static final int DEFAULT_WEIGHT = 16;
+
+    // Parsing
+    static final int DEFAULT_HEADER_READ_BUFFER_SIZE = 1024;
 }

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=1765794&r1=1765793&r2=1765794&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java Thu Oct 20 10:07:49 2016
@@ -24,6 +24,7 @@ import org.apache.coyote.ProtocolExcepti
 import org.apache.coyote.http2.HpackDecoder.HeaderEmitter;
 import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.util.buf.ByteBufferUtils;
 import org.apache.tomcat.util.res.StringManager;
 
 class Http2Parser {
@@ -40,7 +41,8 @@ class Http2Parser {
     private final byte[] frameHeaderBuffer = new byte[9];
 
     private volatile HpackDecoder hpackDecoder;
-    private final ByteBuffer headerReadBuffer = ByteBuffer.allocate(1024);
+    private volatile ByteBuffer headerReadBuffer =
+            ByteBuffer.allocate(Constants.DEFAULT_HEADER_READ_BUFFER_SIZE);
     private volatile int headersCurrentStream = -1;
     private volatile boolean headersEndStream = false;
 
@@ -380,8 +382,24 @@ class Http2Parser {
 
     private void readHeaderPayload(int payloadSize) throws Http2Exception, IOException {
 
-        while (payloadSize > 0) {
-            int toRead = Math.min(headerReadBuffer.remaining(), payloadSize);
+        int remaining = payloadSize;
+
+        while (remaining > 0) {
+            if (headerReadBuffer.remaining() == 0) {
+                // Buffer needs expansion
+                int newSize;
+                if (headerReadBuffer.capacity() < payloadSize) {
+                    // First step, expand to the current payload. That should
+                    // cover most cases.
+                    newSize = payloadSize;
+                } else {
+                    // Header must be spread over multiple frames. Keep doubling
+                    // buffer size until the header can be read.
+                    newSize = headerReadBuffer.capacity() * 2;
+                }
+                headerReadBuffer = ByteBufferUtils.expand(headerReadBuffer, newSize);
+            }
+            int toRead = Math.min(headerReadBuffer.remaining(), remaining);
             // headerReadBuffer in write mode
             input.fill(true, headerReadBuffer, toRead);
             // switch to read mode
@@ -395,7 +413,7 @@ class Http2Parser {
             }
             // switches to write mode
             headerReadBuffer.compact();
-            payloadSize -= toRead;
+            remaining -= toRead;
         }
 
         hpackDecoder.getHeaderEmitter().validateHeaders();
@@ -416,6 +434,11 @@ class Http2Parser {
             output.receivedEndOfStream(streamId);
             headersEndStream = false;
         }
+
+        // Reset size for new request if the buffer was previously expanded
+        if (headerReadBuffer.capacity() > Constants.DEFAULT_HEADER_READ_BUFFER_SIZE) {
+            headerReadBuffer = ByteBuffer.allocate(Constants.DEFAULT_HEADER_READ_BUFFER_SIZE);
+        }
     }
 
 

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1765794&r1=1765793&r2=1765794&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Thu Oct 20 10:07:49 2016
@@ -117,6 +117,11 @@
       <add>
         Add support for trailer headers to the HTTP/2 implementation. (markt)
       </add>
+      <fix>
+        <bug>60232</bug>: When processing headers for an HTTP/2 stream, ensure
+        that the read buffer is large enough for the header being processed.
+        (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Jasper">



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org