diff --git a/Sources/AsyncHTTPClient/HTTPClient.swift b/Sources/AsyncHTTPClient/HTTPClient.swift index ca2343045..5fee5e0a1 100644 --- a/Sources/AsyncHTTPClient/HTTPClient.swift +++ b/Sources/AsyncHTTPClient/HTTPClient.swift @@ -967,6 +967,7 @@ public struct HTTPClientError: Error, Equatable, CustomStringConvertible { case traceRequestWithBody case invalidHeaderFieldNames([String]) case bodyLengthMismatch + case incompatibleHeaders } private var code: Code @@ -1019,4 +1020,6 @@ public struct HTTPClientError: Error, Equatable, CustomStringConvertible { public static func invalidHeaderFieldNames(_ names: [String]) -> HTTPClientError { return HTTPClientError(code: .invalidHeaderFieldNames(names)) } /// Body length is not equal to `Content-Length`. public static let bodyLengthMismatch = HTTPClientError(code: .bodyLengthMismatch) + /// Incompatible headers specified, for example `Transfer-Encoding` and `Content-Length`. + public static let incompatibleHeaders = HTTPClientError(code: .incompatibleHeaders) } diff --git a/Sources/AsyncHTTPClient/RequestValidation.swift b/Sources/AsyncHTTPClient/RequestValidation.swift index e5061a99a..a9c1678f4 100644 --- a/Sources/AsyncHTTPClient/RequestValidation.swift +++ b/Sources/AsyncHTTPClient/RequestValidation.swift @@ -18,6 +18,10 @@ import NIOHTTP1 extension HTTPHeaders { mutating func validate(method: HTTPMethod, body: HTTPClient.Body?) throws { // validate transfer encoding and content length (https://tools.ietf.org/html/rfc7230#section-3.3.1) + if self.contains(name: "Transfer-Encoding"), self.contains(name: "Content-Length") { + throw HTTPClientError.incompatibleHeaders + } + var transferEncoding: String? var contentLength: Int? let encodings = self[canonicalForm: "Transfer-Encoding"].map { $0.lowercased() } @@ -27,11 +31,11 @@ extension HTTPHeaders { } self.remove(name: "Transfer-Encoding") - self.remove(name: "Content-Length") try self.validateFieldNames() guard let body = body else { + self.remove(name: "Content-Length") // if we don't have a body we might not need to send the Content-Length field // https://tools.ietf.org/html/rfc7230#section-3.3.2 switch method { @@ -60,11 +64,15 @@ extension HTTPHeaders { } if encodings.isEmpty { - guard let length = body.length else { - throw HTTPClientError.contentLengthMissing + if let length = body.length { + self.remove(name: "Content-Length") + contentLength = length + } else if !self.contains(name: "Content-Length") { + transferEncoding = "chunked" } - contentLength = length } else { + self.remove(name: "Content-Length") + transferEncoding = encodings.joined(separator: ", ") if !encodings.contains("chunked") { guard let length = body.length else { diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift index 0de4d0c07..5fe7b2417 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift @@ -114,6 +114,7 @@ extension HTTPClientTests { ("testNothingIsLoggedAtInfoOrHigher", testNothingIsLoggedAtInfoOrHigher), ("testAllMethodsLog", testAllMethodsLog), ("testClosingIdleConnectionsInPoolLogsInTheBackground", testClosingIdleConnectionsInPoolLogsInTheBackground), + ("testUploadStreamingNoLength", testUploadStreamingNoLength), ("testConnectErrorPropagatedToDelegate", testConnectErrorPropagatedToDelegate), ("testDelegateCallinsTolerateRandomEL", testDelegateCallinsTolerateRandomEL), ("testContentLengthTooLongFails", testContentLengthTooLongFails), diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index eaf4c6564..df144b70b 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -2374,6 +2374,37 @@ class HTTPClientTests: XCTestCase { self.defaultClient = nil // so it doesn't get shut down again. } + func testUploadStreamingNoLength() throws { + let server = NIOHTTP1TestServer(group: self.serverGroup) + let client = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup)) + defer { + XCTAssertNoThrow(try client.syncShutdown()) + XCTAssertNoThrow(try server.stop()) + } + + var request = try HTTPClient.Request(url: "http://localhost:\(server.serverPort)/") + request.body = .stream { writer in + writer.write(.byteBuffer(ByteBuffer(string: "1234"))) + } + + let future = client.execute(request: request) + + switch try server.readInbound() { + case .head(let head): + XCTAssertEqual(head.headers["transfer-encoding"], ["chunked"]) + default: + XCTFail("Unexpected part") + } + + XCTAssertNoThrow(try server.readInbound()) // .body + XCTAssertNoThrow(try server.readInbound()) // .end + + XCTAssertNoThrow(try server.writeOutbound(.head(.init(version: .init(major: 1, minor: 1), status: .ok)))) + XCTAssertNoThrow(try server.writeOutbound(.end(nil))) + + XCTAssertNoThrow(try future.wait()) + } + func testConnectErrorPropagatedToDelegate() throws { class TestDelegate: HTTPClientResponseDelegate { typealias Response = Void diff --git a/Tests/AsyncHTTPClientTests/RequestValidationTests+XCTest.swift b/Tests/AsyncHTTPClientTests/RequestValidationTests+XCTest.swift index 51e6ff016..08d4cfb32 100644 --- a/Tests/AsyncHTTPClientTests/RequestValidationTests+XCTest.swift +++ b/Tests/AsyncHTTPClientTests/RequestValidationTests+XCTest.swift @@ -28,12 +28,18 @@ extension RequestValidationTests { ("testContentLengthHeaderIsRemovedFromGETIfNoBody", testContentLengthHeaderIsRemovedFromGETIfNoBody), ("testContentLengthHeaderIsAddedToPOSTAndPUTWithNoBody", testContentLengthHeaderIsAddedToPOSTAndPUTWithNoBody), ("testContentLengthHeaderIsChangedIfBodyHasDifferentLength", testContentLengthHeaderIsChangedIfBodyHasDifferentLength), - ("testChunkedEncodingDoesNotHaveContentLengthHeader", testChunkedEncodingDoesNotHaveContentLengthHeader), ("testTRACERequestMustNotHaveBody", testTRACERequestMustNotHaveBody), ("testGET_HEAD_DELETE_CONNECTRequestCanHaveBody", testGET_HEAD_DELETE_CONNECTRequestCanHaveBody), ("testInvalidHeaderFieldNames", testInvalidHeaderFieldNames), ("testValidHeaderFieldNames", testValidHeaderFieldNames), - ("testMultipleContentLengthOnNilStreamLength", testMultipleContentLengthOnNilStreamLength), + ("testNoHeadersNoBody", testNoHeadersNoBody), + ("testNoHeadersHasBody", testNoHeadersHasBody), + ("testContentLengthHeaderNoBody", testContentLengthHeaderNoBody), + ("testContentLengthHeaderHasBody", testContentLengthHeaderHasBody), + ("testTransferEncodingHeaderNoBody", testTransferEncodingHeaderNoBody), + ("testTransferEncodingHeaderHasBody", testTransferEncodingHeaderHasBody), + ("testBothHeadersNoBody", testBothHeadersNoBody), + ("testBothHeadersHasBody", testBothHeadersHasBody), ] } } diff --git a/Tests/AsyncHTTPClientTests/RequestValidationTests.swift b/Tests/AsyncHTTPClientTests/RequestValidationTests.swift index fe3cb9330..3e34f905c 100644 --- a/Tests/AsyncHTTPClientTests/RequestValidationTests.swift +++ b/Tests/AsyncHTTPClientTests/RequestValidationTests.swift @@ -42,32 +42,14 @@ class RequestValidationTests: XCTestCase { XCTAssertEqual(headers.first(name: "Content-Length"), "200") } - func testChunkedEncodingDoesNotHaveContentLengthHeader() { - var headers = HTTPHeaders([ - ("Content-Length", "200"), - ("Transfer-Encoding", "chunked"), - ]) - var buffer = ByteBufferAllocator().buffer(capacity: 200) - buffer.writeBytes([UInt8](repeating: 12, count: 200)) - XCTAssertNoThrow(try headers.validate(method: .PUT, body: .byteBuffer(buffer))) - - // https://tools.ietf.org/html/rfc7230#section-3.3.2 - // A sender MUST NOT send a Content-Length header field in any message - // that contains a Transfer-Encoding header field. - - XCTAssertNil(headers.first(name: "Content-Length")) - XCTAssertEqual(headers.first(name: "Transfer-Encoding"), "chunked") - } - func testTRACERequestMustNotHaveBody() { - var headers = HTTPHeaders([ - ("Content-Length", "200"), - ("Transfer-Encoding", "chunked"), - ]) - var buffer = ByteBufferAllocator().buffer(capacity: 200) - buffer.writeBytes([UInt8](repeating: 12, count: 200)) - XCTAssertThrowsError(try headers.validate(method: .TRACE, body: .byteBuffer(buffer))) { - XCTAssertEqual($0 as? HTTPClientError, .traceRequestWithBody) + for header in [("Content-Length", "200"), ("Transfer-Encoding", "chunked")] { + var headers = HTTPHeaders([header]) + var buffer = ByteBufferAllocator().buffer(capacity: 200) + buffer.writeBytes([UInt8](repeating: 12, count: 200)) + XCTAssertThrowsError(try headers.validate(method: .TRACE, body: .byteBuffer(buffer))) { + XCTAssertEqual($0 as? HTTPClientError, .traceRequestWithBody) + } } } @@ -105,13 +87,181 @@ class RequestValidationTests: XCTestCase { XCTAssertNoThrow(try headers.validate(method: .GET, body: nil)) } - func testMultipleContentLengthOnNilStreamLength() { - var headers = HTTPHeaders([("Content-Length", "1"), ("Content-Length", "2")]) - var buffer = ByteBufferAllocator().buffer(capacity: 10) - buffer.writeBytes([UInt8](repeating: 12, count: 10)) - let body: HTTPClient.Body = .stream { writer in - writer.write(.byteBuffer(buffer)) + // MARK: - Content-Length/Transfer-Encoding Matrix + + // Method kind User sets Body Expectation + // ---------------------------------------------------------------------------------- + // .GET, .HEAD, .DELETE, .CONNECT, .TRACE nothing nil Neither CL nor chunked + // other nothing nil CL=0 + func testNoHeadersNoBody() throws { + for method: HTTPMethod in [.GET, .HEAD, .DELETE, .CONNECT, .TRACE] { + var headers: HTTPHeaders = .init() + XCTAssertNoThrow(try headers.validate(method: method, body: nil)) + XCTAssertTrue(headers["content-length"].isEmpty) + XCTAssertTrue(headers["transfer-encoding"].isEmpty) + } + + for method: HTTPMethod in [.POST, .PUT] { + var headers: HTTPHeaders = .init() + XCTAssertNoThrow(try headers.validate(method: method, body: nil)) + XCTAssertEqual(headers["content-length"].first, "0") + XCTAssertFalse(headers["transfer-encoding"].contains("chunked")) + } + } + + // Method kind User sets Body Expectation + // -------------------------------------------------------------------------------------- + // .GET, .HEAD, .DELETE, .CONNECT, .TRACE nothing not nil CL or chunked + // other nothing not nil CL or chunked + func testNoHeadersHasBody() throws { + // Body length is known + for method: HTTPMethod in [.GET, .HEAD, .DELETE, .CONNECT] { + var headers: HTTPHeaders = .init() + XCTAssertNoThrow(try headers.validate(method: method, body: .byteBuffer(ByteBuffer(bytes: [0])))) + XCTAssertEqual(headers["content-length"].first, "1") + XCTAssertTrue(headers["transfer-encoding"].isEmpty) + } + + // Body length is _not_ known + for method: HTTPMethod in [.GET, .HEAD, .DELETE, .CONNECT] { + var headers: HTTPHeaders = .init() + let body: HTTPClient.Body = .stream { writer in + writer.write(.byteBuffer(ByteBuffer(bytes: [0]))) + } + XCTAssertNoThrow(try headers.validate(method: method, body: body)) + XCTAssertTrue(headers["content-length"].isEmpty) + XCTAssertTrue(headers["transfer-encoding"].contains("chunked")) + } + + // Body length is known + for method: HTTPMethod in [.POST, .PUT] { + var headers: HTTPHeaders = .init() + XCTAssertNoThrow(try headers.validate(method: method, body: .byteBuffer(ByteBuffer(bytes: [0])))) + XCTAssertEqual(headers["content-length"].first, "1") + XCTAssertTrue(headers["transfer-encoding"].isEmpty) + } + + // Body length is _not_ known + for method: HTTPMethod in [.POST, .PUT] { + var headers: HTTPHeaders = .init() + let body: HTTPClient.Body = .stream { writer in + writer.write(.byteBuffer(ByteBuffer(bytes: [0]))) + } + XCTAssertNoThrow(try headers.validate(method: method, body: body)) + XCTAssertTrue(headers["content-length"].isEmpty) + XCTAssertTrue(headers["transfer-encoding"].contains("chunked")) + } + } + + // Method kind User sets Body Expectation + // ------------------------------------------------------------------------------ + // .GET, .HEAD, .DELETE, .CONNECT, .TRACE content-length nil Neither CL nor chunked + // other content-length nil CL=0 + func testContentLengthHeaderNoBody() throws { + for method: HTTPMethod in [.GET, .HEAD, .DELETE, .CONNECT, .TRACE] { + var headers: HTTPHeaders = .init([("Content-Length", "1")]) + XCTAssertNoThrow(try headers.validate(method: method, body: nil)) + XCTAssertTrue(headers["content-length"].isEmpty) + XCTAssertTrue(headers["transfer-encoding"].isEmpty) + } + + for method: HTTPMethod in [.POST, .PUT] { + var headers: HTTPHeaders = .init([("Content-Length", "1")]) + XCTAssertNoThrow(try headers.validate(method: method, body: nil)) + XCTAssertEqual(headers["content-length"].first, "0") + XCTAssertTrue(headers["transfer-encoding"].isEmpty) + } + } + + // Method kind User sets Body Expectation + // -------------------------------------------------------------------------- + // .GET, .HEAD, .DELETE, .CONNECT content-length not nil CL=1 + // other content-length nit nil CL=1 + func testContentLengthHeaderHasBody() throws { + for method: HTTPMethod in [.GET, .HEAD, .DELETE, .CONNECT] { + var headers: HTTPHeaders = .init([("Content-Length", "1")]) + XCTAssertNoThrow(try headers.validate(method: method, body: .byteBuffer(ByteBuffer(bytes: [0])))) + XCTAssertEqual(headers["content-length"].first, "1") + XCTAssertTrue(headers["transfer-encoding"].isEmpty) + } + + for method: HTTPMethod in [.POST, .PUT] { + var headers: HTTPHeaders = .init([("Content-Length", "1")]) + XCTAssertNoThrow(try headers.validate(method: method, body: .byteBuffer(ByteBuffer(bytes: [0])))) + XCTAssertEqual(headers["content-length"].first, "1") + XCTAssertTrue(headers["transfer-encoding"].isEmpty) + } + } + + // Method kind User sets Body Expectation + // ------------------------------------------------------------------------------------------ + // .GET, .HEAD, .DELETE, .CONNECT, .TRACE transfer-encoding: chunked nil nil + // other transfer-encoding: chunked nil nil + func testTransferEncodingHeaderNoBody() throws { + for method: HTTPMethod in [.GET, .HEAD, .DELETE, .CONNECT, .TRACE] { + var headers: HTTPHeaders = .init([("Transfer-Encoding", "chunked")]) + XCTAssertNoThrow(try headers.validate(method: method, body: nil)) + XCTAssertTrue(headers["content-length"].isEmpty) + XCTAssertFalse(headers["transfer-encoding"].contains("chunked")) + } + + for method: HTTPMethod in [.POST, .PUT] { + var headers: HTTPHeaders = .init([("Transfer-Encoding", "chunked")]) + XCTAssertNoThrow(try headers.validate(method: method, body: nil)) + XCTAssertEqual(headers["content-length"].first, "0") + XCTAssertFalse(headers["transfer-encoding"].contains("chunked")) + } + } + + // Method kind User sets Body Expectation + // -------------------------------------------------------------------------------------- + // .GET, .HEAD, .DELETE, .CONNECT transfer-encoding: chunked not nil chunked + // other transfer-encoding: chunked not nil chunked + func testTransferEncodingHeaderHasBody() throws { + for method: HTTPMethod in [.GET, .HEAD, .DELETE, .CONNECT] { + var headers: HTTPHeaders = .init([("Transfer-Encoding", "chunked")]) + XCTAssertNoThrow(try headers.validate(method: method, body: .byteBuffer(ByteBuffer(bytes: [0])))) + XCTAssertTrue(headers["content-length"].isEmpty) + XCTAssertTrue(headers["transfer-encoding"].contains("chunked")) + } + + for method: HTTPMethod in [.POST, .PUT] { + var headers: HTTPHeaders = .init([("Transfer-Encoding", "chunked")]) + XCTAssertNoThrow(try headers.validate(method: method, body: .byteBuffer(ByteBuffer(bytes: [0])))) + XCTAssertTrue(headers["content-length"].isEmpty) + XCTAssertTrue(headers["transfer-encoding"].contains("chunked")) + } + } + + // Method kind User sets Body Expectation + // --------------------------------------------------------------------------------------- + // .GET, .HEAD, .DELETE, .CONNECT, .TRACE CL & chunked (illegal) nil throws error + // other CL & chunked (illegal) nil throws error + func testBothHeadersNoBody() throws { + for method: HTTPMethod in [.GET, .HEAD, .DELETE, .CONNECT, .TRACE] { + var headers: HTTPHeaders = .init([("Content-Length", "1"), ("Transfer-Encoding", "chunked")]) + XCTAssertThrowsError(try headers.validate(method: method, body: nil)) + } + + for method: HTTPMethod in [.POST, .PUT] { + var headers: HTTPHeaders = .init([("Content-Length", "1"), ("Transfer-Encoding", "chunked")]) + XCTAssertThrowsError(try headers.validate(method: method, body: nil)) + } + } + + // Method kind User sets Body Expectation + // ------------------------------------------------------------------------------------------- + // .GET, .HEAD, .DELETE, .CONNECT, .TRACE CL & chunked (illegal) not nil throws error + // other CL & chunked (illegal) not nil throws error + func testBothHeadersHasBody() throws { + for method: HTTPMethod in [.GET, .HEAD, .DELETE, .CONNECT, .TRACE] { + var headers: HTTPHeaders = .init([("Content-Length", "1"), ("Transfer-Encoding", "chunked")]) + XCTAssertThrowsError(try headers.validate(method: method, body: .byteBuffer(ByteBuffer(bytes: [0])))) + } + + for method: HTTPMethod in [.POST, .PUT] { + var headers: HTTPHeaders = .init([("Content-Length", "1"), ("Transfer-Encoding", "chunked")]) + XCTAssertThrowsError(try headers.validate(method: method, body: .byteBuffer(ByteBuffer(bytes: [0])))) } - XCTAssertThrowsError(try headers.validate(method: .PUT, body: body)) } }