This is an automated email from the ASF dual-hosted git repository.
pjfanning pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/pekko-http.git
The following commit(s) were added to refs/heads/main by this push:
new def9e1b80 fix: ignore the reserved bit of an incoming HTTP/2 stream
identifier (#1261)
def9e1b80 is described below
commit def9e1b80c0e04fd3ce69aa95e3e987c627d022a
Author: PJ Fanning <[email protected]>
AuthorDate: Sun Sep 6 12:42:20 2026 +0100
fix: ignore the reserved bit of an incoming HTTP/2 stream identifier (#1261)
Motivation:
`Http2FrameParsing` read the stream identifier with a plain
`readIntBE()`, keeping the reserved high bit, and carried a TODO saying
the bit should be checked. RFC 9113 5.1.1 says the bit is reserved and
MUST be ignored when receiving. Keeping it means a peer that sets it
produces a negative stream id, which never matches the stream the frame
is really for: `streamFor` treats it as closed and the connection is
failed with a GOAWAY instead of the frame being handled on its stream.
Modification:
Mask the reserved bit off (`& 0x7FFFFFFF`) when reading the identifier
and replace the TODO with the rule it was standing in for.
Result:
A frame whose stream identifier carries the reserved bit is handled on
the stream it names, as the spec requires, rather than failing the
connection.
Tests:
- sbt "http2-tests/testOnly
org.apache.pekko.http.impl.engine.http2.Http2ServerSpec
org.apache.pekko.http.impl.engine.http2.Http2ClientSpec
org.apache.pekko.http.impl.engine.http2.framing.Http2FramingSpec" - pass (189
tests); a new test sends HEADERS for stream 1 with the reserved bit set and
expects the request to be dispatched. Verified it fails with the fix stashed
(the connection is failed instead).
- sbt http-core/mimaReportBinaryIssues - pass (internal impl.engine.http2
change, no public API).
References:
None - ignores the reserved stream identifier bit per RFC 9113 5.1.1
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
---
.../pekko/http/impl/engine/http2/framing/Http2FrameParsing.scala | 6 ++++--
.../org/apache/pekko/http/impl/engine/http2/Http2ServerSpec.scala | 8 ++++++++
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git
a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/framing/Http2FrameParsing.scala
b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/framing/Http2FrameParsing.scala
index fc3d4cac9..bc263f348 100644
---
a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/framing/Http2FrameParsing.scala
+++
b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/framing/Http2FrameParsing.scala
@@ -193,8 +193,10 @@ private[http2] class Http2FrameParsing(
val length = reader.readShortBE() << 8 | reader.readByte()
val tpe = reader.readByte()
val flags = new ByteFlag(reader.readByte())
- val streamId = reader.readIntBE()
- // TODO: assert that reserved bit is 0 by checking if streamId > 0
+ // RFC 9113 5.1.1: the high bit of the stream identifier is reserved
and MUST be ignored when receiving.
+ // Without masking it a peer that sets it yields a negative stream
id, which never matches the stream the
+ // frame is really for and fails the connection instead.
+ val streamId = reader.readIntBE() & 0x7FFFFFFF
val payload = reader.take(length)
val maybeframe = FrameType.byId(tpe) match {
case OptionVal.Some(ft) =>
diff --git
a/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/Http2ServerSpec.scala
b/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/Http2ServerSpec.scala
index c90661b85..ffd415924 100644
---
a/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/Http2ServerSpec.scala
+++
b/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/Http2ServerSpec.scala
@@ -316,6 +316,14 @@ class Http2ServerSpec extends Http2SpecWithMaterializer("""
trailingResponseHeaders.size should be(1)
trailingResponseHeaders.head should be(("Status", "grpc-status 10"))
})
+ "ignore the reserved bit of a stream
identifier".inAssertAllStagesStopped(
+ new TestSetup with RequestResponseProbes {
+ // RFC 9113 5.1.1: the high bit of the stream identifier is reserved
and must be ignored when receiving,
+ // so this is a frame for stream 1 rather than one for an unusable
negative stream id
+ network.sendHEADERS(1 | 0x80000000, endStream = true,
network.headersForRequest(Get("/")))
+
+ user.expectRequest()
+ })
"drop a response header whose value contains
CRLF".inAssertAllStagesStopped(
new TestSetup with RequestResponseProbes {
val streamId = 1
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
