This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 9d7def063b47407a09a2f9202beed99f4dcb292a
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Jul 23 17:43:45 2020 +0100

    Move check for current streams to end of header parsing.
---
 java/org/apache/coyote/http2/Http2Parser.java      |  2 +-
 .../apache/coyote/http2/Http2UpgradeHandler.java   | 24 ++++++++++++----------
 .../apache/coyote/http2/TestHttp2Section_5_1.java  | 20 +++++++++++-------
 3 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/java/org/apache/coyote/http2/Http2Parser.java 
b/java/org/apache/coyote/http2/Http2Parser.java
index bf01b33..68dd528 100644
--- a/java/org/apache/coyote/http2/Http2Parser.java
+++ b/java/org/apache/coyote/http2/Http2Parser.java
@@ -647,7 +647,7 @@ class Http2Parser {
         HeaderEmitter headersStart(int streamId, boolean headersEndStream)
                 throws Http2Exception, IOException;
         void headersContinue(int payloadSize, boolean endOfHeaders);
-        void headersEnd(int streamId) throws ConnectionException;
+        void headersEnd(int streamId) throws Http2Exception;
 
         // Priority frames (also headers)
         void reprioritise(int streamId, int parentStreamId, boolean exclusive, 
int weight)
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java 
b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index bf05d4e..b88474f 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -1559,16 +1559,6 @@ public class Http2UpgradeHandler extends AbstractStream 
implements InternalHttpU
             stream.checkState(FrameType.HEADERS);
             stream.receivedStartOfHeaders(headersEndStream);
             closeIdleStreams(streamId);
-            if (localSettings.getMaxConcurrentStreams() < 
activeRemoteStreamCount.incrementAndGet()) {
-                
setConnectionTimeoutForStreamCount(activeRemoteStreamCount.decrementAndGet());
-                // Ignoring maxConcurrentStreams increases the overhead count
-                increaseOverheadCount();
-                throw new 
StreamException(sm.getString("upgradeHandler.tooManyRemoteStreams",
-                        
Long.toString(localSettings.getMaxConcurrentStreams())),
-                        Http2Error.REFUSED_STREAM, streamId);
-            }
-            // Valid new stream reduces the overhead count
-            reduceOverheadCount();
             return stream;
         } else {
             if (log.isDebugEnabled()) {
@@ -1636,12 +1626,24 @@ public class Http2UpgradeHandler extends AbstractStream 
implements InternalHttpU
 
 
     @Override
-    public void headersEnd(int streamId) throws ConnectionException {
+    public void headersEnd(int streamId) throws Http2Exception {
         Stream stream = getStream(streamId, 
connectionState.get().isNewStreamAllowed());
         if (stream != null) {
             setMaxProcessedStream(streamId);
             if (stream.isActive()) {
                 if (stream.receivedEndOfHeaders()) {
+
+                    if (localSettings.getMaxConcurrentStreams() < 
activeRemoteStreamCount.incrementAndGet()) {
+                        
setConnectionTimeoutForStreamCount(activeRemoteStreamCount.decrementAndGet());
+                        // Ignoring maxConcurrentStreams increases the 
overhead count
+                        increaseOverheadCount();
+                        throw new 
StreamException(sm.getString("upgradeHandler.tooManyRemoteStreams",
+                                
Long.toString(localSettings.getMaxConcurrentStreams())),
+                                Http2Error.REFUSED_STREAM, streamId);
+                    }
+                    // Valid new stream reduces the overhead count
+                    reduceOverheadCount();
+
                     processStreamOnContainerThread(stream);
                 }
             }
diff --git a/test/org/apache/coyote/http2/TestHttp2Section_5_1.java 
b/test/org/apache/coyote/http2/TestHttp2Section_5_1.java
index e9433b7..ad575ca 100644
--- a/test/org/apache/coyote/http2/TestHttp2Section_5_1.java
+++ b/test/org/apache/coyote/http2/TestHttp2Section_5_1.java
@@ -227,11 +227,11 @@ public class TestHttp2Section_5_1 extends Http2TestBase {
         // Expecting
         // 1 * headers
         // 56k-1 of body (7 * ~8k)
-        // 1 * error (could be in any order)
-        for (int i = 0; i < 8; i++) {
+        // 1 * error
+        // for a total of 9 frames (could be in any order)
+        for (int i = 0; i < 9; i++) {
             parser.readFrame(true);
         }
-        parser.readFrame(true);
 
         Assert.assertTrue(output.getTrace(),
                 output.getTrace().contains("5-RST-[" +
@@ -243,14 +243,20 @@ public class TestHttp2Section_5_1 extends Http2TestBase {
 
         // Release the remaining body
         sendWindowUpdate(0, (1 << 31) - 2);
-        // Allow for the 8k still in the stream window
+        // Allow for the ~8k still in the stream window
         sendWindowUpdate(3, (1 << 31) - 8193);
 
-        // 192k of body (24 * 8k)
-        // 1 * error (could be in any order)
-        for (int i = 0; i < 24; i++) {
+        // Read until the end of stream 3
+        while (!output.getTrace().contains("3-EndOfStream")) {
             parser.readFrame(true);
         }
+        output.clearTrace();
+
+        // Confirm another request can be sent once concurrency falls back 
below limit
+        sendSimpleGetRequest(7);
+        parser.readFrame(true);
+        parser.readFrame(true);
+        Assert.assertEquals(getSimpleResponseTrace(7), output.getTrace());
     }
 
 


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

Reply via email to