Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ public data class Headers private constructor(
values: List<String>,
): Builder =
apply {
validateName(name)
validateValues(name, values)
headersMap.computeIfAbsent(sanitizeName(name)) { mutableListOf() }.addAll(values)
}
Expand Down Expand Up @@ -218,6 +219,7 @@ public data class Headers private constructor(
values: List<String>,
): Builder =
apply {
validateName(name)
validateValues(name, values)
headersMap[sanitizeName(name)] = values.toMutableList()
}
Expand Down Expand Up @@ -333,5 +335,43 @@ public data class Headers private constructor(
}
}
}

/**
* Rejects header names that cannot legally appear on the wire before they reach a transport.
*
* An embedded carriage return (`\r`) or line feed (`\n`) in a name is the same
* request/header-splitting vector guarded against for values: once the name is serialised an
* attacker could inject a new header or a second request. A NUL or any other ASCII control
* character is likewise illegal in an RFC 7230 field-name (`token`) and is rejected by — or
* silently dropped at — the transport layer, so the two reference transports diverge (OkHttp
* throws unchecked, the JDK transport drops the header) when such a name slips through.
* Validating here at the transport-agnostic model layer fails fast and uniformly.
*
* Note [sanitizeName] trims surrounding whitespace and lower-cases, but it never removes an
* *interior* control character, so this check is the only thing standing between a malformed
* name and the transport. A blank name has no canonical form and is rejected as well.
*
* Policy: reject the C0 control range and DEL (code points `0x00`-`0x1F` and `0x7F`), which
* covers CR, LF, and NUL. This is intentionally narrower than RFC 7230's full `tchar`
* allow-list — restricting names to `tchar` only would reject some non-ASCII names that
* certain transports accept, whereas the control-character set is illegal everywhere and
* covers the splitting/injection surface.
*/
private fun validateName(name: String) {
require(name.isNotBlank()) { "Header name must not be blank." }
name.forEach { ch ->
require(ch.code > LAST_C0_CONTROL && ch.code != DEL_CONTROL) {
"Header name '$name' must not contain control characters " +
"(carriage return, line feed, NUL, or other C0/DEL bytes); " +
"such characters enable request/header splitting."
}
}
}

/** Highest code point in the C0 control range (US, `0x1F`); everything at or below is illegal. */
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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,103 @@ class HeadersTest {
assertNull(headers.get("X-Trace-Id"))
}

// ---- name validation (request/header-splitting guard) -----------------------

@Test
fun `add rejects a name containing a line feed`() {
assertFailsWith<IllegalArgumentException> {
Headers.builder().add("X-Evil\nInjected", "v")
}
}

@Test
fun `add rejects a name containing a carriage return`() {
assertFailsWith<IllegalArgumentException> {
Headers.builder().add("X-Evil\rInjected", "v")
}
}

@Test
fun `add rejects a name containing CRLF`() {
assertFailsWith<IllegalArgumentException> {
Headers.builder().add("X-Evil\r\nInjected: 1", "v")
}
}

@Test
fun `add rejects a name containing a NUL`() {
assertFailsWith<IllegalArgumentException> {
Headers.builder().add("X-Evil\u0000Injected", "v")
}
}

@Test
fun `add rejects a name containing a DEL control character`() {
assertFailsWith<IllegalArgumentException> {
Headers.builder().add("X-Evil\u007FInjected", "v")
}
}

@Test
fun `add list overload rejects a name containing CR or LF`() {
assertFailsWith<IllegalArgumentException> {
Headers.builder().add("X-Evil\nInjected", listOf("v"))
}
}

@Test
fun `set rejects a name containing a line feed`() {
assertFailsWith<IllegalArgumentException> {
Headers.builder().set("X-Evil\nInjected", "v")
}
}

@Test
fun `set rejects a name containing a carriage return`() {
assertFailsWith<IllegalArgumentException> {
Headers.builder().set("X-Evil\rInjected", "v")
}
}

@Test
fun `set list overload rejects a name containing a NUL`() {
assertFailsWith<IllegalArgumentException> {
Headers.builder().set("X-Evil\u0000Injected", listOf("v"))
}
}

@Test
fun `add rejects a blank name`() {
assertFailsWith<IllegalArgumentException> {
Headers.builder().add(" ", "v")
}
}

@Test
fun `the name rejection message names the offending header`() {
val thrown =
assertFailsWith<IllegalArgumentException> {
Headers.builder().add("X-Trace-Id\nInjected", "v")
}
assertTrue(
thrown.message?.lowercase()?.contains("x-trace-id") == true,
"message should name the header, got: ${thrown.message}",
)
}

@Test
fun `normal names without control characters are accepted`() {
val headers =
Headers.builder()
.add("X-Plain", "a")
// Surrounding whitespace is trimmed by name normalisation, not rejected.
.set(" Authorization ", "Bearer t")
.build()

assertEquals("a", headers.get("X-Plain"))
assertEquals("Bearer t", headers.get("Authorization"))
}

// ---- accessors & equality coverage ------------------------------------------

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,11 @@ internal class RequestAdapter(
* `IllegalArgumentException` escape [adapt] (and therefore `execute`, declared
* `@Throws(IOException)`) where a caller's `catch(IOException)` would not observe it.
*
* Note this catch guards against the JDK's restricted *name* set only. Illegal header
* *values* (CR/LF and similar) are now rejected upstream by `Headers.Builder`, so a value
* with control characters never reaches this point — the `IllegalArgumentException` handled
* here is the JDK refusing a restricted name, not a malformed value.
* Note this catch guards against the JDK's restricted *name* set only. Malformed header names
* (CR/LF, NUL, other control characters) and illegal header values (CR/LF) are rejected
* upstream by `Headers.Builder`, so neither a control-character name nor such a value reaches
* this point — the `IllegalArgumentException` handled here is the JDK refusing a *restricted*
* (but otherwise well-formed) name, not a malformed name or value.
*/
private fun attachHeaders(
builder: HttpRequest.Builder,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,10 @@ internal class RequestAdapter(
continue
}
for (value in values) {
// Header names/values are validated upstream by Headers.Builder (CR/LF and other
// illegal characters are rejected at construction), so addHeader receives only
// well-formed values here.
// Header names and values are validated upstream by Headers.Builder: names reject
// control characters (CR/LF, NUL, the rest of the C0/DEL range) and values reject
// CR/LF, so addHeader receives only well-formed input here and never throws on a
// malformed name or value.
builder.addHeader(rawName, value)
}
}
Expand Down
Loading