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

Description: Make the lookup of headers in HeadersBuilder and Headers case-insensitive and preserve the original casing of headers. Implemented on iOS only. Android side-changes are going to be implemented in a follow up PR. GH issue: #2390. Risk Level: Low Testing: Unit Tests Docs Changes: Updated Release Notes: Updated Signed-off-by: Rafal Augustyniak <[email protected]>
envoyproxy · Jun 28, 2022 · 45ee2c3 · 45ee2c3
1 parent 557b70f
commit 45ee2c3
Show file tree

Hide file tree

Showing 27 changed files with 327 additions and 71 deletions.
diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst
@@ -24,6 +24,7 @@ Bugfixes:
 - Android: update Kotlin standard libraries to 1.6.21 (:issue:`#2256 <2256>`)
 - fix bug where finalStreamIntel was not consistently set on cancel (:issue:`#2285 <2285>`)
 - iOS: fix termination crash in ProvisionalDispatcher (:issue:`#2059 <2059>`)
+- iOS: make headers lookup in ``HeadersBuilder`` and ``Headers`` case-insensitive. Rename ``allHeaders`` method to ``caseSensitiveHeaders``. (:issue:`#2383 <2383>`)
 - iOS: use correct DNS resolver when using C++ config builder (:issue: `#2378 <2378 >`)
 
 Features:

