Make it more difficult to accidentally log sensitive headers (#6551) (#6740)

Co-authored-by: Jesse Wilson <[email protected]>

Author: yschimke

okhttp/src/main/kotlin/okhttp3/Headers.kt

@@ -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
 
 /**
@@ -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")
       }
     }
@@ -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)
@@ -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)
@@ -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")
         }
       }
     }

okhttp/src/main/kotlin/okhttp3/internal/Util.kt

@@ -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
@@ -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)

okhttp/src/test/java/okhttp3/HeadersTest.java

@@ -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")
@@ -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")