Lower case headers in response and use TreeMap to allow case insensitive access (#418)

CHANGELOG.md

@@ -4,6 +4,10 @@
   * Previously the OkhttpClient would throw an exception, and ApacheHttpClient
     would report a wrong, possibly negative value
 * Adds support for encoded query parameters in `@QueryMap` via `@QueryMap(encoded = true)`
+* Keys in `Response.headers` are now lower-cased. This map is now case-insensitive with regards to keys,
+  and iterates in lexicographic order.
+  * This is a step towards supporting http2, as header names in http1 are treated as case-insensitive
+    and http2 down-cases header names.
 
 ### Version 8.17
 * Adds support to RxJava Completable via `HystrixFeign` builder with fallback support

core/src/main/java/feign/Response.java

@@ -24,8 +24,10 @@ import java.io.Reader;
 import java.nio.charset.Charset;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.LinkedHashMap;
+import java.util.LinkedList;
+import java.util.Locale;
 import java.util.Map;
+import java.util.TreeMap;
 
 import static feign.Util.UTF_8;
 import static feign.Util.checkNotNull;
@@ -47,10 +49,7 @@ public final class Response implements Closeable {
     checkState(status >= 200, "Invalid status code: %s", status);
     this.status = status;
     this.reason = reason; //nullable
-    LinkedHashMap<String, Collection<String>> copyOf =
-        new LinkedHashMap<String, Collection<String>>();
-    copyOf.putAll(checkNotNull(headers, "headers"));
-    this.headers = Collections.unmodifiableMap(copyOf);
+    this.headers = Collections.unmodifiableMap(caseInsensitiveCopyOf(headers));
     this.body = body; //nullable
   }
 
@@ -92,6 +91,9 @@ public final class Response implements Closeable {
     return reason;
   }
 
+  /**
+   * Returns a case-insensitive mapping of header names to their values.
+   */
   public Map<String, Collection<String>> headers() {
     return headers;
   }
@@ -242,4 +244,17 @@ public final class Response implements Closeable {
       return decodeOrDefault(data, UTF_8, "Binary data");
     }
   }
+
+  private static Map<String, Collection<String>> caseInsensitiveCopyOf(Map<String, Collection<String>> headers) {
+    Map<String, Collection<String>> result = new TreeMap<String, Collection<String>>(String.CASE_INSENSITIVE_ORDER);
+
+    for (Map.Entry<String, Collection<String>> entry : headers.entrySet()) {
+      String headerName = entry.getKey();
+      if (!result.containsKey(headerName)) {
+        result.put(headerName.toLowerCase(Locale.ROOT), new LinkedList<String>());
+      }
+      result.get(headerName).addAll(entry.getValue());
+    }
+    return result;
+  }
 }

core/src/test/java/feign/LoggerTest.java