diff --git a/examples/objective-c/hello_world/ViewController.m b/examples/objective-c/hello_world/ViewController.m
@@ -90,9 +90,9 @@ - (void)performRequest {
     NSString *message = [NSString stringWithFormat:@"received headers with status %i", statusCode];
 
     NSMutableString *headerMessage = [NSMutableString new];
-    for (NSString *name in headers.allHeaders) {
+    for (NSString *name in headers.caseSensitiveHeaders) {
       if ([self.filteredHeaders containsObject:name]) {
-        NSArray<NSString *> *values = headers.allHeaders[name];
+        NSArray<NSString *> *values = headers.caseSensitiveHeaders[name];
         NSString *joined = [values componentsJoinedByString:@", "];
         NSString *pair = [NSString stringWithFormat:@"%@: %@\n", name, joined];
         [headerMessage appendString:pair];

diff --git a/examples/swift/async_await/ContentView.swift b/examples/swift/async_await/ContentView.swift
@@ -72,12 +72,8 @@ private extension StreamClient {
         let stream = self
             .newStreamPrototype()
             .setOnResponseHeaders { headers, _, _ in
-                let allHeaders = headers.allHeaders()
-
-                if allHeaders[":status"]?.first == "200",
-                   // TODO(jpsim): Expose an API that enforces case-insensitive lookups
-                   let contentLengthValue = allHeaders["Content-Length"] ??
-                                            allHeaders["content-length"],
+                if headers.value(forName: ":status")?.first == "200",
+                   let contentLengthValue = headers.value(forName: "content-length"),
                    let firstContentLength = contentLengthValue.first,
                    let contentLengthInt = Int64(firstContentLength)
                 {
@@ -87,7 +83,7 @@ private extension StreamClient {
                     return
                 }
 
-                let headerMessage = allHeaders
+                let headerMessage = headers.caseSensitiveHeaders()
                     .map { "\($0.key): \($0.value.joined(separator: ", "))" }
                     .joined(separator: "\n")
 

diff --git a/examples/swift/hello_world/ViewController.swift b/examples/swift/hello_world/ViewController.swift
@@ -70,7 +70,7 @@ final class ViewController: UITableViewController {
         let statusCode = headers.httpStatus.map(String.init) ?? "nil"
         let message = "received headers with status \(statusCode)"
 
-        let headerMessage = headers.allHeaders()
+        let headerMessage = headers.caseSensitiveHeaders()
           .filter { kFilteredHeaders.contains($0.key) }
           .map { "\($0.key): \($0.value.joined(separator: ", "))" }
           .joined(separator: "\n")

diff --git a/library/swift/BUILD b/library/swift/BUILD
@@ -17,6 +17,7 @@ swift_library(
         "FinalStreamIntel.swift",
         "Headers.swift",
         "HeadersBuilder.swift",
+        "HeadersContainer.swift",
         "KeyValueStore.swift",
         "LogLevel.swift",
         "NetworkMonitoringMode.swift",

diff --git a/library/swift/Headers.swift b/library/swift/Headers.swift
@@ -4,45 +4,64 @@ import Foundation
 /// To instantiate new instances, see `{Request|Response}HeadersBuilder`.
 @objcMembers
 public class Headers: NSObject {
-  let headers: [String: [String]]
+  let container: HeadersContainer
 
   /// Get the value for the provided header name.
   ///
+  /// - note: The lookup for a header name is a case-insensitive operation.
+  ///
   /// - parameter name: Header name for which to get the current value.
   ///
   /// - returns: The current headers specified for the provided name.
   public func value(forName name: String) -> [String]? {
-    return self.headers[name]
+    return self.container.value(forName: name)
   }
 
-  /// Accessor for all underlying headers as a map.
+  /// Accessor for all underlying case-sensitive headers. When possible,
+  /// use case-insensitive accessors instead.
+  ///
+  /// - warning: 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".
   ///
-  /// - returns: The underlying headers.
-  public func allHeaders() -> [String: [String]] {
-    return self.headers
+  /// - returns: The underlying case-sensitive headers.
+  public func caseSensitiveHeaders() -> [String: [String]] {
+    return self.container.allHeaders()
   }
 
   /// Internal initializer used by builders.
   ///
-  /// - parameter headers: Headers to set.
-  required init(headers: [String: [String]]) {
-    self.headers = headers
+  /// - parameter container: Headers to set.
+  required init(container: HeadersContainer) {
+    self.container = container
     super.init()
   }
+
+  /// Inialize the receiver with a given headers map.
+  ///
+  /// - parameter headers: The headers map to use.
+  convenience init(headers: [String: [String]]) {
+    self.init(container: HeadersContainer(headers: headers))
+  }
+
+  override convenience init() {
+    self.init(headers: [:])
+  }
 }
 
 // MARK: - Equatable
 
 extension Headers {
   public override func isEqual(_ object: Any?) -> Bool {
-    return (object as? Self)?.headers == self.headers
+    return (object as? Self)?.container == self.container
   }
 }
 
 // MARK: - CustomStringConvertible
 
 extension Headers {
   public override var description: String {
-    return "\(type(of: self)) \(self.headers.description)"
+    return "\(type(of: self)) \(self.caseSensitiveHeaders())"
   }
 }
diff --git a/library/swift/HeadersBuilder.swift b/library/swift/HeadersBuilder.swift
@@ -3,14 +3,19 @@ import Foundation
 private let kRestrictedPrefixes = [":", "x-envoy-mobile"]
 
 private func isRestrictedHeader(name: String) -> Bool {
-  return name == "host" || kRestrictedPrefixes.contains { name.hasPrefix($0) }
+  let isHostHeader = name.caseInsensitiveCompare("host") == .orderedSame
+  lazy var hasRestrictedPrefix = kRestrictedPrefixes
+    .contains { name.range(of: $0, options: [.caseInsensitive, .anchored]) != nil }
+  return isHostHeader || hasRestrictedPrefix
 }
 
 /// Base builder class used to construct `Headers` instances.
+/// It preserves the original casing of headers and enforces
+/// a case-insensitive lookup and setting of headers.
 /// See `{Request|Response}HeadersBuilder` for usage.
 @objcMembers
 public class HeadersBuilder: NSObject {
-  private(set) var headers: [String: [String]]
+  private(set) var container: HeadersContainer
 
   /// Append a value to the header name.
   ///
@@ -24,7 +29,7 @@ public class HeadersBuilder: NSObject {
       return self
     }
 
-    self.headers[name, default: []].append(value)
+    self.container.add(name: name, value: value)
     return self
   }
 
@@ -40,7 +45,7 @@ public class HeadersBuilder: NSObject {
       return self
     }
 
-    self.headers[name] = value
+    self.container.set(name: name, value: value)
     return self
   }
 
@@ -55,7 +60,7 @@ public class HeadersBuilder: NSObject {
       return self
     }
 
-    self.headers[name] = nil
+    self.container.set(name: name, value: nil)
     return self
   }
 
@@ -69,22 +74,34 @@ public class HeadersBuilder: NSObject {
   /// - returns: This builder.
   @discardableResult
   func internalSet(name: String, value: [String]) -> Self {
-    self.headers[name] = value
+    self.container.set(name: name, value: value)
     return self
   }
 
+  func allHeaders() -> [String: [String]] {
+    return self.container.allHeaders()
+  }
+
   // Only explicitly implemented to work around a swiftinterface issue in Swift 5.1. This can be
   // removed once envoy is only built with Swift 5.2+
   public override init() {
-    self.headers = [:]
+    self.container = HeadersContainer()
     super.init()
   }
 
-  /// Initialize a new builder. Subclasses should provide their own public convenience initializers.
+  // Initialize a new builder using the provided headers container.
   ///
-  /// - parameter headers: The headers with which to start.
-  required init(headers: [String: [String]]) {
-    self.headers = headers
+  /// - parameter container: The headers container to initialize the receiver with.
+  init(container: HeadersContainer) {
+    self.container = container
+    super.init()
+  }
+
+  // Initialize a new builder. Subclasses should provide their own public convenience initializers.
+  //
+  // - parameter headers: The headers with which to start.
+  init(headers: [String: [String]]) {
+    self.container = HeadersContainer(headers: headers)
     super.init()
   }
 }
@@ -93,6 +110,6 @@ public class HeadersBuilder: NSObject {
 
 extension HeadersBuilder {
   public override func isEqual(_ object: Any?) -> Bool {
-    return (object as? Self)?.headers == self.headers
+    return (object as? Self)?.container == self.container
   }
 }
diff --git a/library/swift/HeadersContainer.swift b/library/swift/HeadersContainer.swift
@@ -0,0 +1,110 @@
+/// The container which 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.
+struct HeadersContainer: Equatable {
+  private var headers: [String: Header]
+
+  /// Represents a headers name together with all of its values.
+  /// It preserves the original casing of the header name.
+  struct Header: Equatable {
+    private(set) var name: String
+    private(set) var value: [String]
+
+    init(name: String, value: [String] = []) {
+      self.name = name
+      self.value = value
+    }
+
+    mutating func addValue(_ value: [String]) {
+      self.value.append(contentsOf: value)
+    }
+
+    mutating func addValue(_ value: String) {
+      self.value.append(value)
+    }
+  }
+
+  /// Initialize a new instance of the receiver using the provided headers map.
+  ///
+  /// - parameter headers: The headers map.
+  init(headers: [String: [String]]) {
+    var underlyingHeaders = [String: Header]()
+    for (name, value) in headers {
+      let lowercasedName = name.lowercased()
+      /// Dictionaries in Swift 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
+      /// "[A: ["1"]", "a: ["2"]]" headers to be always converted to ["A": ["1", "2"]] and
+      /// never to "a": ["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.
+      guard let existingHeader = underlyingHeaders[lowercasedName] else {
+        underlyingHeaders[lowercasedName] = Header(name: name, value: value)
+        continue
+      }
+
+      if existingHeader.name > name {
+        underlyingHeaders[lowercasedName] =
+          Header(name: name, value: value + existingHeader.value)
+      } else {
+        underlyingHeaders[lowercasedName]?.addValue(value)
+      }
+    }
+    self.headers = underlyingHeaders
+  }
+
+  /// Initialize an empty headers container.
+  init() {
+    self.headers = [:]
+  }
+
+  /// Add a value to a header with a given name.
+  ///
+  /// - parameter 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.
+  /// - parameter value: The value to add.
+  mutating func add(name: String, value: String) {
+      self.headers[name.lowercased(), default: Header(name: name)].addValue(value)
+  }
+
+  /// Set the value of a given header.
+  ///
+  /// - parameter name:  The name of the header.
+  /// - parameter value: The value to set the header value to.
+  mutating func set(name: String, value: [String]?) {
+    guard let value = value else {
+      self.headers[name.lowercased()] = nil
+      return
+    }
+    self.headers[name.lowercased()] = Header(name: name, value: value)
+  }
+
+  /// Get the value for the provided header name.
+  ///
+  /// - parameter name: The case-insensitive header name for which to
+  ///                   get the current value.
+  ///
+  /// - returns: The value associated with a given header.
+  func value(forName name: String) -> [String]? {
+    return self.headers[name.lowercased()]?.value
+  }
+
+  /// Return all underlying headers.
+  ///
+  /// - returns: The underlying headers.
+  func allHeaders() -> [String: [String]] {
+    return Dictionary(uniqueKeysWithValues: self.headers.map { _, value in
+      return (value.name, value.value)
+    })
+  }
+}
+
+extension HeadersContainer: CustomStringConvertible {
+  var description: String {
+    return self.headers.description
+  }
+}
diff --git a/library/swift/RequestHeaders.swift b/library/swift/RequestHeaders.swift
@@ -29,6 +29,6 @@ public class RequestHeaders: Headers {
   ///
   /// - returns: The new builder.
   public func toRequestHeadersBuilder() -> RequestHeadersBuilder {
-    return RequestHeadersBuilder(headers: self.headers)
+    return RequestHeadersBuilder(container: self.container)
   }
 }
diff --git a/library/swift/RequestHeadersBuilder.swift b/library/swift/RequestHeadersBuilder.swift
@@ -52,6 +52,6 @@ public final class RequestHeadersBuilder: HeadersBuilder {
   ///
   /// - returns: New instance of request headers.
   public func build() -> RequestHeaders {
-    return RequestHeaders(headers: self.headers)
+    return RequestHeaders(container: self.container)
   }
 }
diff --git a/library/swift/RequestTrailers.swift b/library/swift/RequestTrailers.swift
@@ -7,6 +7,6 @@ public final class RequestTrailers: Trailers {
   ///
   /// - returns: The new builder.
   public func toRequestTrailersBuilder() -> RequestTrailersBuilder {
-    return RequestTrailersBuilder(headers: self.headers)
+    return RequestTrailersBuilder(container: self.container)
   }
 }
diff --git a/library/swift/RequestTrailersBuilder.swift b/library/swift/RequestTrailersBuilder.swift
@@ -5,13 +5,13 @@ import Foundation
 public final class RequestTrailersBuilder: HeadersBuilder {
   /// Initialize a new instance of the builder.
   public override convenience init() {
-    self.init(headers: [:])
+    self.init(container: HeadersContainer(headers: [:]))
   }
 
   /// Build the request trailers using the current builder.
   ///
   /// - returns: New instance of request trailers.
   public func build() -> RequestTrailers {
-    return RequestTrailers(headers: self.headers)
+    return RequestTrailers(container: self.container)
   }
 }
diff --git a/library/swift/ResponseHeaders.swift b/library/swift/ResponseHeaders.swift
@@ -11,6 +11,6 @@ public final class ResponseHeaders: Headers {
   ///
   /// - returns: The new builder.
   public func toResponseHeadersBuilder() -> ResponseHeadersBuilder {
-    return ResponseHeadersBuilder(headers: self.headers)
+    return ResponseHeadersBuilder(container: self.container)
   }
 }