Make it more difficult to accidentally log sensitive headers (square#…

…6551)
tristantarrant · Nov 30, 2022 · 2de85d7 · 2de85d7
1 parent 06644bb
commit 2de85d7
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 2 deletions.
diff --git a/okhttp-tests/src/test/java/okhttp3/HeadersTest.java b/okhttp-tests/src/test/java/okhttp3/HeadersTest.java
@@ -387,6 +387,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")
@@ -421,6 +452,25 @@ 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")

diff --git a/okhttp/src/main/java/okhttp3/Headers.java b/okhttp/src/main/java/okhttp3/Headers.java
@@ -17,6 +17,8 @@
 
 package okhttp3;
 
+import static okhttp3.internal.Util.isSensitiveHeader;
+
 import java.time.Instant;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -183,7 +185,9 @@ public Builder newBuilder() {
   @Override public String toString() {
     StringBuilder result = new StringBuilder();
     for (int i = 0, size = size(); i < size; i++) {
-      result.append(name(i)).append(": ").append(value(i)).append("\n");
+      String name = name(i);
+      String value = isSensitiveHeader(name) ? "██" : value(i);
+      result.append(name(i)).append(": ").append(value).append("\n");
     }
     return result.toString();
   }
@@ -282,7 +286,8 @@ static void checkValue(String value, String name) {
       char c = value.charAt(i);
       if ((c <= '\u001f' && c != '\t') || c >= '\u007f') {
         throw new IllegalArgumentException(Util.format(
-            "Unexpected char %#04x at %d in %s value: %s", (int) c, i, name, value));
+            "Unexpected char %#04x at %d in %s value%s", (int) c, i, name,
+              isSensitiveHeader(name) ? "" : (": " + value)));
       }
     }
   }

diff --git a/okhttp/src/main/java/okhttp3/internal/Util.java b/okhttp/src/main/java/okhttp3/internal/Util.java
@@ -458,6 +458,14 @@ public static boolean verifyAsIpAddress(String host) {
     return VERIFY_AS_IP_ADDRESS.matcher(host).matches();
   }
 
+  /** Returns true if we should void putting this header in an exception or toString(). */
+  public static boolean isSensitiveHeader(String name) {
+    return name.equalsIgnoreCase("Authorization")
+          || name.equalsIgnoreCase("Cookie")
+          || name.equalsIgnoreCase("Proxy-Authorization")
+          || name.equalsIgnoreCase("Set-Cookie");
+  }
+
   /** Returns a {@link Locale#US} formatted {@link String}. */
   public static String format(String format, Object... args) {
     return String.format(Locale.US, format, args);