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 2020/08/17 18:49:34 UTC
[tomcat] 01/02: Move check for current streams to end of header
parsing.
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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org