api: make Headers and HeadersBuilder case-insensitive (#2400)

Description: An Android change that mimics iOS changes from #2383. Make the lookup of headers in HeadersBuilder and Headers case-insensitive and preserve the original casing of headers. Risk Level: Low Testing: Unit Docs Changes: Done Release Notes: Done Fixes: #2390 Signed-off-by: Rafal Augustyniak <[email protected]>
envoyproxy · Jul 6, 2022 · 52d6a4f · 52d6a4f
1 parent 8434e67
commit 52d6a4f
Show file tree

Hide file tree

Showing 33 changed files with 469 additions and 134 deletions.
diff --git a/examples/java/hello_world/MainActivity.java b/examples/java/hello_world/MainActivity.java
@@ -108,7 +108,8 @@ private void makeRequest() {
           String message = "received headers with status " + status;
 
           StringBuilder sb = new StringBuilder();
-          for (Map.Entry<String, List<String>> entry : responseHeaders.getHeaders().entrySet()) {
+          for (Map.Entry<String, List<String>> entry :
+               responseHeaders.caseSensitiveHeaders().entrySet()) {
             String name = entry.getKey();
             if (FILTERED_HEADERS.contains(name)) {
               sb.append(name).append(": ").append(String.join(", ", entry.getValue())).append("\n");

diff --git a/examples/kotlin/hello_world/MainActivity.kt b/examples/kotlin/hello_world/MainActivity.kt
@@ -117,7 +117,7 @@ class MainActivity : Activity() {
         val message = "received headers with status $status"
 
         val sb = StringBuilder()
-        for ((name, value) in responseHeaders.headers) {
+        for ((name, value) in responseHeaders.caseSensitiveHeaders()) {
           if (name in FILTERED_HEADERS) {
             sb.append(name).append(": ").append(value.joinToString()).append("\n")
           }

diff --git a/library/kotlin/io/envoyproxy/envoymobile/BUILD b/library/kotlin/io/envoyproxy/envoymobile/BUILD
@@ -44,6 +44,7 @@ envoy_mobile_kt_library(
         "EnvoyError.kt",
         "Headers.kt",
         "HeadersBuilder.kt",
+        "HeadersContainer.kt",
         "KeyValueStore.kt",
         "LogLevel.kt",
         "PulseClient.kt",

diff --git a/library/kotlin/io/envoyproxy/envoymobile/Headers.kt b/library/kotlin/io/envoyproxy/envoymobile/Headers.kt
@@ -5,35 +5,37 @@ package io.envoyproxy.envoymobile
  * To instantiate new instances, see `{Request|Response}HeadersBuilder`.
  */
 open class Headers {
-  @Suppress("MemberNameEqualsClassName")
-  val headers: Map<String, List<String>>
+  val container: HeadersContainer
 
   /**
    * Internal constructor used by builders.
    *
-   * @param headers: Headers to set.
+   * @param container: The headers container to set.
    */
-  protected constructor(headers: Map<String, List<String>>) {
-    this.headers = headers
+  internal constructor(container: HeadersContainer) {
+    this.container = container
   }
 
   /**
-   * Get the value for the provided header name.
+   * Get the value for the provided header name. It's discouraged
+   * to use this dictionary for equality key-based lookups as this
+   * may lead to issues with headers that do not follow expected
+   * casing i.e., "Content-Length" instead of "content-length".
    *
    * @param name: Header name for which to get the current value.
    *
-   * @return List<String>?, The current headers specified for the provided name.
+   * @return The current headers specified for the provided name.
    */
   fun value(name: String): List<String>? {
-    return headers[name]
+    return container.value(name)
   }
 
   /**
    * Accessor for all underlying headers as a map.
    *
-   * @return Map<String, List<String>>, The underlying headers.
+   * @return The underlying headers.
    */
-  fun allHeaders(): Map<String, List<String>> {
-    return headers
+  fun caseSensitiveHeaders(): Map<String, List<String>> {
+    return container.caseSensitiveHeaders()
   }
 }
diff --git a/library/kotlin/io/envoyproxy/envoymobile/HeadersBuilder.kt b/library/kotlin/io/envoyproxy/envoymobile/HeadersBuilder.kt
@@ -5,15 +5,15 @@ package io.envoyproxy.envoymobile
  * See `{Request|Response}HeadersBuilder` for usage.
  */
 open class HeadersBuilder {
-  protected val headers: MutableMap<String, MutableList<String>>
+  protected val container: HeadersContainer
 
   /**
    * Instantiate a new builder, only used by child classes.
    *
-   * @param headers: The headers to start with.
+   * @param container: The headers container to start with.
    */
-  protected constructor(headers: MutableMap<String, MutableList<String>>) {
-    this.headers = headers
+  internal constructor(container: HeadersContainer) {
+    this.container = container
   }
 
   /**
@@ -28,7 +28,7 @@ open class HeadersBuilder {
     if (isRestrictedHeader(name)) {
       return this
     }
-    headers.getOrPut(name) { mutableListOf<String>() }.add(value)
+    container.add(name, value)
     return this
   }
 
@@ -44,7 +44,7 @@ open class HeadersBuilder {
     if (isRestrictedHeader(name)) {
       return this
     }
-    headers[name] = value
+    container.set(name, value)
     return this
   }
 
@@ -59,7 +59,7 @@ open class HeadersBuilder {
     if (isRestrictedHeader(name)) {
       return this
     }
-    headers.remove(name)
+    container.remove(name)
     return this
   }
 
@@ -72,10 +72,10 @@ open class HeadersBuilder {
    * @return HeadersBuilder, This builder.
    */
   internal open fun internalSet(name: String, value: MutableList<String>): HeadersBuilder {
-    headers[name] = value
+    container.set(name, value)
     return this
   }
 
   private fun isRestrictedHeader(name: String) = name.startsWith(":") ||
-    name.startsWith("x-envoy-mobile") || name == "host"
+    name.startsWith("x-envoy-mobile", ignoreCase = true) || name.equals("host", ignoreCase = true)
 }
diff --git a/library/kotlin/io/envoyproxy/envoymobile/HeadersContainer.kt b/library/kotlin/io/envoyproxy/envoymobile/HeadersContainer.kt
@@ -0,0 +1,135 @@
+package io.envoyproxy.envoymobile
+
+/**
+ * The container that manages the underlying headers map.
+ * It maintains the original casing of passed header names.
+ * It treats headers names as case-insensitive for the purpose
+ * of header lookups and header name conflict resolutions.
+ */
+open class HeadersContainer {
+  protected val headers: MutableMap<String, Header>
+
+  /**
+   * Represents a header name together with all of its values.
+   * It preserves the original casing of the header name.
+   */
+  data class Header(val name: String, var value: MutableList<String>) {
+    constructor(name: String) : this(name, mutableListOf())
+
+    fun add(value: List<String>) {
+      this.value.addAll(value)
+    }
+
+    fun add(value: String) {
+      this.value.add(value)
+    }
+  }
+
+  /**
+   * Instantiate a new instance of the receiver using the provided headers map
+   *
+   * @param headers: The headers to start with.
+   */
+  internal constructor(headers: Map<String, MutableList<String>>) {
+    var underlyingHeaders = mutableMapOf<String, Header>()
+    /**
+     * Dictionaries are unordered collections. Process headers with names
+     * that are the same when lowercased in an alphabetical order to avoid a situation
+     * in which the result of the initialization is non-derministic i.e., we want
+     * mapOf("A" to listOf("1"), "a" to listOf("2")) headers to be always converted to
+     * mapOf("A" to listOf("1", "2")) and never to mapOf("a" to listOf("2", "1")).
+     *
+     * If a given header name already exists in the processed headers map, check
+     * if the currently processed header name is before the existing header name as
+     * determined by an alphabetical order.
+     */
+    headers.forEach {
+      val lowercased = it.key.lowercase()
+      val existing = underlyingHeaders[lowercased]
+
+      if (existing == null) {
+        underlyingHeaders[lowercased] = Header(it.key, it.value)
+      } else if (existing.name > it.key) {
+        underlyingHeaders[lowercased] = Header(it.key, (it.value + existing.value).toMutableList())
+      } else {
+        underlyingHeaders[lowercased]?.add(it.value)
+      }
+    }
+
+    this.headers = underlyingHeaders
+  }
+
+  companion object {
+    /**
+     * Create a new instance of the receiver using a provider headers map.
+     * Not implemented as a constructor due to conflicting JVM signatures with
+     * other constructors.
+     *
+     * @param headers: The headers to create the container with.
+     */
+    fun create(headers: Map<String, List<String>>) : HeadersContainer {
+      return HeadersContainer(headers.mapValues { it.value.toMutableList() })
+    }
+  }
+
+  /**
+   * Add a value to a header with a given name.
+   *
+   * @param name  The name of the header. For the purpose of headers lookup
+   *              and header name conflict resolution, the name of the header
+   *              is considered to be case-insensitive.
+   * @param value The value to add.
+   */
+  fun add(name: String, value: String) {
+    val lowercased = name.lowercase()
+    headers[lowercased]?.let { it.add(value) } ?: run {
+      headers.put(lowercased, Header(name, mutableListOf(value)))
+    }
+  }
+
+
+  /**
+   * Set the value of a given header.
+   *
+   * @param name  The name of the header.
+   * @param value The value to set the header value to.
+   */
+  fun set(name: String, value: List<String>) {
+    headers[name.lowercase()] = Header(name, value.toMutableList())
+  }
+
+  /**
+   * Remove a given header.
+   *
+   * @param name The name of the header to remove.
+   */
+  fun remove(name: String) {
+    headers.remove(name.lowercase())
+  }
+
+  /**
+   * Get the value for the provided header name.
+   *
+   * @param name The case-insensitive header name for which to
+   *             get the current value.
+   * @return The value associated with a given header.
+   */
+  fun value(name: String): List<String>? {
+    return headers[name.lowercase()]?.value
+  }
+
+  /**
+   * Accessor for all underlying case-sensitive headers. When possible,
+   * use case-insensitive accessors instead.
+   *
+   * @return The underlying headers.
+   */
+  fun caseSensitiveHeaders(): Map<String, List<String>> {
+    var caseSensitiveHeaders = mutableMapOf<String, List<String>>()
+    headers.forEach {
+      caseSensitiveHeaders.put(it.value.name, it.value.value)
+    }
+
+    return caseSensitiveHeaders
+  }
+}
diff --git a/library/kotlin/io/envoyproxy/envoymobile/RequestHeaders.kt b/library/kotlin/io/envoyproxy/envoymobile/RequestHeaders.kt
@@ -9,7 +9,9 @@ open class RequestHeaders : Headers {
    *
    * @param headers: Headers to set.
    */
-  internal constructor(headers: Map<String, List<String>>) : super(headers)
+  internal constructor(headers: Map<String, List<String>>) : super(HeadersContainer.create(headers))
+
+  internal constructor(container: HeadersContainer): super(container)
 
   /**
    * Method for the request.
@@ -49,9 +51,5 @@ open class RequestHeaders : Headers {
    *
    * @return RequestHeadersBuilder, The new builder.
    */
-  fun toRequestHeadersBuilder() = RequestHeadersBuilder(
-    headers.mapValues {
-      it.value.toMutableList()
-    }.toMutableMap()
-  )
+  fun toRequestHeadersBuilder() = RequestHeadersBuilder(container)
 }
diff --git a/library/kotlin/io/envoyproxy/envoymobile/RequestHeadersBuilder.kt b/library/kotlin/io/envoyproxy/envoymobile/RequestHeadersBuilder.kt
@@ -18,22 +18,30 @@ class RequestHeadersBuilder : HeadersBuilder {
     authority: String,
     path: String
   ) :
-    super(
-      mutableMapOf(
+    super(HeadersContainer(
+      mapOf(
         ":authority" to mutableListOf(authority),
         ":method" to mutableListOf(method.stringValue),
         ":path" to mutableListOf(path),
         ":scheme" to mutableListOf(scheme)
       )
     )
+    )
 
   /**
    * Instantiate a new builder. Used only by RequestHeaders to convert back to
    * RequestHeadersBuilder.
    *
    * @param headers: The headers to start with.
    */
-  internal constructor(headers: MutableMap<String, MutableList<String>>) : super(headers)
+  internal constructor(headers: Map<String, MutableList<String>>) : super(HeadersContainer(headers))
+
+  /**
+   * Instantiate a new builder.
+   *
+   * @param container: The headers container to start with.
+   */
+  internal constructor(container: HeadersContainer) : super(container)
 
   override fun add(name: String, value: String): RequestHeadersBuilder {
     super.add(name, value)
@@ -92,6 +100,6 @@ class RequestHeadersBuilder : HeadersBuilder {
    * @return RequestHeaders, New instance of request headers.
    */
   fun build(): RequestHeaders {
-    return RequestHeaders(headers)
+    return RequestHeaders(container)
   }
 }
diff --git a/library/kotlin/io/envoyproxy/envoymobile/RequestTrailers.kt b/library/kotlin/io/envoyproxy/envoymobile/RequestTrailers.kt
@@ -12,14 +12,17 @@ class RequestTrailers : Trailers {
    */
   internal constructor(trailers: Map<String, List<String>>) : super(trailers)
 
+  /**
+   * Instantiate a new builder.
+   *
+   * @param container: The headers container to start with.
+   */
+  internal constructor(container: HeadersContainer) : super(container)
+
   /**
    * Convert the trailers back to a builder for mutation.
    *
    * @return RequestTrailersBuilder, The new builder.
    */
-  fun toRequestTrailersBuilder() = RequestTrailersBuilder(
-    headers.mapValues {
-      it.value.toMutableList()
-    }.toMutableMap()
-  )
+  fun toRequestTrailersBuilder() = RequestTrailersBuilder(container)
 }
diff --git a/library/kotlin/io/envoyproxy/envoymobile/RequestTrailersBuilder.kt b/library/kotlin/io/envoyproxy/envoymobile/RequestTrailersBuilder.kt
@@ -4,18 +4,26 @@ package io.envoyproxy.envoymobile
  * Builder used for constructing instances of `RequestTrailers`.
  */
 class RequestTrailersBuilder : HeadersBuilder {
-  /**
-   * Initialize a new instance of the builder.
+  /*
+   * Instantiate a new builder.
    */
-  constructor() : super(mutableMapOf())
+  internal constructor() : super(HeadersContainer(mapOf()))
 
-  /**
+  /*
+   * Instantiate a new instance of the builder.
+   *
+   * @param container: The headers container to start with.
+   */
+  internal constructor(container: HeadersContainer) : super(container)
+
+  /*
    * Instantiate a new builder. Used only by RequestTrailers to convert back to
    * RequestTrailersBuilder.
    *
    * @param trailers: The trailers to start with.
    */
-  internal constructor(trailers: MutableMap<String, MutableList<String>>) : super(trailers)
+  internal constructor(trailers: MutableMap<String, MutableList<String>>)
+    : super(HeadersContainer(trailers))
 
   override fun add(name: String, value: String): RequestTrailersBuilder {
     super.add(name, value)
@@ -43,6 +51,6 @@ class RequestTrailersBuilder : HeadersBuilder {
    * @return RequestTrailers, New instance of request trailers.
    */
   fun build(): RequestTrailers {
-    return RequestTrailers(headers)
+    return RequestTrailers(container)
   }
 }