Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement a very lazy translation of headers #117

Merged
merged 1 commit into from
Jan 2, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 = httpHeaders else {
return HeadersContainer()
}
return HeadersContainer.create(from: httpHeaders)
return HeadersContainer(with: httpHeaders)
}

set {
httpHeaders = newValue.httpHeaders()
httpHeaders = 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 @@ -115,7 +115,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
133 changes: 86 additions & 47 deletions Sources/KituraNet/HTTP/HeadersContainer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,25 @@ 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,9 +58,16 @@ public class HeadersContainer {

set(newValue) {
if let newValue = newValue {
set(key, value: newValue)
} else {
remove(key)
nioHeaders.replace(name: key, values: newValue)
if mode == .dual {
set(key, value: newValue)
}
}
else {
nioHeaders.remove(name: key)
if mode == .dual {
remove(key)
}
}
}
}
Expand All @@ -54,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]

switch lowerCaseKey {
var entry = nioHeaders[key]
switch(lowerCaseKey) {

case "set-cookie":
if entry != nil {
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 entry != nil {
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 @@ -96,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 @@ -115,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 @@ -124,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 @@ -148,37 +205,19 @@ extension HeadersContainer: Collection {
///
/// - Returns: A (key, value) tuple.
public subscript(position: Index) -> (key: String, value: [String]) {
get {
checkAndSwitchToDualMode()
return headers[position].value
}
}

/// Get the next Index in the `HeadersContainer` collection after the one specified.
///
/// - Parameter after: The Index whose successor is to be returned.
///
/// - Returns: The Index in the `HeadersContainer` collection after the one specified.
public func index(after index: Index) -> Index {
return headers.index(after: index)
}
}

/// 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
public func index(after i: Index) -> Index {
checkAndSwitchToDualMode()
return headers.index(after: i)
}
}
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!")
}
}
}