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