This is an automated email from the ASF dual-hosted git repository.
markt-asf pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new cb6d0c8710 Register the use of an HTTP/2 stream identifier earlier.
cb6d0c8710 is described below
commit cb6d0c871004a80a5228c1aa8def73394aabbd85
Author: Mark Thomas <[email protected]>
AuthorDate: Mon Jul 27 14:18:47 2026 +0100
Register the use of an HTTP/2 stream identifier earlier.
This ensures there is no possibility of a re-used stream identifier
being accepted, regardless of how early in the HEADERS frame processing
an error is detected.
---
.../apache/coyote/http2/Http2UpgradeHandler.java | 9 +----
test/org/apache/coyote/http2/TestHttp2Limits.java | 2 +-
.../apache/coyote/http2/TestHttp2Section_4_2.java | 2 +-
.../apache/coyote/http2/TestHttp2Section_4_3.java | 4 +-
.../apache/coyote/http2/TestHttp2Section_5_1.java | 45 ++++++++++++++++++++++
.../apache/coyote/http2/TestHttp2Section_5_5.java | 2 +-
.../apache/coyote/http2/TestHttp2Section_6_2.java | 8 ++--
.../apache/coyote/http2/TestHttp2Section_6_3.java | 2 +-
webapps/docs/changelog.xml | 6 +++
9 files changed, 61 insertions(+), 19 deletions(-)
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index d8b669b035..4bd58bc289 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -1699,6 +1699,7 @@ class Http2UpgradeHandler extends AbstractStream
implements InternalHttpUpgradeH
if (streamId > maxProcessedStreamId) {
stream = createRemoteStream(streamId);
activeRemoteStreamCount.incrementAndGet();
+ maxProcessedStreamId = streamId;
} else {
// ID for new stream must always be greater than any
previous stream
throw new
ConnectionException(sm.getString("upgradeHandler.stream.old",
Integer.valueOf(streamId),
@@ -1745,7 +1746,6 @@ class Http2UpgradeHandler extends AbstractStream
implements InternalHttpUpgradeH
getAbstractNonZeroStream(streamId,
connectionState.get().isNewStreamAllowed());
if (abstractNonZeroStream instanceof Stream stream) {
boolean processStream = false;
- setMaxProcessedStream(streamId);
if (stream.isActive()) {
if (stream.receivedEndOfHeaders()) {
if (localSettings.getMaxConcurrentStreams() <
activeRemoteStreamCount.get()) {
@@ -1797,13 +1797,6 @@ class Http2UpgradeHandler extends AbstractStream
implements InternalHttpUpgradeH
}
- private void setMaxProcessedStream(int streamId) {
- if (maxProcessedStreamId < streamId) {
- maxProcessedStreamId = streamId;
- }
- }
-
-
@Override
public void reset(int streamId, long errorCode) throws Http2Exception {
if (log.isTraceEnabled()) {
diff --git a/test/org/apache/coyote/http2/TestHttp2Limits.java
b/test/org/apache/coyote/http2/TestHttp2Limits.java
index c1281e85ac..697cbe813f 100644
--- a/test/org/apache/coyote/http2/TestHttp2Limits.java
+++ b/test/org/apache/coyote/http2/TestHttp2Limits.java
@@ -312,7 +312,7 @@ public class TestHttp2Limits extends Http2TestBase {
try {
parser.readFrame();
MatcherAssert.assertThat(output.getTrace(),
-
RegexMatcher.matchesRegex("0-Goaway-\\[1\\]-\\[11\\]-\\[" + limitMessage +
"\\]"));
+
RegexMatcher.matchesRegex("0-Goaway-\\[3\\]-\\[11\\]-\\[" + limitMessage +
"\\]"));
} catch (IOException ignore) {
// Expected on some platforms
}
diff --git a/test/org/apache/coyote/http2/TestHttp2Section_4_2.java
b/test/org/apache/coyote/http2/TestHttp2Section_4_2.java
index 3745fbe07e..b20d441ec6 100644
--- a/test/org/apache/coyote/http2/TestHttp2Section_4_2.java
+++ b/test/org/apache/coyote/http2/TestHttp2Section_4_2.java
@@ -124,7 +124,7 @@ public class TestHttp2Section_4_2 extends Http2TestBase {
os.write(headers);
os.flush();
- handleGoAwayResponse(1, Http2Error.FRAME_SIZE_ERROR);
+ handleGoAwayResponse(3, Http2Error.FRAME_SIZE_ERROR);
}
diff --git a/test/org/apache/coyote/http2/TestHttp2Section_4_3.java
b/test/org/apache/coyote/http2/TestHttp2Section_4_3.java
index 772c26f2e7..0fbcfa5c35 100644
--- a/test/org/apache/coyote/http2/TestHttp2Section_4_3.java
+++ b/test/org/apache/coyote/http2/TestHttp2Section_4_3.java
@@ -43,7 +43,7 @@ public class TestHttp2Section_4_3 extends Http2TestBase {
// Process the request
writeFrame(frameHeader, headersPayload);
- handleGoAwayResponse(1, Http2Error.COMPRESSION_ERROR);
+ handleGoAwayResponse(3, Http2Error.COMPRESSION_ERROR);
}
@@ -84,6 +84,6 @@ public class TestHttp2Section_4_3 extends Http2TestBase {
sendPing();
- handleGoAwayResponse(1, Http2Error.COMPRESSION_ERROR);
+ handleGoAwayResponse(3, Http2Error.COMPRESSION_ERROR);
}
}
diff --git a/test/org/apache/coyote/http2/TestHttp2Section_5_1.java
b/test/org/apache/coyote/http2/TestHttp2Section_5_1.java
index d72fb03efe..740f98b48e 100644
--- a/test/org/apache/coyote/http2/TestHttp2Section_5_1.java
+++ b/test/org/apache/coyote/http2/TestHttp2Section_5_1.java
@@ -23,6 +23,9 @@ import java.util.logging.Logger;
import org.junit.Assert;
import org.junit.Test;
+import org.apache.tomcat.util.http.Method;
+import org.apache.tomcat.util.http.MimeHeaders;
+
/**
* Unit tests for Section 5.ยง of <a
href="https://tools.ietf.org/html/rfc7540";>RFC 7540</a>. <br>
* The order of tests in this class is aligned with the order of the
requirements in the RFC.
@@ -430,4 +433,46 @@ public class TestHttp2Section_5_1 extends Http2TestBase {
Assert.assertTrue(output.getTrace(),
output.getTrace().contains("0-Goaway-[5]-[" +
Http2Error.FLOW_CONTROL_ERROR.getCode() + "]"));
}
+
+
+ @Test
+ public void testReuseAfterReset() throws Exception {
+ http2Connect();
+
+ // Create an invalid request
+ // Generate headers
+ byte[] headersFrameHeader = new byte[9];
+ ByteBuffer headersPayload = ByteBuffer.allocate(128);
+
+ MimeHeaders headers = new MimeHeaders();
+ headers.addValue(":method").setString(Method.GET);
+ headers.addValue(":scheme").setString("http");
+ headers.addValue(":path").setString("/simple");
+ headers.addValue(":authority").setString("localhost:" + getPort());
+ headers.addValue("Connection").setString("close");
+
+ hpackEncoder.encode(headers, headersPayload);
+
+ headersPayload.flip();
+
+ ByteUtil.setThreeBytes(headersFrameHeader, 0, headersPayload.limit());
+ headersFrameHeader[3] = FrameType.HEADERS.getIdByte();
+ // Flags. end of headers (0x04)
+ headersFrameHeader[4] = 0x04;
+ // Stream id
+ ByteUtil.set31Bits(headersFrameHeader, 5, 3);
+
+ writeFrame(headersFrameHeader, headersPayload);
+
+ parser.readFrame();
+ Assert.assertEquals("Reset expected due to invalid header",
"3-RST-[1]\n", output.getTrace());
+ output.clearTrace();
+
+ // Re-use stream 3 for a valid request
+ sendSimpleGetRequest(3);
+ parser.readFrame();
+ String trace = output.getTrace();
+ Assert.assertTrue("Goaway expected due to stream reuse [" + trace +
"]",
+ trace.startsWith("0-Goaway-[3]-[1]-["));
+ }
}
diff --git a/test/org/apache/coyote/http2/TestHttp2Section_5_5.java
b/test/org/apache/coyote/http2/TestHttp2Section_5_5.java
index c383a2e631..c202eecb0f 100644
--- a/test/org/apache/coyote/http2/TestHttp2Section_5_5.java
+++ b/test/org/apache/coyote/http2/TestHttp2Section_5_5.java
@@ -88,6 +88,6 @@ public class TestHttp2Section_5_5 extends Http2TestBase {
os.write(UNKNOWN_FRAME);
os.flush();
- handleGoAwayResponse(1, Http2Error.COMPRESSION_ERROR);
+ handleGoAwayResponse(3, Http2Error.COMPRESSION_ERROR);
}
}
diff --git a/test/org/apache/coyote/http2/TestHttp2Section_6_2.java
b/test/org/apache/coyote/http2/TestHttp2Section_6_2.java
index 107919c0e1..de9b90415f 100644
--- a/test/org/apache/coyote/http2/TestHttp2Section_6_2.java
+++ b/test/org/apache/coyote/http2/TestHttp2Section_6_2.java
@@ -63,7 +63,7 @@ public class TestHttp2Section_6_2 extends Http2TestBase {
sendSimpleGetRequest(3, padding);
- handleGoAwayResponse(1);
+ handleGoAwayResponse(3);
}
@@ -87,7 +87,7 @@ public class TestHttp2Section_6_2 extends Http2TestBase {
os.write(headerFrame);
os.flush();
- handleGoAwayResponse(1);
+ handleGoAwayResponse(3);
}
@@ -121,9 +121,7 @@ public class TestHttp2Section_6_2 extends Http2TestBase {
os.write(headerFrame);
os.flush();
- // 1 is the last stream processed before the connection error (stream 1
- // from the initial HTTP/1.1 upgrade)
- handleGoAwayResponse(1);
+ handleGoAwayResponse(3);
}
diff --git a/test/org/apache/coyote/http2/TestHttp2Section_6_3.java
b/test/org/apache/coyote/http2/TestHttp2Section_6_3.java
index 1115f52457..5432daa27e 100644
--- a/test/org/apache/coyote/http2/TestHttp2Section_6_3.java
+++ b/test/org/apache/coyote/http2/TestHttp2Section_6_3.java
@@ -51,7 +51,7 @@ public class TestHttp2Section_6_3 extends Http2TestBase {
sendPriority(5, 3, 15);
- handleGoAwayResponse(1, Http2Error.COMPRESSION_ERROR);
+ handleGoAwayResponse(3, Http2Error.COMPRESSION_ERROR);
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index c8752e414c..59ef41b3ef 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -327,6 +327,12 @@
<code>:authority</code> pseudo header or a <code>Host</code> header).
(markt)
</fix>
+ <fix>
+ Register the use of an HTTP/2 stream identifier earlier so that there
is
+ no possibility of a re-used stream identifier being accepted,
regardless
+ of how early in the HEADERS frame processing an error is detected.
+ (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Jasper">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
