This is an automated email from the ASF dual-hosted git repository.

pjfanning pushed a commit to branch 1.4.x
in repository https://gitbox.apache.org/repos/asf/pekko-http.git


The following commit(s) were added to refs/heads/1.4.x by this push:
     new 42b04aca8 fix: drop rendered headers containing NUL as well as CR and 
LF (#1260) (#1272)
42b04aca8 is described below

commit 42b04aca812197fbc6e12c2c76a904b8e4fda276
Author: PJ Fanning <[email protected]>
AuthorDate: Sun Sep 6 08:57:19 2026 +0100

    fix: drop rendered headers containing NUL as well as CR and LF (#1260) 
(#1272)
    
    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 3deea894c..69dcf2231 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]

Reply via email to