Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cherrypick #6551 onto 4.9.x #6740

Merged
merged 1 commit into from Jul 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 23 additions & 6 deletions okhttp/src/main/kotlin/okhttp3/Headers.kt
Expand Up @@ -28,6 +28,7 @@ import okhttp3.Headers.Builder
import okhttp3.internal.format
import okhttp3.internal.http.toHttpDateOrNull
import okhttp3.internal.http.toHttpDateString
import okhttp3.internal.isSensitiveHeader
import org.codehaus.mojo.animal_sniffer.IgnoreJRERequirement

/**
Expand Down Expand Up @@ -180,12 +181,27 @@ class Headers private constructor(

override fun hashCode(): Int = namesAndValues.contentHashCode()

/**
* Returns header names and values. The names and values are separated by `: ` and each pair is
* followed by a newline character `\n`.
*
* Since OkHttp 5 this redacts these sensitive headers:
*
* * `Authorization`
* * `Cookie`
* * `Proxy-Authorization`
* * `Set-Cookie`
*
* To get all headers as a human-readable string use `toMultimap().toString()`.
*/
override fun toString(): String {
return buildString {
for (i in 0 until size) {
append(name(i))
val name = name(i)
val value = value(i)
append(name)
append(": ")
append(value(i))
append(if (isSensitiveHeader(name)) "██" else value)
append("\n")
}
}
Expand Down Expand Up @@ -370,7 +386,7 @@ class Headers private constructor(
}

// Check for malformed headers.
for (i in 0 until namesAndValues.size step 2) {
for (i in namesAndValues.indices step 2) {
val name = namesAndValues[i]
val value = namesAndValues[i + 1]
checkName(name)
Expand Down Expand Up @@ -420,7 +436,7 @@ class Headers private constructor(

private fun checkName(name: String) {
require(name.isNotEmpty()) { "name is empty" }
for (i in 0 until name.length) {
for (i in name.indices) {
val c = name[i]
require(c in '\u0021'..'\u007e') {
format("Unexpected char %#04x at %d in header name: %s", c.toInt(), i, name)
Expand All @@ -429,10 +445,11 @@ class Headers private constructor(
}

private fun checkValue(value: String, name: String) {
for (i in 0 until value.length) {
for (i in value.indices) {
val c = value[i]
require(c == '\t' || c in '\u0020'..'\u007e') {
format("Unexpected char %#04x at %d in %s value: %s", c.toInt(), i, name, value)
format("Unexpected char %#04x at %d in %s value", c.toInt(), i, name) +
(if (isSensitiveHeader(name)) "" else ": $value")
}
}
}
Expand Down
9 changes: 8 additions & 1 deletion okhttp/src/main/kotlin/okhttp3/internal/Util.kt
Expand Up @@ -38,7 +38,6 @@ import java.util.concurrent.ThreadFactory
import java.util.concurrent.TimeUnit
import kotlin.text.Charsets.UTF_32BE
import kotlin.text.Charsets.UTF_32LE
import okhttp3.Call
import okhttp3.EventListener
import okhttp3.Headers
import okhttp3.Headers.Companion.headersOf
Expand Down Expand Up @@ -248,6 +247,14 @@ fun String.canParseAsIpAddress(): Boolean {
return VERIFY_AS_IP_ADDRESS.matches(this)
}

/** Returns true if we should void putting this this header in an exception or toString(). */
fun isSensitiveHeader(name: String): Boolean {
return name.equals("Authorization", ignoreCase = true) ||
name.equals("Cookie", ignoreCase = true) ||
name.equals("Proxy-Authorization", ignoreCase = true) ||
name.equals("Set-Cookie", ignoreCase = true)
}

/** Returns a [Locale.US] formatted [String]. */
fun format(format: String, vararg args: Any): String {
return String.format(Locale.US, format, *args)
Expand Down
49 changes: 49 additions & 0 deletions okhttp/src/test/java/okhttp3/HeadersTest.java
Expand Up @@ -380,6 +380,37 @@ public final class HeadersTest {
}
}

@Test public void sensitiveHeadersNotIncludedInExceptions() {
try {
new Headers.Builder().add("Authorization", "valué1");
fail("Should have complained about invalid name");
} catch (IllegalArgumentException expected) {
assertThat(expected.getMessage()).isEqualTo(
"Unexpected char 0xe9 at 4 in Authorization value");
}
try {
new Headers.Builder().add("Cookie", "valué1");
fail("Should have complained about invalid name");
} catch (IllegalArgumentException expected) {
assertThat(expected.getMessage()).isEqualTo(
"Unexpected char 0xe9 at 4 in Cookie value");
}
try {
new Headers.Builder().add("Proxy-Authorization", "valué1");
fail("Should have complained about invalid name");
} catch (IllegalArgumentException expected) {
assertThat(expected.getMessage()).isEqualTo(
"Unexpected char 0xe9 at 4 in Proxy-Authorization value");
}
try {
new Headers.Builder().add("Set-Cookie", "valué1");
fail("Should have complained about invalid name");
} catch (IllegalArgumentException expected) {
assertThat(expected.getMessage()).isEqualTo(
"Unexpected char 0xe9 at 4 in Set-Cookie value");
}
}

@Test public void headersEquals() {
Headers headers1 = new Headers.Builder()
.add("Connection", "close")
Expand Down Expand Up @@ -414,6 +445,24 @@ public final class HeadersTest {
assertThat(headers.toString()).isEqualTo("A: a\nB: bb\n");
}

@Test public void headersToStringRedactsSensitiveHeaders() {
Headers headers = new Headers.Builder()
.add("content-length", "99")
.add("authorization", "peanutbutter")
.add("proxy-authorization", "chocolate")
.add("cookie", "drink=coffee")
.add("set-cookie", "accessory=sugar")
.add("user-agent", "OkHttp")
.build();
assertThat(headers.toString()).isEqualTo(""
+ "content-length: 99\n"
+ "authorization: ██\n"
+ "proxy-authorization: ██\n"
+ "cookie: ██\n"
+ "set-cookie: ██\n"
+ "user-agent: OkHttp\n");
}

@Test public void headersAddAll() {
Headers sourceHeaders = new Headers.Builder()
.add("A", "aa")
Expand Down