From 0eeea4b0005432bd985aee2245e065ae97f00cb7 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Mon, 31 Aug 2026 16:55:23 +0100 Subject: [PATCH] fix: drop rendered headers containing NUL as well as CR and LF 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 --- .../apache/pekko/http/impl/util/Rendering.scala | 15 +++++++++++---- .../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 0547fbd239..e40b448b17 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 94312d5ccc..2af2c09016 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 "" + } } } }