Permalink
Browse files

Changes to `HTTPHeaders` API to integrate with HTTP/2 `HPACKHeaders` …

…API (#525)

Motivation:

HTTP/2 adds an additional item to a header key/value pair:
its indexability. By default, headers can be inserted into
the HPACK header table index; sometimes this will be denied
(common case is 'content-length' header, which is rarely going
to be the same, and will just cause the table to purge more
often than is really necessary). Additionally, a header may be
marked as not only non-indexable but also non-rewritable by
proxies. HTTPHeaders provides nowhere to store this, so the
HPACKHeaders implementation referenced in
apple/swift-nio-http2#10 was created to add in that capability.

Now, given that we really want the HTTP version to be an
implementation detail, we want to keep the HPACK details hidden,
and would thus be using HTTPHeaders in client APIs. NIOHTTP2
already has a HTTP1-to-HTTP2 channel handler that translates
between the two. Thus it behooves us to have the means to copy
the actual key/value pairs between the two types without making
a round-trip from UTF-8 bytes to UTF-16 Strings. These changes
allow NIOHPACK or NIOHTTP2 to implement that round-trip internally.

Modifications:

- HTTPHeader and HTTPHeaderIndex types are now public.
- HTTPHeaders.buffer and HTTPHeaders.headers properties are now
  public.
- A new static method, HTTPHeaders.createHeaderBlock(buffer:headers:)
  was created to serve as a public means to create a HTTPHeaders from
  a ByteBuffer from outside NIOHTTP1. @Lukasa suggested this should
  be a static method rather than an initializer.

Result:

Nothing in NIOHTTP1 changes in operation. All public types have
documentation comments noting that they are only public for very
specific reasons, and are not intended for general use. Once this
is committed, NIOHPACK & NIOHTTP2 will be able to implement fast
round-trips between HTTPHeaders and HPACKHeaders.
  • Loading branch information...
AlanQuatermain authored and Lukasa committed Aug 9, 2018
1 parent db88f35 commit 5279d73d2549a17297bcffc2431ffbd21a8621e6
@@ -3,6 +3,7 @@ Tomer Doron <tomerd@apple.com>
Max Moiseev <moiseev@apple.com>
Johannes Weiß <johannesweiss@apple.com>
Adam Nemecek <adamnemecek@gmail.com>
Jim Dovey <jimdovey@mac.com> <jdovey@linkedin.com>
<bas@basbroek.nl> <BasThomas@users.noreply.github.com>
<daniel_dunbar@apple.com> <daniel@zuster.org>
<johannesweiss@apple.com> <github@tux4u.de>
@@ -26,6 +26,7 @@ needs to be listed here.
- Helge Heß <helge@alwaysrightinstitute.com>
- Ian Partridge <i.partridge@uk.ibm.com>
- Jason Toffaletti <toffaletti@gmail.com>
- Jim Dovey <jimdovey@mac.com>
- Johannes Weiß <johannesweiss@apple.com>
- John Connolly <connoljo2@gmail.com>
- Karim ElNaggar <karimfarid.naggar@gmail.com>
@@ -368,13 +368,19 @@ public struct HTTPResponseHead: Equatable {
}
/// The Index for a header name or value that points into the underlying `ByteBuffer`.
struct HTTPHeaderIndex {
///
/// - note: This is public to aid in the creation of supplemental HTTP libraries, e.g.
/// NIOHTTP2 and NIOHPACK. It is not intended for general use.
public struct HTTPHeaderIndex {
let start: Int
let length: Int
}
/// Struct which holds name, value pairs.
struct HTTPHeader {
///
/// - note: This is public to aid in the creation of supplemental HTTP libraries, e.g.
/// NIOHTTP2 and NIOHPACK. It is not intended for general use.
public struct HTTPHeader {
let name: HTTPHeaderIndex
let value: HTTPHeaderIndex
}
@@ -494,6 +500,41 @@ public struct HTTPHeaders: CustomStringConvertible {
}
return headersArray.description
}
/// Creates a header block from a pre-filled contiguous string buffer containing a
/// UTF-8 encoded HTTP header block, along with a list of the locations of each
/// name/value pair within the block.
///
/// - note: This is public to aid in the creation of supplemental HTTP libraries, e.g.
/// NIOHTTP2 and NIOHPACK. It is not intended for general use.
///
/// - Parameters:
/// - buffer: A buffer containing UTF-8 encoded HTTP headers.
/// - headers: The locations within `buffer` of the name and value of each header.
/// - Returns: A new `HTTPHeaders` using the provided buffer as storage.
public static func createHeaderBlock(buffer: ByteBuffer, headers: [HTTPHeader]) -> HTTPHeaders {
return HTTPHeaders(buffer: buffer, headers: headers, keepAliveState: KeepAliveState.unknown)
}
/// Provides access to raw UTF-8 storage of the headers in this header block, along with
/// a list of the header strings' indices.
///
/// - note: This is public to aid in the creation of supplemental HTTP libraries, e.g.
/// NIOHTTP2 and NIOHPACK. It is not intended for general use.
///
/// - parameters:
/// - block: A block that will be provided UTF-8 header block information.
/// - buf: A raw `ByteBuffer` containing potentially-contiguous sequences of UTF-8 encoded
/// characters.
/// - locations: An array of `HTTPHeader`s, each of which contains information on the location in
/// the buffer of both a header's name and value.
/// - contiguous: A `Bool` indicating whether the headers are stored contiguously, with no padding
/// or orphaned data within the block. If this is `true`, then the buffer represents
/// a HTTP/1 header block appropriately encoded for the wire.
public func withUnsafeBufferAndIndices<R>(_ block: (_ buf: ByteBuffer, _ locations: [HTTPHeader], _ contiguous: Bool) throws -> R) rethrows -> R {
return try block(self.buffer, self.headers, self.continuous)
}
/// Constructor used by our decoder to construct headers without the need of converting bytes to string.
init(buffer: ByteBuffer, headers: [HTTPHeader], keepAliveState: KeepAliveState) {
@@ -41,6 +41,8 @@ extension HTTPHeadersTest {
("testKeepAliveStateHasClose", testKeepAliveStateHasClose),
("testResolveNonContiguousHeaders", testResolveNonContiguousHeaders),
("testStringBasedHTTPListHeaderIterator", testStringBasedHTTPListHeaderIterator),
("testUnsafeBufferAccess", testUnsafeBufferAccess),
("testCreateFromBufferAndLocations", testCreateFromBufferAndLocations),
]
}
}
@@ -232,5 +232,67 @@ class HTTPHeadersTest : XCTestCase {
XCTAssertEqual(String(decoding: currentToken!, as: UTF8.self), "close")
currentToken = tokenSource.next()
XCTAssertNil(currentToken)
}
func testUnsafeBufferAccess() {
let originalHeaders = [ ("X-Header", "1"),
("X-SomeHeader", "3"),
("X-Header", "2")]
let originalHeadersString = "X-Header: 1\r\nX-SomeHeader: 3\r\nX-Header: 2\r\n"
var headers1 = HTTPHeaders(originalHeaders)
// ensure we can access the underlying buffer and header locations
headers1.withUnsafeBufferAndIndices { (buf, locations, contiguous) in
XCTAssertTrue(contiguous)
XCTAssertEqual(locations.count, 3)
XCTAssertEqual(buf.readableBytes, originalHeadersString.utf8.count) // NB: String considers "\r\n" to be one character
let str = buf.getString(at: 0, length: buf.readableBytes)
XCTAssertEqual(str, originalHeadersString)
}
// remove a header
headers1.remove(name: "X-SomeHeader")
// should no longer be contiguous
headers1.withUnsafeBufferAndIndices { (_, _, contiguous) in
XCTAssertFalse(contiguous)
}
}
func testCreateFromBufferAndLocations() {
let originalHeaders = [ ("User-Agent", "1"),
("host", "2"),
("X-SOMETHING", "3"),
("X-Something", "4"),
("SET-COOKIE", "foo=bar"),
("Set-Cookie", "buz=cux")]
// create our own buffer and location list
var buf = ByteBufferAllocator().buffer(capacity: 128)
var locations: [HTTPHeader] = []
for (name, value) in originalHeaders {
let nstart = buf.writerIndex
buf.write(string: name)
let nameLoc = HTTPHeaderIndex(start: nstart, length: buf.writerIndex - nstart)
buf.write(string: ": ")
let vstart = buf.writerIndex
buf.write(string: value)
let valueLoc = HTTPHeaderIndex(start: vstart, length: buf.writerIndex - vstart)
buf.write(string: "\r\n")
locations.append(HTTPHeader(name: nameLoc, value: valueLoc))
}
// create HTTP headers
let headers = HTTPHeaders.createHeaderBlock(buffer: buf, headers: locations)
// looking up headers value is case-insensitive
XCTAssertEqual(["1"], headers["User-Agent"])
XCTAssertEqual(["1"], headers["User-agent"])
XCTAssertEqual(["2"], headers["Host"])
XCTAssertEqual(["3", "4"], headers["X-Something"])
XCTAssertEqual(["foo=bar", "buz=cux"], headers["set-cookie"])
}
}

0 comments on commit 5279d73

Please sign in to comment.