@@ -81,7 +81,7 @@ public class LoggerTest {
               "\\[SendsStuff#login\\] Content-Length: 80",
               "\\[SendsStuff#login\\] ---> END HTTP \\(80-byte body\\)",
               "\\[SendsStuff#login\\] <--- HTTP/1.1 200 OK \\([0-9]+ms\\)",
-              "\\[SendsStuff#login\\] Content-Length: 3",
+              "\\[SendsStuff#login\\] content-length: 3",
               "\\[SendsStuff#login\\] <--- END HTTP \\(3-byte body\\)")},
           {Level.FULL, Arrays.asList(
               "\\[SendsStuff#login\\] ---> POST http://localhost:[0-9]+/ HTTP/1.1",
@@ -91,7 +91,7 @@ public class LoggerTest {
               "\\[SendsStuff#login\\] \\{\"customer_name\": \"netflix\", \"user_name\": \"denominator\", \"password\": \"password\"\\}",
               "\\[SendsStuff#login\\] ---> END HTTP \\(80-byte body\\)",
               "\\[SendsStuff#login\\] <--- HTTP/1.1 200 OK \\([0-9]+ms\\)",
-              "\\[SendsStuff#login\\] Content-Length: 3",
+              "\\[SendsStuff#login\\] content-length: 3",
               "\\[SendsStuff#login\\] ",
               "\\[SendsStuff#login\\] foo",
               "\\[SendsStuff#login\\] <--- END HTTP \\(3-byte body\\)")}
@@ -167,7 +167,7 @@ public class LoggerTest {
               "\\[SendsStuff#login\\] Content-Length: 80",
               "\\[SendsStuff#login\\] ---> END HTTP \\(80-byte body\\)",
               "\\[SendsStuff#login\\] <--- HTTP/1.1 200 OK \\([0-9]+ms\\)",
-              "\\[SendsStuff#login\\] Content-Length: 3",
+              "\\[SendsStuff#login\\] content-length: 3",
               "\\[SendsStuff#login\\] <--- ERROR SocketTimeoutException: Read timed out \\([0-9]+ms\\)")},
           {Level.FULL, Arrays.asList(
               "\\[SendsStuff#login\\] ---> POST http://localhost:[0-9]+/ HTTP/1.1",
@@ -177,7 +177,7 @@ public class LoggerTest {
               "\\[SendsStuff#login\\] \\{\"customer_name\": \"netflix\", \"user_name\": \"denominator\", \"password\": \"password\"\\}",
               "\\[SendsStuff#login\\] ---> END HTTP \\(80-byte body\\)",
               "\\[SendsStuff#login\\] <--- HTTP/1.1 200 OK \\([0-9]+ms\\)",
-              "\\[SendsStuff#login\\] Content-Length: 3",
+              "\\[SendsStuff#login\\] content-length: 3",
               "\\[SendsStuff#login\\] ",
               "\\[SendsStuff#login\\] <--- ERROR SocketTimeoutException: Read timed out \\([0-9]+ms\\)",
               "\\[SendsStuff#login\\] java.net.SocketTimeoutException: Read timed out.*",

core/src/test/java/feign/ResponseTest.java

@@ -17,10 +17,15 @@ package feign;
 
 import org.junit.Test;
 
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
 
 import static feign.assertj.FeignAssertions.assertThat;
+import static org.assertj.core.api.Assertions.entry;
 
 public class ResponseTest {
 
@@ -32,4 +37,37 @@ public class ResponseTest {
     assertThat(response.reason()).isNull();
     assertThat(response.toString()).isEqualTo("HTTP/1.1 200\n\n");
   }
+
+  @Test
+  public void lowerCasesNamesOfHeaders() {
+    Response response = Response.create(200,
+                                        null,
+                                        Collections.singletonMap("Content-Type",
+                                                                 Collections.singletonList("application/json")),
+                                        new byte[0]);
+    assertThat(response.headers()).containsOnly(entry(("content-type"), Collections.singletonList("application/json")));
+  }
+
+  @Test
+  public void canAccessHeadersCaseInsensitively() {
+    List<String> valueList = Collections.singletonList("application/json");
+    Response response = Response.create(200,
+                                        null,
+                                        Collections.singletonMap("Content-Type", valueList),
+                                        new byte[0]);
+    assertThat(response.headers().get("content-type")).isEqualTo(valueList);
+    assertThat(response.headers().get("Content-Type")).isEqualTo(valueList);
+  }
+
+  @Test
+  public void headerValuesWithSameNameOnlyVaryingInCaseAreMerged() {
+    Map<String, Collection<String>> headersMap = new LinkedHashMap<>();
+    headersMap.put("Set-Cookie", Arrays.asList("Cookie-A=Value", "Cookie-B=Value"));
+    headersMap.put("set-cookie", Arrays.asList("Cookie-C=Value"));
+
+    Response response = Response.create(200, null, headersMap, new byte[0]);
+
+    List<String> expectedHeaderValue = Arrays.asList("Cookie-A=Value", "Cookie-B=Value", "Cookie-C=Value");
+    assertThat(response.headers()).containsOnly(entry(("set-cookie"), expectedHeaderValue));
+  }
 }