Skip to content

Commit

Permalink
Implement a very lazy translation of headers
Browse files Browse the repository at this point in the history
Add HeaderContainer tests in dual mode
  • Loading branch information
Pushkar N Kulkarni committed Dec 19, 2018
1 parent 710d459 commit e9ca626
Show file tree
Hide file tree
Showing 6 changed files with 179 additions and 57 deletions.
4 changes: 2 additions & 2 deletions Sources/KituraNet/ClientResponse.swift
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,11 @@ public class ClientResponse {
guard let httpHeaders = _headers else {
return HeadersContainer()
}
return HeadersContainer.create(from: httpHeaders)
return HeadersContainer(with: httpHeaders)
}

set {
_headers = newValue.httpHeaders()
_headers = newValue.nioHeaders
}
}
/// The HTTP Status code, as an Int, sent in the response by the remote server.
Expand Down
2 changes: 1 addition & 1 deletion Sources/KituraNet/HTTP/HTTPServerRequest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ public class HTTPServerRequest: ServerRequest {
}

init(ctx: ChannelHandlerContext, requestHead: HTTPRequestHead, enableSSL: Bool) {
self.headers = HeadersContainer.create(from: requestHead.headers)
self.headers = HeadersContainer(with: requestHead.headers)
self.method = requestHead.method.string()
self.httpVersionMajor = requestHead.version.major
self.httpVersionMinor = requestHead.version.minor
Expand Down
2 changes: 1 addition & 1 deletion Sources/KituraNet/HTTP/HTTPServerResponse.swift
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ public class HTTPServerResponse: ServerResponse {

/// Send response to the client
private func sendResponse(channel: Channel, handler: HTTPRequestHandler, status: HTTPResponseStatus, withBody: Bool = true) throws {
let response = HTTPResponseHead(version: httpVersion, status: status, headers: headers.httpHeaders())
let response = HTTPResponseHead(version: httpVersion, status: status, headers: headers.nioHeaders)
channel.write(handler.wrapOutboundOut(.head(response)), promise: nil)
if withBody && buffer.readableBytes > 0 {
channel.write(handler.wrapOutboundOut(.body(.byteBuffer(buffer))), promise: nil)
Expand Down
123 changes: 79 additions & 44 deletions Sources/KituraNet/HTTP/HeadersContainer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,26 @@ public class HeadersContainer {

/// The header storage
internal var headers: [String: (key: String, value: [String])] = [:]

/// An alternate backing store of type `NIOHTTP1.HTTPHeader` used to avoid translations between HeadersContainer and HTTPHeaders
var nioHeaders: HTTPHeaders = HTTPHeaders()

/// Create an instance of `HeadersContainer`
public init() {}

/// A special initializer for HTTPServerRequest, to make the latter better performant
init(with nioHeaders: HTTPHeaders) {
self.nioHeaders = nioHeaders
}

private enum _Mode {
case nio // Headers are simply backed up by `NIOHTTP1.HTTPHeaders`
case dual // Headers are backed up by a dictionary as well, we switch to this mode while using HeadersContainer as Collection
}

// The default mode is nio
private var mode: _Mode = .nio

/// Access the value of a HTTP header using subscript syntax.
///
/// - Parameter key: The HTTP header key
Expand All @@ -42,10 +58,16 @@ public class HeadersContainer {

set(newValue) {
if let newValue = newValue {
set(key, value: newValue)
nioHeaders.replace(name: key, values: newValue)
if mode == .dual {
set(key, value: newValue)
}
}
else {
remove(key)
nioHeaders.remove(name: key)
if mode == .dual {
remove(key)
}
}
}
}
Expand All @@ -55,36 +77,49 @@ public class HeadersContainer {
/// - Parameter key: The HTTP header key
/// - Parameter value: An array of strings to add as values of the HTTP header
public func append(_ key: String, value: [String]) {

let lowerCaseKey = key.lowercased()
let entry = headers[lowerCaseKey]
var entry = nioHeaders[key]

switch(lowerCaseKey) {

case "set-cookie":
if let _ = entry {
headers[lowerCaseKey]?.value += value
if entry.count > 0 {
entry += value
nioHeaders.replace(name: key, values: entry)
if mode == .dual {
headers[lowerCaseKey]?.value += value
}
} else {
set(key, lowerCaseKey: lowerCaseKey, value: value)
nioHeaders.add(name: key, values: value)
if mode == .dual {
set(key, lowerCaseKey: lowerCaseKey, value: value)
}
}

case "content-type", "content-length", "user-agent", "referer", "host",
"authorization", "proxy-authorization", "if-modified-since",
"if-unmodified-since", "from", "location", "max-forwards",
"retry-after", "etag", "last-modified", "server", "age", "expires":
if let _ = entry {
if entry.count > 0 {
Log.warning("Duplicate header \(key) discarded")
break
}
fallthrough

default:
guard let oldValue = entry?.value.first else {
set(key, lowerCaseKey: lowerCaseKey, value: value)
return
if nioHeaders[key].count == 0 {
nioHeaders.add(name: key, values: value)
if mode == .dual {
set(key, lowerCaseKey: lowerCaseKey, value: value)
}
} else {
let oldValue = nioHeaders[key].first!
let newValue = oldValue + ", " + value.joined(separator: ", ")
nioHeaders.replaceOrAdd(name: key, value: newValue)
if mode == .dual {
headers[lowerCaseKey]?.value[0] = newValue
}
}
let newValue = oldValue + ", " + value.joined(separator: ", ")
headers[lowerCaseKey]?.value[0] = newValue
}
}

Expand All @@ -97,12 +132,17 @@ public class HeadersContainer {
}

private func get(_ key: String) -> [String]? {
return headers[key.lowercased()]?.value
let values = nioHeaders[key]
// `HTTPHeaders.subscript` returns a [] if no header is found, but `HeadersContainer` is expected to return a nil
return values.count > 0 ? values : nil
}

/// Remove all of the headers
public func removeAll() {
headers.removeAll(keepingCapacity: true)
nioHeaders = HTTPHeaders()
if mode == .dual {
headers.removeAll(keepingCapacity: true)
}
}

private func set(_ key: String, value: [String]) {
Expand All @@ -116,6 +156,14 @@ public class HeadersContainer {
private func remove(_ key: String) {
headers.removeValue(forKey: key.lowercased())
}

func checkAndSwitchToDualMode() {
guard mode == .nio else { return }
mode = .dual
for header in nioHeaders {
headers[header.name.lowercased()] = (header.name, nioHeaders[header.name])
}
}
}

extension HTTPHeaders {
Expand All @@ -125,22 +173,30 @@ extension HTTPHeaders {
}
}

mutating func replaceOrAdd(name: String, values: [String]) {
values.forEach {
replaceOrAdd(name: name, value: $0)
mutating func replace(name: String, values: [String]) {
self.replaceOrAdd(name: name, value: values[0])
for value in values.suffix(from: 1) {
self.add(name: name, value: value)
}
}
}
/// Conformance to the `Collection` protocol
/// As soon as either of these properties or methods are invoked, we need to switch to the `dual` mode of operation
extension HeadersContainer: Collection {

public typealias Index = DictionaryIndex<String, (key: String, value: [String])>

/// The starting index of the `HeadersContainer` collection
public var startIndex:Index { return headers.startIndex }
public var startIndex:Index {
checkAndSwitchToDualMode()
return headers.startIndex
}

/// The ending index of the `HeadersContainer` collection
public var endIndex:Index { return headers.endIndex }
public var endIndex:Index {
checkAndSwitchToDualMode()
return headers.endIndex
}

/// Get a (key value) tuple from the `HeadersContainer` collection at the specified position.
///
Expand All @@ -150,6 +206,7 @@ extension HeadersContainer: Collection {
/// - Returns: A (key, value) tuple.
public subscript(position: Index) -> (key: String, value: [String]) {
get {
checkAndSwitchToDualMode()
return headers[position].value
}
}
Expand All @@ -160,29 +217,7 @@ extension HeadersContainer: Collection {
///
/// - Returns: The Index in the `HeadersContainer` collection after the one specified.
public func index(after i: Index) -> Index {
checkAndSwitchToDualMode()
return headers.index(after: i)
}
}

/// Kitura uses HeadersContainer and NIOHTTP1 expects HTTPHeader - bridging methods
extension HeadersContainer {
/// HTTPHeaders to HeadersContainer
static func create(from httpHeaders: HTTPHeaders) -> HeadersContainer {
let headerContainer = HeadersContainer()
for header in httpHeaders {
headerContainer.append(header.name, value: header.value)
}
return headerContainer
}

func httpHeaders() -> HTTPHeaders {
var httpHeaders = HTTPHeaders()
for (_, keyValues) in headers {
for value in keyValues.1 {
httpHeaders.add(name: keyValues.0, value: value)
}
}
return httpHeaders
}
}

16 changes: 8 additions & 8 deletions Tests/KituraNetTests/HTTPResponseTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -64,24 +64,24 @@ class HTTPResponseTests: KituraNetTest {
func testHeadersContainerHTTPHeaders() {
let headers = HeadersContainer()
headers["Content-Type"] = ["image/png, text/plain"]
XCTAssertEqual(headers.httpHeaders()["Content-Type"], headers["Content-Type"]!)
XCTAssertEqual(headers.nioHeaders["Content-Type"], headers["Content-Type"]!)
headers["Content-Type"] = ["text/html"]
XCTAssertEqual(headers.httpHeaders()["Content-Type"], headers["Content-Type"]!)
XCTAssertEqual(headers.nioHeaders["Content-Type"], headers["Content-Type"]!)
headers["Content-Type"] = nil
XCTAssertFalse(headers.httpHeaders().contains(name: "Content-Type"))
XCTAssertFalse(headers.nioHeaders.contains(name: "Content-Type"))
headers["Set-Cookie"] = ["ID=123BAS; Path=/; Secure; HttpOnly"]
headers.append("Set-Cookie", value: ["ID=KI9H12; Path=/; Secure; HttpOnly"])
XCTAssertEqual(headers["Set-Cookie"]!, headers.httpHeaders()["Set-Cookie"])
XCTAssertEqual(headers["Set-Cookie"]!, headers.nioHeaders["Set-Cookie"])
headers["Content-Type"] = ["text/html"]
headers.append("Content-Type", value: "text/json")
XCTAssertEqual(headers.httpHeaders()["Content-Type"], headers["Content-Type"]!)
XCTAssertEqual(headers.nioHeaders["Content-Type"], headers["Content-Type"]!)
headers["foo"] = ["bar0"]
headers.append("foo", value: "bar1")
XCTAssertEqual(headers.httpHeaders()["foo"], headers["foo"]!)
XCTAssertEqual(headers.nioHeaders["foo"], headers["foo"]!)
headers.append("foo", value: ["bar2", "bar3"])
XCTAssertEqual(headers.httpHeaders()["foo"], headers["foo"]!)
XCTAssertEqual(headers.nioHeaders["foo"], headers["foo"]!)
headers.removeAll()
XCTAssertFalse(headers.httpHeaders().contains(name: "foo"))
XCTAssertFalse(headers.nioHeaders.contains(name: "foo"))
}

func testMultipleWritesToResponse() {
Expand Down
89 changes: 88 additions & 1 deletion Tests/KituraNetTests/MiscellaneousTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ class MiscellaneousTests: KituraNetTest {
static var allTests : [(String, (MiscellaneousTests) -> () throws -> Void)] {
return [
("testEscape", testEscape),
("testHeadersContainers", testHeadersContainers)
("testHeadersContainers", testHeadersContainers),
("testHeadersContainerDualMode", testHeadersContainerDualMode),
]
}

Expand Down Expand Up @@ -63,4 +64,90 @@ class MiscellaneousTests: KituraNetTest {
XCTAssert(foundContentType, "Didn't find the Content-Type header")
XCTAssert(foundSetCookie, "Didn't find the Set-Cookie header")
}

func testHeadersContainerDualMode() {
let headers = HeadersContainer()
headers.append("Foo", value: "Bar")
headers.append("Foo", value: "Baz")
let numValues = headers["Foo"]?.count ?? 0
XCTAssertEqual(numValues, 1, "Foo didn't have one value, as expected. It had \(numValues) values")

let numValues1 = headers[headers.startIndex].value.count
XCTAssertEqual(numValues1, 1, "Foo didn't have one value, as expected. It had \(numValues1) values")

// Special case: cookies
let cookieHeaders = HeadersContainer()
cookieHeaders.append("Set-Cookie", value: "os=Linux")
cookieHeaders.append("Set-Cookie", value: "browser=Safari")
let numValues2 = cookieHeaders["Set-Cookie"]?.count ?? 0
XCTAssertEqual(numValues2, 2, "Set-Cookie didn't have two values, as expected. It had \(numValues2) values")

let numValues3 = cookieHeaders[cookieHeaders.startIndex].value.count
XCTAssertEqual(numValues3, 2, "Set-Cookie didn't have two values, as expected. It had \(numValues3) values")

// Special case: Content-Type
let contentHeaders = HeadersContainer()
contentHeaders.append("Content-Type", value: "application/json")
contentHeaders.append("Content-Type", value: "application/xml")
let numValues4 = contentHeaders["Content-Type"]?.count ?? 0
XCTAssertEqual(numValues4, 1, "Content-Type didn't have one value, as expected. It had \(numValues4) values")
XCTAssertEqual(contentHeaders["Content-Type"]?.first!, "application/json")

let numValues5 = contentHeaders[contentHeaders.startIndex].value.count
XCTAssertEqual(numValues5, 1, "Content-Type didn't have one value, as expected. It had \(numValues5) values")

// Append arrays
headers.append("Foo", value: ["Cons", "Damn"])
let numValues6 = headers["Foo"]?.count ?? 0
XCTAssertEqual(numValues6, 1, "Foo didn't have one value, as expected. It had \(numValues6) values")

cookieHeaders.append("Set-Cookie", value: ["abx=xyz", "def=fgh"])
let numValues7 = cookieHeaders["Set-Cookie"]?.count ?? 0
XCTAssertEqual(numValues7, 4, "Set-Cookie didn't have four values, as expected. It had \(numValues7) values")

// Append arrays to a new container
let headers1 = HeadersContainer()
headers1.append("Foo", value: "Bar")
headers1.append("Foo", value: ["Cons", "Damn"])
let numValues8 = headers1[headers1.startIndex].value.count
XCTAssertEqual(numValues8, 1, "Foo didn't have one value, as expected. It had \(numValues8) values")

let cookieHeaders1 = HeadersContainer()
cookieHeaders1.append("Set-Cookie", value: "os=Linux")
cookieHeaders1.append("Set-Cookie", value: ["browser=Safari", "temp=xyz"])
let numValues9 = cookieHeaders1[cookieHeaders1.startIndex].value.count
XCTAssertEqual(numValues9, 3, "Set-Cookie didn't have three values, as expected. It had \(numValues9) values")

// Set using subscript function before and after mode switch
let headers2 = HeadersContainer()
headers2.append("Foo", value: "Bar")
headers2["Foo"] = ["Baz"]
XCTAssertNotNil(headers2["Foo"])
XCTAssertEqual(headers2["Foo"]!.count, 1)
XCTAssertEqual(headers2["Foo"]!.first!, "Baz")

let numValues10 = headers2[headers2.startIndex].value.count
XCTAssertEqual(numValues10, 1)
headers2["Foo"] = ["Bar", "Baz"]
XCTAssertEqual(headers2["Foo"]!.count, 2)
XCTAssertEqual(headers2["Foo"]!.first!, "Bar")

let cookieHeaders2 = HeadersContainer()
cookieHeaders2.append("Set-Cookie", value: "os=Linux")
cookieHeaders2["Set-Cookie"] = ["browser=Safari", "temp=xyz"]
let numValues11 = cookieHeaders2[cookieHeaders2.startIndex].value.count
XCTAssertEqual(numValues11, 2, "Set-Cookie didn't have two values, as expected. It had \(numValues11) values")

// removeAll
headers2.removeAll()
cookieHeaders2.removeAll()

for _ in headers2 {
XCTFail("Headers2 must be empty!")
}

for _ in cookieHeaders2 {
XCTFail("cookieHeaders2 must be empty!")
}
}
}

0 comments on commit e9ca626

Please sign in to comment.