fix: reject control characters in header names and values, harden transport header adaptation (#140)

PR: #140

Author: OmarAlJarrah

sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/common/HeaderValidation.kt

@@ -0,0 +1,136 @@
+/*
+ * Copyright (c) 2026 dexpace and Omar Aljarrah
+ *
+ * Licensed under the MIT License. See LICENSE in the project root.
+ * SPDX-License-Identifier: MIT
+ */
+
+package org.dexpace.sdk.core.http.common
+
+/**
+ * Validates an HTTP header name at the transport-agnostic model layer and returns its trimmed
+ * form. Shared by the String-keyed [Headers.Builder] API and the typed [HttpHeaderName.fromString]
+ * entry point so a malformed name cannot slip through either one — the two were previously
+ * inconsistent (only the String API validated, and only against the raw input). The trimmed name is
+ * returned so callers reuse it instead of trimming a second time.
+ *
+ * The check runs on the **trimmed** name. `String.trim()` removes only surrounding *whitespace* —
+ * the full Unicode class `Char.isWhitespace` recognises (`Character.isWhitespace ||
+ * Character.isSpaceChar`: ASCII space and tab, the C0 line/separator controls, and the Unicode
+ * space separators such as NBSP) — so leading or trailing whitespace is stripped before it could
+ * reach the wire and is harmless. A surrounding control byte that is *not* whitespace — NUL, DEL,
+ * and the other non-whitespace C0 codes — survives the trim and is rejected, exactly like an
+ * *interior* control character. What is rejected:
+ *
+ *  - **A blank name.** A field-name must be a non-empty RFC 7230 `token`; an empty or
+ *    all-whitespace name has no canonical form.
+ *  - **Any interior control character** — the C0 control range and DEL (code points `0x00`–`0x1F`
+ *    and `0x7F`), which covers CR, LF, and NUL. An embedded `\r`/`\n` is the same
+ *    request/header-splitting vector guarded against for header values: once the name is
+ *    serialised an attacker could inject a new header or a second request. A NUL or other control
+ *    character is illegal in a field-name, and the two reference transports handle it differently at
+ *    their raw API (OkHttp's `addHeader` throws unchecked, the JDK builder drops it); their adapters
+ *    now catch and drop uniformly, but a splitting vector should never get that far. Validating here
+ *    rejects it loudly at construction — fast, uniform, and transport-independent.
+ *
+ * Policy: the control-character set is intentionally narrower than RFC 7230's full `tchar`
+ * allow-list — restricting names to `tchar` would reject some non-ASCII names that certain
+ * transports accept, whereas the control-character set is illegal everywhere and covers the
+ * splitting/injection surface. This mirrors the conservative stance taken for values in
+ * [requireValidHeaderValues].
+ *
+ * @return the trimmed, validated name
+ * @throws IllegalArgumentException if the trimmed name is blank or contains a control character
+ */
+@JvmSynthetic
+internal fun requireValidHeaderName(rawName: String): String {
+    val trimmed = rawName.trim()
+    require(trimmed.isNotEmpty()) { "Header name must not be blank." }
+    trimmed.forEach { ch ->
+        require(!isProhibitedInName(ch.code)) {
+            "Header name '${escapeControlCharacters(rawName)}' must not contain control characters " +
+                "(carriage return, line feed, NUL, or other C0/DEL bytes); " +
+                "such characters enable request/header splitting."
+        }
+    }
+    return trimmed
+}
+
+/**
+ * Validates the [values] of a header [name] at the transport-agnostic model layer, applying the
+ * same control-character policy as [requireValidHeaderName] with **one deliberate exception**:
+ * horizontal tab (`0x09`) is permitted. Unlike a field-name `token`, an RFC 7230 field-value may
+ * carry HTAB as whitespace between field-content, and the two reference transports accept it (it is
+ * the one control byte OkHttp's value rule allows), so rejecting it would refuse a legitimate value.
+ *
+ * Every other C0 control (`0x00`–`0x1F`, which covers CR, LF, and NUL) and DEL (`0x7F`) is
+ * rejected. A bare CR/LF is the request/header-splitting vector — once a value is serialised an
+ * attacker could inject a new header or a second request — and the remaining control bytes are
+ * illegal in a field-value on every transport. The earlier policy here rejected only CR/LF; the
+ * broader control-character set closes the same splitting/injection surface the name check does
+ * while staying narrower than the strict field-value grammar.
+ *
+ * Non-ASCII (for example UTF-8) bytes are NOT rejected — that is the conservative stance shared
+ * with the name check: a value some transports accept is not refused at the model layer. [name]
+ * only labels the error message; the value itself is never echoed, so a secret or oversized value
+ * is not leaked into a log line.
+ *
+ * @throws IllegalArgumentException if any value contains a prohibited control character
+ */
+@JvmSynthetic
+internal fun requireValidHeaderValues(
+    name: String,
+    values: List<String>,
+) {
+    values.forEach { value ->
+        value.forEach { ch ->
+            require(!isProhibitedInValue(ch.code)) {
+                "Header value for '$name' must not contain control characters (carriage return, " +
+                    "line feed, NUL, or other C0/DEL bytes, except horizontal tab); " +
+                    "such characters enable request/header splitting."
+            }
+        }
+    }
+}
+
+/** Whether [code] is a control character prohibited in a header name — the full C0 range and DEL. */
+private fun isProhibitedInName(code: Int): Boolean = code <= LAST_C0_CONTROL || code == DEL_CONTROL
+
+/**
+ * Whether [code] is a control character prohibited in a header value — the same set as for a name,
+ * minus horizontal tab (`0x09`), which RFC 7230 permits as field-value whitespace.
+ */
+private fun isProhibitedInValue(code: Int): Boolean =
+    (code <= LAST_C0_CONTROL && code != HORIZONTAL_TAB) || code == DEL_CONTROL
+
+/**
+ * Renders [name] for an error message with every control character replaced by its `\uXXXX`
+ * escape, so a raw CR/LF/NUL from the rejected name never lands verbatim in a log line while the
+ * printable portion still identifies the offending header.
+ */
+private fun escapeControlCharacters(name: String): String =
+    buildString {
+        name.forEach { ch ->
+            if (ch.code <= LAST_C0_CONTROL || ch.code == DEL_CONTROL) {
+                append("\\u")
+                append(ch.code.toString(HEX_RADIX).padStart(ESCAPE_HEX_WIDTH, '0'))
+            } else {
+                append(ch)
+            }
+        }
+    }
+
+/** Horizontal tab (`0x09`) — the one C0 control RFC 7230 permits in a field-value (but not a name). */
+private const val HORIZONTAL_TAB: Int = 0x09
+
+/** Highest code point in the C0 control range (US, `0x1F`); everything at or below is illegal in a name. */
+private const val LAST_C0_CONTROL: Int = 0x1F
+
+/** The DEL control character (`0x7F`), the lone control code above the C0 range. */
+private const val DEL_CONTROL: Int = 0x7F
+
+/** Radix for rendering a control character's code point as the hex digits of a `\uXXXX` escape. */
+private const val HEX_RADIX: Int = 16
+
+/** Zero-padded width of a `\uXXXX` escape's hex digits. */
+private const val ESCAPE_HEX_WIDTH: Int = 4

sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/common/Headers.kt

@@ -160,8 +160,9 @@ public data class Headers private constructor(
             values: List<String>,
         ): Builder =
             apply {
-                validateValues(name, values)
-                headersMap.computeIfAbsent(sanitizeName(name)) { mutableListOf() }.addAll(values)
+                val trimmedName = requireValidHeaderName(name)
+                requireValidHeaderValues(trimmedName, values)
+                headersMap.computeIfAbsent(canonicalKey(trimmedName)) { mutableListOf() }.addAll(values)
             }
 
         /**
@@ -180,7 +181,7 @@ public data class Headers private constructor(
             values: List<String>,
         ): Builder =
             apply {
-                validateValues(name.caseInsensitiveName, values)
+                requireValidHeaderValues(name.caseInsensitiveName, values)
                 headersMap.computeIfAbsent(name.caseInsensitiveName) { mutableListOf() }.addAll(values)
             }
 
@@ -218,8 +219,9 @@ public data class Headers private constructor(
             values: List<String>,
         ): Builder =
             apply {
-                validateValues(name, values)
-                headersMap[sanitizeName(name)] = values.toMutableList()
+                val trimmedName = requireValidHeaderName(name)
+                requireValidHeaderValues(trimmedName, values)
+                headersMap[canonicalKey(trimmedName)] = values.toMutableList()
             }
 
         /**
@@ -246,7 +248,7 @@ public data class Headers private constructor(
             values: List<String>,
         ): Builder =
             apply {
-                validateValues(name.caseInsensitiveName, values)
+                requireValidHeaderValues(name.caseInsensitiveName, values)
                 headersMap[name.caseInsensitiveName] = values.toMutableList()
             }
 
@@ -304,34 +306,18 @@ public data class Headers private constructor(
         public fun builder(): Builder = Builder()
 
         /**
-         * Normalises a header name to its canonical (lower-case, trimmed) storage key.
+         * Normalises a raw, caller-supplied header name to its canonical (lower-case, trimmed)
+         * storage key. Used by the accessors and `remove`, which receive untrimmed input.
          * `Locale.US` is used deliberately — HTTP header names are ASCII-only per RFC 7230,
          * so locale-sensitive folding (Turkish `i`, etc.) would be incorrect here.
          */
         private fun sanitizeName(value: String): String = value.lowercase(Locale.US).trim()
 
         /**
-         * Rejects header values that would enable request/header splitting before they reach a
-         * transport. A bare carriage return (`\r`) or line feed (`\n`) in a value lets an
-         * attacker inject a new header or even a second request once the value is serialised;
-         * OkHttp throws unchecked on such values and the JDK transport silently drops them, so we
-         * validate here at the transport-agnostic model layer to fail fast and uniformly.
-         *
-         * Policy: reject **only** CR/LF, not OkHttp's stricter printable-ASCII-only rule. CR/LF
-         * are the splitting vector and are illegal in every transport; tightening further would
-         * reject legitimate UTF-8 values that some transports (and the JDK) accept, so the
-         * conservative CR/LF check is the right model-layer contract.
+         * Canonical storage key for a name that was already trimmed and validated by
+         * [requireValidHeaderName]. Only case-folding is needed — re-trimming (as [sanitizeName]
+         * does for raw input) would be redundant. `Locale.US` per [sanitizeName]'s rationale.
          */
-        private fun validateValues(
-            name: String,
-            values: List<String>,
-        ) {
-            values.forEach { value ->
-                require(value.indexOf('\r') < 0 && value.indexOf('\n') < 0) {
-                    "Header value for '$name' must not contain a carriage return or line feed " +
-                        "(\\r / \\n); such characters enable request/header splitting."
-                }
-            }
-        }
+        private fun canonicalKey(validatedName: String): String = validatedName.lowercase(Locale.US)
     }
 }

sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/common/HttpHeaderName.kt

@@ -24,7 +24,8 @@ import java.util.concurrent.ConcurrentHashMap
  * first caller to intern a given name "wins"; subsequent lookups with different casing
  * yield the same shared instance.
  *
- * Whitespace is trimmed from the input before interning.
+ * Whitespace is trimmed from the input before interning, and the name is validated: a blank name
+ * or one carrying an interior control character is rejected (see [fromString]).
  *
  * Designed for Java 8 bytecode compatibility — no APIs newer than Java 8 are used.
  */
@@ -216,10 +217,21 @@ public class HttpHeaderName private constructor(
          * lower-case (US locale) for the interning key. The case-preserved form of the
          * first caller to intern a given key wins; subsequent calls with different casing
          * yield the same shared instance.
+         *
+         * The name is validated up front by [requireValidHeaderName]: a blank name, or one whose
+         * trimmed form contains an interior control character (CR, LF, NUL, or any other C0/DEL
+         * byte), is rejected with an [IllegalArgumentException]. This is the same guard the
+         * String-keyed [Headers.Builder] API applies, so an interned name carried through the typed
+         * header API is guaranteed control-character-free and cannot reach a transport as a
+         * header-splitting vector.
+         *
+         * @throws IllegalArgumentException if [name] is blank or contains a control character
          */
         @JvmStatic
         public fun fromString(name: String): HttpHeaderName {
-            val trimmed = name.trim()
+            // requireValidHeaderName trims and validates, returning the trimmed form so we do not
+            // trim a second time before interning.
+            val trimmed = requireValidHeaderName(name)
             val key = trimmed.lowercase(Locale.US)
             // computeIfAbsent is available on Java 8.
             return INTERN.computeIfAbsent(key) { HttpHeaderName(trimmed, key) }