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 6cd9e7b80 fix: drop rendered headers containing NUL as well as CR and
LF (#1260)
6cd9e7b80 is described below
commit 6cd9e7b80dd3c8eb23deeb49e36b2b9c0f82a912
Author: PJ Fanning <[email protected]>
AuthorDate: Wed Sep 2 09:22:53 2026 +0100
fix: drop rendered headers containing NUL as well as CR and LF (#1260)
Motivation:
The guard in `Rendering.~~(HttpHeader)` renders a header and then scans
the rendered bytes, discarding the header if it finds one of the
characters that must never reach the wire. It only looked for CR and
LF, so a `RawHeader` value carrying a NUL was rendered as-is. NUL is
not a legal field-value character, and a downstream consumer that
treats the value as a C string truncates it there, so two parties can
disagree about where the value ends. The HTTP/2 renderer rejects CR, LF
and NUL alike, so HTTP/1.1 was the weaker of the two.
Modification:
Add `Rendering.isIllegalHeaderChar`, which covers CR, LF and NUL, and
use it from all four `check` implementations (`StringRendering`,
`ByteArrayRendering`, `ByteStringRendering` and
`CustomCharsetByteStringRendering`) instead of repeating the character
comparison a fourth time. It takes an `Int` so the `Char` and `Byte`
based renderings can pass their element straight in.
Result:
A header whose name or value contains NUL is discarded like one
containing CR or LF, on every rendering implementation, and the rule
now lives in one place.
Tests:
- sbt "http-core/testOnly org.apache.pekko.http.impl.util.RenderingSpec
org.apache.pekko.http.impl.engine.rendering.ResponseRendererSpec
org.apache.pekko.http.impl.engine.rendering.RequestRendererSpec" - pass (80
tests); a new case in the shared rendering table asserts a header with NUL in
the value is discarded. Verified it fails with the fix stashed, once for each
of the four renderings.
- sbt http-core/mimaReportBinaryIssues - pass (internal impl.util change,
no public API).
References:
None - extends the outgoing header guard to NUL
---
.../scala/org/apache/pekko/http/impl/util/Rendering.scala | 15 +++++++++++----
.../org/apache/pekko/http/impl/util/RenderingSpec.scala | 11 +++++++++++
2 files changed, 22 insertions(+), 4 deletions(-)
diff --git
a/http-core/src/main/scala/org/apache/pekko/http/impl/util/Rendering.scala
b/http-core/src/main/scala/org/apache/pekko/http/impl/util/Rendering.scala
index 0547fbd23..e40b448b1 100644
--- a/http-core/src/main/scala/org/apache/pekko/http/impl/util/Rendering.scala
+++ b/http-core/src/main/scala/org/apache/pekko/http/impl/util/Rendering.scala
@@ -262,6 +262,13 @@ private[http] object Rendering {
val floatFormat = new DecimalFormat("0.0##",
DecimalFormatSymbols.getInstance(Locale.ROOT))
val `\"` = CharPredicate('\\', '"')
+ /**
+ * Characters that must never reach the wire inside a rendered header: CR
and LF would split the message, and NUL
+ * is not a legal field-value character and can truncate the value in a
consumer that treats it as a C string.
+ * Takes an `Int` so that both the `Char` and the `Byte` based renderings
can pass their element in directly.
+ */
+ def isIllegalHeaderChar(ch: Int): Boolean = ch == '\r' || ch == '\n' || ch
== 0
+
// US-ASCII printable chars except for '"' and escape chars '\' and (for
faulty clients) '%'
// https://tools.ietf.org/html/rfc6266#appendix-D
val contentDispositionFilenameSafeChars = CharPredicate.Printable -- "%\"\\"
@@ -299,7 +306,7 @@ private[http] class StringRendering extends Rendering {
@tailrec def rec(mark: Int): Boolean =
if (mark < sb.length()) {
val ch = sb.charAt(mark)
- if (ch == '\r' || ch == '\n') {
+ if (Rendering.isIllegalHeaderChar(ch)) {
sb.delete(origMark, sb.length())
false
} else rec(mark + 1)
@@ -369,7 +376,7 @@ private[http] class ByteArrayRendering(sizeHint: Int,
logDiscardedHeader: String
@tailrec def rec(mark: Int): Boolean =
if (mark < size) {
- if (array(mark) == '\r' || array(mark) == '\n') {
+ if (Rendering.isIllegalHeaderChar(array(mark))) {
logDiscardedHeader("Invalid outgoing header was discarded. " +
LogByteStringTools.printByteString(
ByteString.fromArray(array, origMark, size - origMark)))
size = origMark
@@ -416,7 +423,7 @@ private[http] class ByteStringRendering(sizeHint: Int,
logDiscardedHeader: Strin
@tailrec def rec(mark: Int): Boolean =
if (mark < builder.length) {
val ch = contents(mark)
- if (ch == '\r' || ch == '\n') {
+ if (Rendering.isIllegalHeaderChar(ch)) {
logDiscardedHeader(
"Invalid outgoing header was discarded. " +
LogByteStringTools.printByteString(contents.drop(origMark)))
builder.clear()
@@ -491,7 +498,7 @@ private[http] class
CustomCharsetByteStringRendering(nioCharset: Charset, sizeHi
@tailrec def rec(mark: Int): Boolean =
if (mark < builder.length) {
val ch = contents(mark)
- if (ch == '\r' || ch == '\n') {
+ if (Rendering.isIllegalHeaderChar(ch)) {
builder.clear()
builder.append(contents.take(origMark))
false
diff --git
a/http-core/src/test/scala/org/apache/pekko/http/impl/util/RenderingSpec.scala
b/http-core/src/test/scala/org/apache/pekko/http/impl/util/RenderingSpec.scala
index 94312d5cc..2af2c0901 100644
---
a/http-core/src/test/scala/org/apache/pekko/http/impl/util/RenderingSpec.scala
+++
b/http-core/src/test/scala/org/apache/pekko/http/impl/util/RenderingSpec.scala
@@ -98,6 +98,17 @@ class RenderingSpec extends PekkoSpecWithMaterializer with
Matchers {
rendered shouldBe ""
}
+ // NUL is not a legal field-value character and can truncate the value
in a consumer that treats it as a
+ // C string, so it must not reach the wire either
+ "do not render header with NUL in the value" in {
+ val r = setup.create()
+ val rendered =
+ EventFilter.warning(pattern = "Invalid outgoing header was
discarded").intercept {
+ setup.result(r ~~ RawHeader("Test", "broken\u0000value"))
+ }
+
+ rendered shouldBe ""
+ }
}
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
