This is an automated email from the ASF dual-hosted git repository.
markt-asf pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.1.x by this push:
new 64aede2ee0 Trigger stream reset rather than connection close where
appropriate
64aede2ee0 is described below
commit 64aede2ee0bf8cabdb3a77505dad826a76484629
Author: Mark Thomas <[email protected]>
AuthorDate: Tue May 19 08:55:58 2026 +0100
Trigger stream reset rather than connection close where appropriate
RFC 9113 requires a stream reset (protocol error) in each of these error
cases
---
java/org/apache/coyote/http2/Http2Parser.java | 13 +-
.../apache/coyote/http2/Http2UpgradeHandler.java | 4 +-
java/org/apache/coyote/http2/Stream.java | 26 ++--
java/org/apache/coyote/http2/StreamProcessor.java | 2 +-
.../apache/coyote/http2/TestHttp2Section_8_1.java | 131 +++++++++++++++++++++
webapps/docs/changelog.xml | 4 +
6 files changed, 162 insertions(+), 18 deletions(-)
diff --git a/java/org/apache/coyote/http2/Http2Parser.java
b/java/org/apache/coyote/http2/Http2Parser.java
index c1fadc27e0..c956dd52d4 100644
--- a/java/org/apache/coyote/http2/Http2Parser.java
+++ b/java/org/apache/coyote/http2/Http2Parser.java
@@ -188,7 +188,16 @@ class Http2Parser {
Integer.toString(dataLength), padding));
}
- ByteBuffer dest = output.startRequestBodyFrame(streamId, dataLength,
endOfStream);
+ ByteBuffer dest;
+ try {
+ dest = output.startRequestBodyFrame(streamId, dataLength,
endOfStream);
+ } catch (StreamException se) {
+ swallowPayload(streamId, FrameType.DATA.getId(), dataLength,
false, buffer);
+ if (Flags.hasPadding(flags)) {
+ swallowPayload(streamId, FrameType.DATA.getId(), padLength,
true, buffer);
+ }
+ throw se;
+ }
if (dest == null) {
swallowPayload(streamId, FrameType.DATA.getId(), dataLength,
false, buffer);
// Process padding before sending any notifications in case padding
@@ -828,7 +837,7 @@ class Http2Parser {
void endRequestBodyFrame(int streamId, int dataLength) throws
Http2Exception, IOException;
- void receivedEndOfStream(int streamId) throws ConnectionException;
+ void receivedEndOfStream(int streamId) throws Http2Exception;
/**
* Notification triggered when the parser swallows some or all of a
DATA frame payload without writing it to the
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index fe16ff3380..0272594482 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -1860,7 +1860,7 @@ class Http2UpgradeHandler extends AbstractStream
implements InternalHttpUpgradeH
@Override
- public void receivedEndOfStream(int streamId) throws ConnectionException {
+ public void receivedEndOfStream(int streamId) throws Http2Exception {
AbstractNonZeroStream abstractNonZeroStream =
getAbstractNonZeroStream(streamId,
connectionState.get().isNewStreamAllowed());
if (abstractNonZeroStream instanceof Stream) {
@@ -1870,7 +1870,7 @@ class Http2UpgradeHandler extends AbstractStream
implements InternalHttpUpgradeH
}
- private void receivedEndOfStream(Stream stream) throws ConnectionException
{
+ private void receivedEndOfStream(Stream stream) throws Http2Exception {
stream.receivedEndOfStream();
if (!stream.isActive()) {
decrementActiveRemoteStreamCount(stream);
diff --git a/java/org/apache/coyote/http2/Stream.java
b/java/org/apache/coyote/http2/Stream.java
index eea43c4d4d..19e306594e 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -706,22 +706,22 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
}
- final void receivedStartOfHeaders(boolean headersEndStream) throws
Http2Exception {
+ final void receivedStartOfHeaders(boolean headersEndStream) {
if (headerState == HEADER_STATE_START) {
headerState = HEADER_STATE_PSEUDO;
handler.getHpackDecoder().setMaxHeaderCount(handler.getProtocol().getMaxHeaderCount());
handler.getHpackDecoder().setMaxHeaderSize(handler.getProtocol().getMaxHeaderSize());
} else if (headerState == HEADER_STATE_PSEUDO || headerState ==
HEADER_STATE_REGULAR) {
// Trailer headers MUST include the end of stream flag
- if (headersEndStream) {
- headerState = HEADER_STATE_TRAILER;
-
handler.getHpackDecoder().setMaxHeaderCount(handler.getProtocol().getMaxTrailerCount());
-
handler.getHpackDecoder().setMaxHeaderSize(handler.getProtocol().getMaxTrailerSize());
- } else {
- throw new ConnectionException(
+ if (!headersEndStream) {
+ headerException = new StreamException(
sm.getString("stream.trailerHeader.noEndOfStream",
getConnectionId(), getIdAsString()),
- Http2Error.PROTOCOL_ERROR);
+ Http2Error.PROTOCOL_ERROR, getIdAsInt());
}
+ // Always process headers to keep HPack encoder/decoder in sync
+ headerState = HEADER_STATE_TRAILER;
+
handler.getHpackDecoder().setMaxHeaderCount(handler.getProtocol().getMaxTrailerCount());
+
handler.getHpackDecoder().setMaxHeaderSize(handler.getProtocol().getMaxTrailerSize());
}
// Parser will catch attempt to send a headers frame after the stream
// has closed.
@@ -734,20 +734,20 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
contentLengthReceived += dataLength;
long contentLengthHeader = coyoteRequest.getContentLengthLong();
if (contentLengthHeader > -1 && contentLengthReceived >
contentLengthHeader) {
- throw new ConnectionException(
+ throw new StreamException(
sm.getString("stream.header.contentLength",
getConnectionId(), getIdAsString(),
Long.valueOf(contentLengthHeader),
Long.valueOf(contentLengthReceived)),
- Http2Error.PROTOCOL_ERROR);
+ Http2Error.PROTOCOL_ERROR, getIdAsInt());
}
}
- final void receivedEndOfStream() throws ConnectionException {
+ final void receivedEndOfStream() throws Http2Exception {
if (isContentLengthInconsistent()) {
- throw new ConnectionException(
+ throw new StreamException(
sm.getString("stream.header.contentLength",
getConnectionId(), getIdAsString(),
Long.valueOf(coyoteRequest.getContentLengthLong()),
Long.valueOf(contentLengthReceived)),
- Http2Error.PROTOCOL_ERROR);
+ Http2Error.PROTOCOL_ERROR, getIdAsInt());
}
state.receivedEndOfStream();
inputBuffer.notifyEof();
diff --git a/java/org/apache/coyote/http2/StreamProcessor.java
b/java/org/apache/coyote/http2/StreamProcessor.java
index ea20748894..b272f2c6bc 100644
--- a/java/org/apache/coyote/http2/StreamProcessor.java
+++ b/java/org/apache/coyote/http2/StreamProcessor.java
@@ -314,7 +314,7 @@ class StreamProcessor extends AbstractProcessor implements
NonPipeliningProcesso
stream.getInputBuffer().insertReplayedBody(body);
try {
stream.receivedEndOfStream();
- } catch (ConnectionException ignore) {
+ } catch (Http2Exception ignore) {
// Exception will not be thrown in this case
}
}
diff --git a/test/org/apache/coyote/http2/TestHttp2Section_8_1.java
b/test/org/apache/coyote/http2/TestHttp2Section_8_1.java
index 9643411cdf..1f49c322c1 100644
--- a/test/org/apache/coyote/http2/TestHttp2Section_8_1.java
+++ b/test/org/apache/coyote/http2/TestHttp2Section_8_1.java
@@ -28,6 +28,7 @@ import org.apache.catalina.startup.Tomcat;
import org.apache.coyote.ContinueResponseTiming;
import org.apache.coyote.http11.AbstractHttp11Protocol;
import org.apache.tomcat.util.http.Method;
+import org.apache.tomcat.util.http.MimeHeaders;
/**
* Unit tests for Section 8.1 of <a
href="https://tools.ietf.org/html/rfc7540";>RFC 7540</a>. <br>
@@ -580,4 +581,134 @@ public class TestHttp2Section_8_1 extends Http2TestBase {
String trace = output.getTrace();
Assert.assertTrue(trace, trace.contains("3-RST-[1]"));
}
+
+
+ @Test
+ public void testMalformedTrailers() throws Exception {
+ http2Connect();
+
+ ((AbstractHttp11Protocol<?>) http2Protocol.getHttp11Protocol())
+ .setAllowedTrailerHeaders(TRAILER_HEADER_NAME);
+
+ // Disable overhead protection for window update as it breaks some
tests
+ http2Protocol.setOverheadWindowUpdateThreshold(0);
+
+ byte[] headersFrameHeader = new byte[9];
+ ByteBuffer headersPayload = ByteBuffer.allocate(128);
+ byte[] dataFrameHeader = new byte[9];
+ ByteBuffer dataPayload = ByteBuffer.allocate(256);
+ byte[] trailerFrameHeader = new byte[9];
+ ByteBuffer trailerPayload = ByteBuffer.allocate(256);
+
+ buildPostRequest(headersFrameHeader, headersPayload, false, null,
dataPayload.capacity(), "/simple",
+ dataFrameHeader,dataPayload, null, true, 3);
+
+ // Write the headers
+ writeFrame(headersFrameHeader, headersPayload);
+ // Body
+ writeFrame(dataFrameHeader, dataPayload);
+
+ // Trailers
+ buildTrailerHeaders(trailerFrameHeader, trailerPayload, 3);
+ // Remove the end of stream flag (0x01) but leave end of headers (0x04)
+ trailerFrameHeader[4] = 0x04;
+ writeFrame(trailerFrameHeader, trailerPayload);
+
+ // Expect 2 window updates and a reset due to a protocol error - any
order
+ parser.readFrame();
+ parser.readFrame();
+ parser.readFrame();
+ Assert.assertTrue(output.getTrace(),
output.getTrace().contains("3-RST-[1]"));
+ output.clearTrace();
+
+ // Ensure connection is still valid
+ sendSimpleGetRequest(5);
+ // Read headers
+ // In async mode, may see multiple resets for Stream 3
+ boolean skip = true;
+ while (skip) {
+ parser.readFrame();
+ if (output.getTrace().startsWith("3-RST")) {
+ output.clearTrace();
+ } else {
+ skip = false;
+ }
+ }
+ // Read body
+ parser.readFrame();
+ Assert.assertEquals(getSimpleResponseTrace(5), output.getTrace());
+ }
+
+
+ @Test
+ public void testPayloadTooSmall() throws Exception {
+ doTest1kPayload(2048);
+ }
+
+
+ @Test
+ public void testPayloadTooBig() throws Exception {
+ doTest1kPayload(512);
+ }
+
+
+ private void doTest1kPayload(int contentLength) throws Exception {
+ http2Connect();
+
+ // Set up a POST request
+ // Generate headers
+ byte[] headersFrameHeader = new byte[9];
+ ByteBuffer headersPayload = ByteBuffer.allocate(128);
+
+ MimeHeaders headers = new MimeHeaders();
+ headers.addValue(":method").setString(Method.POST);
+ headers.addValue(":scheme").setString("http");
+ headers.addValue(":path").setString("/simple");
+ headers.addValue(":authority").setString("localhost:" + getPort());
+ headers.addValue("content-length").setLong(contentLength);
+ 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);
+
+ // Generate body
+ byte[] dataFrameHeader = new byte[9];
+ ByteBuffer dataPayload = ByteBuffer.allocate(1024);
+ while (dataPayload.hasRemaining()) {
+ dataPayload.put((byte) 'x');
+ }
+ dataPayload.flip();
+
+ // Size
+ ByteUtil.setThreeBytes(dataFrameHeader, 0, dataPayload.limit());
+ // Flags - end of stream 0x01
+ dataFrameHeader[4] = 0x01;
+ // Stream ID
+ ByteUtil.set31Bits(dataFrameHeader, 5, 3);
+
+ writeFrame(dataFrameHeader, dataPayload);
+
+ // Expect 2 window updates and a reset due to a protocol error - any
order
+ parser.readFrame();
+ parser.readFrame();
+ parser.readFrame();
+ Assert.assertTrue(output.getTrace(),
output.getTrace().contains("3-RST-[1]"));
+ output.clearTrace();
+
+ // Ensure connection is still valid
+ sendSimpleGetRequest(5);
+ // Read headers
+ parser.readFrame();
+ // Read body
+ parser.readFrame();
+ Assert.assertEquals(getSimpleResponseTrace(5), output.getTrace());
+ }
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 57a7aeb5f4..7449af1f82 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -145,6 +145,10 @@
waiting for reads to time out when it is known that no more data will
be
received. (markt)
</fix>
+ <fix>
+ Ensure that malformed HTTP/2 messages that should trigger a stream
reset
+ do so, rather than triggered a connection close. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Cluster">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
