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 9a138bad2 Drop chunk extensions containing NUL, not only CR or LF
(#1344)
9a138bad2 is described below
commit 9a138bad2b2cda9e03a92311509d73eeeb61055b
Author: PJ Fanning <[email protected]>
AuthorDate: Mon Oct 12 22:09:52 2026 +0100
Drop chunk extensions containing NUL, not only CR or LF (#1344)
* Drop chunk extensions containing NUL, not only CR or LF
Motivation:
renderChunk writes a chunk extension raw into the chunk-size line and
drops it if it contains CR or LF, but not NUL. Since #1260 the header
and trailer renderers drop all three via Rendering.isIllegalHeaderChar,
so a NUL in an application-supplied chunk extension was the one place
it still reached the wire. Review of the threat model in #1295 found
the gap: P2 had to be narrowed to exclude chunk extensions from its
NUL claim.
Modification:
Check the extension with Rendering.isIllegalHeaderChar, the predicate
the header and trailer paths use. Add a ResponseRendererSpec case
beside the existing CRLF one.
Result:
A chunk extension containing CR, LF or NUL is dropped on both the
server response and client request paths, which share renderChunk.
Tests:
- sbt "http-core / Test / testOnly ...ResponseRendererSpec -- -z NUL":
failed before the fix (rendered "7;ok\0bad")
- sbt "http-core / Test / testOnly ...ResponseRendererSpec
...RequestRendererSpec": 63 passed
- scalafmt --mode diff-ref=upstream/main: no changes
References:
Refs #1256, #1260, #1295
* Explain why NUL is dropped from chunk extensions and reword test comment
---
.../http/impl/engine/rendering/RenderSupport.scala | 7 ++++---
.../engine/rendering/ResponseRendererSpec.scala | 21 +++++++++++++++++++++
2 files changed, 25 insertions(+), 3 deletions(-)
diff --git
a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/rendering/RenderSupport.scala
b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/rendering/RenderSupport.scala
index cfd7c6e49..51116b89a 100644
---
a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/rendering/RenderSupport.scala
+++
b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/rendering/RenderSupport.scala
@@ -151,9 +151,10 @@ private[http] object RenderSupport {
2 + 2
val r = new ByteStringRendering(renderedSize)
r ~~% data.length
- // drop an extension carrying CR/LF: it is rendered raw into the
chunk-size line, so a CR/LF would break the chunk
- // framing / split the response. The extension is optional metadata, so
omitting an illegal one is safe
- if (extension.nonEmpty && extension.indexOf('\r') < 0 &&
extension.indexOf('\n') < 0) r ~~ ';' ~~ extension
+ // drop an extension carrying CR, LF or NUL: it is rendered raw into the
chunk-size line, so a CR/LF would break the
+ // chunk framing / split the response, and a NUL is not a legal character
there and can truncate the line in a
+ // consumer that treats it as a C string. The extension is optional
metadata, so omitting an illegal one is safe
+ if (extension.nonEmpty && !extension.exists(ch =>
Rendering.isIllegalHeaderChar(ch))) r ~~ ';' ~~ extension
r ~~ CrLf
chunk match {
case HttpEntity.Chunk(data, _) => r ~~ data
diff --git
a/http-core/src/test/scala/org/apache/pekko/http/impl/engine/rendering/ResponseRendererSpec.scala
b/http-core/src/test/scala/org/apache/pekko/http/impl/engine/rendering/ResponseRendererSpec.scala
index 06df8cab6..dcece8ed2 100644
---
a/http-core/src/test/scala/org/apache/pekko/http/impl/engine/rendering/ResponseRendererSpec.scala
+++
b/http-core/src/test/scala/org/apache/pekko/http/impl/engine/rendering/ResponseRendererSpec.scala
@@ -445,6 +445,27 @@ class ResponseRendererSpec extends AnyFreeSpec with
Matchers with BeforeAndAfter
}
}
+ "dropping a chunk extension whose value contains NUL" in new TestSetup()
{
+ // NUL is as illegal in the chunk-size line as CR/LF, consistent with
how trailers and headers are rendered
+ HttpResponse(entity = Chunked(
+ ContentTypes.`text/plain(UTF-8)`,
+ source(
+ Chunk(ByteString("body123"), "ok\u0000bad"),
+ LastChunk))) should renderTo {
+ """HTTP/1.1 200 OK
+ |Server: pekko-http/1.0.0
+ |Date: Thu, 25 Aug 2011 09:10:29 GMT
+ |Transfer-Encoding: chunked
+ |Content-Type: text/plain; charset=UTF-8
+ |
+ |7
+ |body123
+ |0
+ |
+ |"""
+ }
+ }
+
"with one chunk and and extra LastChunks at the end (which should be
ignored)" in new TestSetup() {
HttpResponse(entity = Chunked(
ContentTypes.`text/plain(UTF-8)`,
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
