From 185dc1dc4cb43b1f038b0ceaca056ab796569d85 Mon Sep 17 00:00:00 2001 From: David Nadoba Date: Thu, 18 Nov 2021 20:54:41 +0100 Subject: [PATCH 1/6] refactor request validation --- .../ConnectionPool/RequestBodyLength.swift | 20 +++ Sources/AsyncHTTPClient/HTTPClient.swift | 3 + Sources/AsyncHTTPClient/HTTPHandler.swift | 19 ++- .../AsyncHTTPClient/RequestValidation.swift | 135 ++++++++++-------- .../RequestValidationTests+XCTest.swift | 5 + .../RequestValidationTests.swift | 128 +++++++++++------ 6 files changed, 206 insertions(+), 104 deletions(-) create mode 100644 Sources/AsyncHTTPClient/ConnectionPool/RequestBodyLength.swift diff --git a/Sources/AsyncHTTPClient/ConnectionPool/RequestBodyLength.swift b/Sources/AsyncHTTPClient/ConnectionPool/RequestBodyLength.swift new file mode 100644 index 000000000..9905070c3 --- /dev/null +++ b/Sources/AsyncHTTPClient/ConnectionPool/RequestBodyLength.swift @@ -0,0 +1,20 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the AsyncHTTPClient open source project +// +// Copyright (c) 2021 Apple Inc. and the AsyncHTTPClient project authors +// Licensed under Apache License v2.0 +// +// See LICENSE.txt for license information +// See CONTRIBUTORS.txt for the list of AsyncHTTPClient project authors +// +// SPDX-License-Identifier: Apache-2.0 +// +//===----------------------------------------------------------------------===// + +enum RequestBodyLength: Equatable { + /// size of the reuqest body is not known before starting the request + case dynamic + /// size of the request body is fixed and exactly `length` bytes + case fixed(length: Int) +} diff --git a/Sources/AsyncHTTPClient/HTTPClient.swift b/Sources/AsyncHTTPClient/HTTPClient.swift index 088329513..4a181e388 100644 --- a/Sources/AsyncHTTPClient/HTTPClient.swift +++ b/Sources/AsyncHTTPClient/HTTPClient.swift @@ -884,6 +884,7 @@ public struct HTTPClientError: Error, Equatable, CustomStringConvertible { case cancelled case identityCodingIncorrectlyPresent case chunkedSpecifiedMultipleTimes + case transferEncodingSpecifiedButChunkedIsNotTheFinalEncoding case invalidProxyResponse case contentLengthMissing case proxyAuthenticationRequired @@ -938,6 +939,8 @@ public struct HTTPClientError: Error, Equatable, CustomStringConvertible { public static let identityCodingIncorrectlyPresent = HTTPClientError(code: .identityCodingIncorrectlyPresent) /// Request contains multiple chunks definitions. public static let chunkedSpecifiedMultipleTimes = HTTPClientError(code: .chunkedSpecifiedMultipleTimes) + /// Request specifies `Transfer-Encoding` but `chunked` is not the final encoding + public static let transferEncodingSpecifiedButChunkedIsNotTheFinalEncoding = HTTPClientError(code: .transferEncodingSpecifiedButChunkedIsNotTheFinalEncoding) /// Proxy response was invalid. public static let invalidProxyResponse = HTTPClientError(code: .invalidProxyResponse) /// Request does not contain `Content-Length` header. diff --git a/Sources/AsyncHTTPClient/HTTPHandler.swift b/Sources/AsyncHTTPClient/HTTPHandler.swift index 31f5e9337..8fb3003fe 100644 --- a/Sources/AsyncHTTPClient/HTTPHandler.swift +++ b/Sources/AsyncHTTPClient/HTTPHandler.swift @@ -43,8 +43,7 @@ extension HTTPClient { } } - /// Body size. Request validation will be failed with `HTTPClientErrors.contentLengthMissing` if nil, - /// unless `Trasfer-Encoding: chunked` header is set. + /// Body size. if nil and `Transfer-Encoding` header is **not** set, `Transfer-Encoding` will be automatically be set to `chunked` public var length: Int? /// Body chunk provider. public var stream: (StreamWriter) -> EventLoopFuture @@ -309,7 +308,7 @@ extension HTTPClient { head.headers.add(name: "host", value: host) } - let metadata = try head.headers.validate(method: self.method, body: self.body) + let metadata = try head.headers.validateAndFixTransportFraming(method: self.method, bodyLength: .init(self.body)) return (head, metadata) } @@ -820,3 +819,17 @@ internal struct RedirectHandler { } } } + +extension RequestBodyLength { + init(_ body: HTTPClient.Body?) { + guard let body = body else { + self = .fixed(length: 0) + return + } + guard let length = body.length else { + self = .dynamic + return + } + self = .fixed(length: length) + } +} diff --git a/Sources/AsyncHTTPClient/RequestValidation.swift b/Sources/AsyncHTTPClient/RequestValidation.swift index aec694fd5..97909e9b3 100644 --- a/Sources/AsyncHTTPClient/RequestValidation.swift +++ b/Sources/AsyncHTTPClient/RequestValidation.swift @@ -16,32 +16,32 @@ import NIOCore import NIOHTTP1 extension HTTPHeaders { - mutating func validate(method: HTTPMethod, body: HTTPClient.Body?) throws -> RequestFramingMetadata { - var metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(0)) - - if self[canonicalForm: "connection"].lazy.map({ $0.lowercased() }).contains("close") { - metadata.connectionClose = true - } - - // 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") { + mutating func validateAndFixTransportFraming( + method: HTTPMethod, + bodyLength: RequestBodyLength + ) throws -> RequestFramingMetadata { + let contentLength = self.first(name: "Content-Length") + let encodings = self[canonicalForm: "Transfer-Encoding"] + + // "Transfer-Encoding" and "Content-Length" are not allowed to present at the same time (https://tools.ietf.org/html/rfc7230#section-3.3.1) + guard encodings.isEmpty || contentLength == nil else { throw HTTPClientError.incompatibleHeaders } - var transferEncoding: String? - var contentLength: Int? - let encodings = self[canonicalForm: "Transfer-Encoding"].map { $0.lowercased() } + try self.validateFieldNames() + try Self.validateTransferEncoding(encodings) - guard !encodings.contains("identity") else { - throw HTTPClientError.identityCodingIncorrectlyPresent + if contentLength != nil { + self.remove(name: "Content-Length") + } + if !encodings.isEmpty { + self.remove(name: "Transfer-Encoding") } - self.remove(name: "Transfer-Encoding") - - try self.validateFieldNames() + let connectionClose = self[canonicalForm: "connection"].lazy.map { $0.lowercased() }.contains("close") - guard let body = body else { - self.remove(name: "Content-Length") + switch bodyLength { + case .fixed(0): // 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 { @@ -49,60 +49,46 @@ extension HTTPHeaders { // A user agent SHOULD NOT send a Content-Length header field when the request // message does not contain a payload body and the method semantics do not // anticipate such a body. - return metadata + break default: // A user agent SHOULD send a Content-Length in a request message when // no Transfer-Encoding is sent and the request method defines a meaning // for an enclosed payload body. self.add(name: "Content-Length", value: "0") - return metadata } - } - - if case .TRACE = method { - // A client MUST NOT send a message body in a TRACE request. - // https://tools.ietf.org/html/rfc7230#section-4.3.8 - throw HTTPClientError.traceRequestWithBody - } - - guard (encodings.lazy.filter { $0 == "chunked" }.count <= 1) else { - throw HTTPClientError.chunkedSpecifiedMultipleTimes - } - - if encodings.isEmpty { - if let length = body.length { - self.remove(name: "Content-Length") - contentLength = length - } else if !self.contains(name: "Content-Length") { - transferEncoding = "chunked" + return .init(connectionClose: connectionClose, body: .fixedSize(0)) + case .fixed(let length): + if case .TRACE = method { + // A client MUST NOT send a message body in a TRACE request. + // https://tools.ietf.org/html/rfc7230#section-4.3.8 + throw HTTPClientError.traceRequestWithBody + } + if encodings.isEmpty { + self.add(name: "Content-Length", value: String(length)) + return .init(connectionClose: connectionClose, body: .fixedSize(length)) + } else { + self.add(name: "Transfer-Encoding", value: encodings.joined(separator: ", ")) + return .init(connectionClose: connectionClose, body: .stream) + } + case .dynamic: + if case .TRACE = method { + // A client MUST NOT send a message body in a TRACE request. + // https://tools.ietf.org/html/rfc7230#section-4.3.8 + throw HTTPClientError.traceRequestWithBody } - } else { - self.remove(name: "Content-Length") - transferEncoding = encodings.joined(separator: ", ") - if !encodings.contains("chunked") { - guard let length = body.length else { - throw HTTPClientError.contentLengthMissing - } - contentLength = length + if encodings.isEmpty && contentLength == nil { + // if a user forgot to specify a Content-Length and Transfer-Encoding, we will set it for them + self.add(name: "Transfer-Encoding", value: "chunked") + } else { + self.add(name: "Transfer-Encoding", value: encodings.joined(separator: ", ")) } - } - // add headers if required - if let enc = transferEncoding { - self.add(name: "Transfer-Encoding", value: enc) - metadata.body = .stream - } else if let length = contentLength { - // A sender MUST NOT send a Content-Length header field in any message - // that contains a Transfer-Encoding header field. - self.add(name: "Content-Length", value: String(length)) - metadata.body = .fixedSize(length) + return .init(connectionClose: connectionClose, body: .stream) } - - return metadata } - private func validateFieldNames() throws { + func validateFieldNames() throws { let invalidFieldNames = self.compactMap { (name, _) -> String? in let satisfy = name.utf8.allSatisfy { (char) -> Bool in switch char { @@ -137,4 +123,33 @@ extension HTTPHeaders { throw HTTPClientError.invalidHeaderFieldNames(invalidFieldNames) } } + + static func validateTransferEncoding( + _ encodings: Encodings + ) throws where Encodings: Sequence, Encodings.Element: StringProtocol { + let encodings = encodings.map { $0.lowercased() } + + guard !encodings.contains("identity") else { + throw HTTPClientError.identityCodingIncorrectlyPresent + } + + // If `Transfer-Encoding` is specified, `chunked` needs to be the last encoding and should not be specified multiple times + // https://datatracker.ietf.org/doc/html/rfc7230#section-3.3.1 + let chunkedEncodingCount = encodings.lazy.filter { $0 == "chunked" }.count + switch chunkedEncodingCount { + case 0: + if !encodings.isEmpty { + throw HTTPClientError.transferEncodingSpecifiedButChunkedIsNotTheFinalEncoding + } + case 1: + guard encodings.last == "chunked" else { + throw HTTPClientError.transferEncodingSpecifiedButChunkedIsNotTheFinalEncoding + } + case 2...: + throw HTTPClientError.chunkedSpecifiedMultipleTimes + default: + // unreachable because `chunkedEncodingCount` is guaranteed to be positive + preconditionFailure() + } + } } diff --git a/Tests/AsyncHTTPClientTests/RequestValidationTests+XCTest.swift b/Tests/AsyncHTTPClientTests/RequestValidationTests+XCTest.swift index 839a8dcc2..b56cb0790 100644 --- a/Tests/AsyncHTTPClientTests/RequestValidationTests+XCTest.swift +++ b/Tests/AsyncHTTPClientTests/RequestValidationTests+XCTest.swift @@ -43,6 +43,11 @@ extension RequestValidationTests { ("testBothHeadersNoBody", testBothHeadersNoBody), ("testBothHeadersHasBody", testBothHeadersHasBody), ("testHostHeaderIsSetCorrectlyInCreateRequestHead", testHostHeaderIsSetCorrectlyInCreateRequestHead), + ("testTraceMethodIsNotAllowedToHaveAFixedLengthBody", testTraceMethodIsNotAllowedToHaveAFixedLengthBody), + ("testTraceMethodIsNotAllowedToHaveADynamicLengthBody", testTraceMethodIsNotAllowedToHaveADynamicLengthBody), + ("testTransferEncodingsArePreservedIfBodyLengthIsFixed", testTransferEncodingsArePreservedIfBodyLengthIsFixed), + ("testTransferEncodingsArePreservedIfBodyLengthIsDynamic", testTransferEncodingsArePreservedIfBodyLengthIsDynamic), + ("testTransferEncodingValidation", testTransferEncodingValidation), ] } } diff --git a/Tests/AsyncHTTPClientTests/RequestValidationTests.swift b/Tests/AsyncHTTPClientTests/RequestValidationTests.swift index 18d1f5e50..02678a453 100644 --- a/Tests/AsyncHTTPClientTests/RequestValidationTests.swift +++ b/Tests/AsyncHTTPClientTests/RequestValidationTests.swift @@ -21,7 +21,7 @@ class RequestValidationTests: XCTestCase { func testContentLengthHeaderIsRemovedFromGETIfNoBody() { var headers = HTTPHeaders([("Content-Length", "0")]) var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validate(method: .GET, body: .none)) + XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: .GET, bodyLength: .fixed(length: 0))) XCTAssertNil(headers.first(name: "Content-Length")) XCTAssertEqual(metadata?.body, .fixedSize(0)) } @@ -29,23 +29,21 @@ class RequestValidationTests: XCTestCase { func testContentLengthHeaderIsAddedToPOSTAndPUTWithNoBody() { var putHeaders = HTTPHeaders() var putMetadata: RequestFramingMetadata? - XCTAssertNoThrow(putMetadata = try putHeaders.validate(method: .PUT, body: .none)) + XCTAssertNoThrow(putMetadata = try putHeaders.validateAndFixTransportFraming(method: .PUT, bodyLength: .fixed(length: 0))) XCTAssertEqual(putHeaders.first(name: "Content-Length"), "0") XCTAssertEqual(putMetadata?.body, .fixedSize(0)) var postHeaders = HTTPHeaders() var postMetadata: RequestFramingMetadata? - XCTAssertNoThrow(postMetadata = try postHeaders.validate(method: .POST, body: .none)) + XCTAssertNoThrow(postMetadata = try postHeaders.validateAndFixTransportFraming(method: .POST, bodyLength: .fixed(length: 0))) XCTAssertEqual(postHeaders.first(name: "Content-Length"), "0") XCTAssertEqual(postMetadata?.body, .fixedSize(0)) } func testContentLengthHeaderIsChangedIfBodyHasDifferentLength() { var headers = HTTPHeaders([("Content-Length", "0")]) - var buffer = ByteBufferAllocator().buffer(capacity: 200) - buffer.writeBytes([UInt8](repeating: 12, count: 200)) var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validate(method: .PUT, body: .byteBuffer(buffer))) + XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: .PUT, bodyLength: .fixed(length: 200))) XCTAssertEqual(headers.first(name: "Content-Length"), "200") XCTAssertEqual(metadata?.body, .fixedSize(200)) } @@ -53,23 +51,18 @@ class RequestValidationTests: XCTestCase { func testTRACERequestMustNotHaveBody() { 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))) { + XCTAssertThrowsError(try headers.validateAndFixTransportFraming(method: .TRACE, bodyLength: .fixed(length: 200))) { XCTAssertEqual($0 as? HTTPClientError, .traceRequestWithBody) } } } func testGET_HEAD_DELETE_CONNECTRequestCanHaveBody() { - var buffer = ByteBufferAllocator().buffer(capacity: 100) - buffer.writeBytes([UInt8](repeating: 12, count: 100)) - // GET, HEAD, DELETE and CONNECT requests can have a payload. (though uncommon) let allowedMethods: [HTTPMethod] = [.GET, .HEAD, .DELETE, .CONNECT] var headers = HTTPHeaders() for method in allowedMethods { - XCTAssertNoThrow(try headers.validate(method: method, body: .byteBuffer(buffer))) + XCTAssertNoThrow(try headers.validateAndFixTransportFraming(method: method, bodyLength: .fixed(length: 100))) } } @@ -79,7 +72,7 @@ class RequestValidationTests: XCTestCase { ("User Agent", "Haha"), ]) - XCTAssertThrowsError(try headers.validate(method: .GET, body: nil)) { error in + XCTAssertThrowsError(try headers.validateAndFixTransportFraming(method: .GET, bodyLength: .fixed(length: 0))) { error in XCTAssertEqual(error as? HTTPClientError, HTTPClientError.invalidHeaderFieldNames(["User Agent"])) } } @@ -92,7 +85,7 @@ class RequestValidationTests: XCTestCase { ("!#$%&'*+-.^_`|~", "Haha"), ]) - XCTAssertNoThrow(try headers.validate(method: .GET, body: nil)) + XCTAssertNoThrow(try headers.validateAndFixTransportFraming(method: .GET, bodyLength: .fixed(length: 0))) } func testMetadataDetectConnectionClose() { @@ -100,14 +93,14 @@ class RequestValidationTests: XCTestCase { ("Connection", "close"), ]) var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validate(method: .GET, body: nil)) + XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: .GET, bodyLength: .fixed(length: 0))) XCTAssertEqual(metadata?.connectionClose, true) } func testMetadataDefaultIsConnectionCloseIsFalse() { var headers = HTTPHeaders([]) var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validate(method: .GET, body: nil)) + XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: .GET, bodyLength: .fixed(length: 0))) XCTAssertEqual(metadata?.connectionClose, false) } @@ -121,7 +114,7 @@ class RequestValidationTests: XCTestCase { for method: HTTPMethod in [.GET, .HEAD, .DELETE, .CONNECT, .TRACE] { var headers: HTTPHeaders = .init() var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validate(method: method, body: nil)) + XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: method, bodyLength: .fixed(length: 0))) XCTAssertTrue(headers["content-length"].isEmpty) XCTAssertTrue(headers["transfer-encoding"].isEmpty) XCTAssertEqual(metadata?.body, .fixedSize(0)) @@ -130,7 +123,7 @@ class RequestValidationTests: XCTestCase { for method: HTTPMethod in [.POST, .PUT] { var headers: HTTPHeaders = .init() var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validate(method: method, body: nil)) + XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: method, bodyLength: .fixed(length: 0))) XCTAssertEqual(headers["content-length"].first, "0") XCTAssertFalse(headers["transfer-encoding"].contains("chunked")) XCTAssertEqual(metadata?.body, .fixedSize(0)) @@ -146,7 +139,7 @@ class RequestValidationTests: XCTestCase { for method: HTTPMethod in [.GET, .HEAD, .DELETE, .CONNECT] { var headers: HTTPHeaders = .init() var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validate(method: method, body: .byteBuffer(ByteBuffer(bytes: [0])))) + XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: method, bodyLength: .fixed(length: 1))) XCTAssertEqual(headers["content-length"].first, "1") XCTAssertTrue(headers["transfer-encoding"].isEmpty) XCTAssertEqual(metadata?.body, .fixedSize(1)) @@ -155,11 +148,8 @@ class RequestValidationTests: XCTestCase { // 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]))) - } var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validate(method: method, body: body)) + XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: method, bodyLength: .dynamic)) XCTAssertTrue(headers["content-length"].isEmpty) XCTAssertTrue(headers["transfer-encoding"].contains("chunked")) XCTAssertEqual(metadata?.body, .stream) @@ -169,7 +159,7 @@ class RequestValidationTests: XCTestCase { for method: HTTPMethod in [.POST, .PUT] { var headers: HTTPHeaders = .init() var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validate(method: method, body: .byteBuffer(ByteBuffer(bytes: [0])))) + XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: method, bodyLength: .fixed(length: 1))) XCTAssertEqual(headers["content-length"].first, "1") XCTAssertTrue(headers["transfer-encoding"].isEmpty) XCTAssertEqual(metadata?.body, .fixedSize(1)) @@ -178,11 +168,8 @@ class RequestValidationTests: XCTestCase { // 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]))) - } var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validate(method: method, body: body)) + XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: method, bodyLength: .dynamic)) XCTAssertTrue(headers["content-length"].isEmpty) XCTAssertTrue(headers["transfer-encoding"].contains("chunked")) XCTAssertEqual(metadata?.body, .stream) @@ -197,7 +184,7 @@ class RequestValidationTests: XCTestCase { for method: HTTPMethod in [.GET, .HEAD, .DELETE, .CONNECT, .TRACE] { var headers: HTTPHeaders = .init([("Content-Length", "1")]) var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validate(method: method, body: nil)) + XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: method, bodyLength: .fixed(length: 0))) XCTAssertTrue(headers["content-length"].isEmpty) XCTAssertTrue(headers["transfer-encoding"].isEmpty) XCTAssertEqual(metadata?.body, .fixedSize(0)) @@ -206,7 +193,7 @@ class RequestValidationTests: XCTestCase { for method: HTTPMethod in [.POST, .PUT] { var headers: HTTPHeaders = .init([("Content-Length", "1")]) var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validate(method: method, body: nil)) + XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: method, bodyLength: .fixed(length: 0))) XCTAssertEqual(headers["content-length"].first, "0") XCTAssertTrue(headers["transfer-encoding"].isEmpty) XCTAssertEqual(metadata?.body, .fixedSize(0)) @@ -221,7 +208,7 @@ class RequestValidationTests: XCTestCase { for method: HTTPMethod in [.GET, .HEAD, .DELETE, .CONNECT] { var headers: HTTPHeaders = .init([("Content-Length", "1")]) var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validate(method: method, body: .byteBuffer(ByteBuffer(bytes: [0])))) + XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: method, bodyLength: .fixed(length: 1))) XCTAssertEqual(headers["content-length"].first, "1") XCTAssertTrue(headers["transfer-encoding"].isEmpty) XCTAssertEqual(metadata?.body, .fixedSize(1)) @@ -230,7 +217,7 @@ class RequestValidationTests: XCTestCase { for method: HTTPMethod in [.POST, .PUT] { var headers: HTTPHeaders = .init([("Content-Length", "1")]) var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validate(method: method, body: .byteBuffer(ByteBuffer(bytes: [0])))) + XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: method, bodyLength: .fixed(length: 1))) XCTAssertEqual(headers["content-length"].first, "1") XCTAssertTrue(headers["transfer-encoding"].isEmpty) XCTAssertEqual(metadata?.body, .fixedSize(1)) @@ -245,7 +232,7 @@ class RequestValidationTests: XCTestCase { for method: HTTPMethod in [.GET, .HEAD, .DELETE, .CONNECT, .TRACE] { var headers: HTTPHeaders = .init([("Transfer-Encoding", "chunked")]) var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validate(method: method, body: nil)) + XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: method, bodyLength: .fixed(length: 0))) XCTAssertTrue(headers["content-length"].isEmpty) XCTAssertFalse(headers["transfer-encoding"].contains("chunked")) XCTAssertEqual(metadata?.body, .fixedSize(0)) @@ -254,7 +241,7 @@ class RequestValidationTests: XCTestCase { for method: HTTPMethod in [.POST, .PUT] { var headers: HTTPHeaders = .init([("Transfer-Encoding", "chunked")]) var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validate(method: method, body: nil)) + XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: method, bodyLength: .fixed(length: 0))) XCTAssertEqual(headers["content-length"].first, "0") XCTAssertFalse(headers["transfer-encoding"].contains("chunked")) XCTAssertEqual(metadata?.body, .fixedSize(0)) @@ -269,7 +256,7 @@ class RequestValidationTests: XCTestCase { for method: HTTPMethod in [.GET, .HEAD, .DELETE, .CONNECT] { var headers: HTTPHeaders = .init([("Transfer-Encoding", "chunked")]) var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validate(method: method, body: .byteBuffer(ByteBuffer(bytes: [0])))) + XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: method, bodyLength: .fixed(length: 1))) XCTAssertTrue(headers["content-length"].isEmpty) XCTAssertTrue(headers["transfer-encoding"].contains("chunked")) XCTAssertEqual(metadata?.body, .stream) @@ -278,7 +265,7 @@ class RequestValidationTests: XCTestCase { for method: HTTPMethod in [.POST, .PUT] { var headers: HTTPHeaders = .init([("Transfer-Encoding", "chunked")]) var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validate(method: method, body: .byteBuffer(ByteBuffer(bytes: [0])))) + XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: method, bodyLength: .fixed(length: 1))) XCTAssertTrue(headers["content-length"].isEmpty) XCTAssertTrue(headers["transfer-encoding"].contains("chunked")) XCTAssertEqual(metadata?.body, .stream) @@ -292,12 +279,12 @@ class RequestValidationTests: XCTestCase { 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)) + XCTAssertThrowsError(try headers.validateAndFixTransportFraming(method: method, bodyLength: .fixed(length: 0))) } for method: HTTPMethod in [.POST, .PUT] { var headers: HTTPHeaders = .init([("Content-Length", "1"), ("Transfer-Encoding", "chunked")]) - XCTAssertThrowsError(try headers.validate(method: method, body: nil)) + XCTAssertThrowsError(try headers.validateAndFixTransportFraming(method: method, bodyLength: .fixed(length: 0))) } } @@ -308,12 +295,12 @@ class RequestValidationTests: XCTestCase { 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])))) + XCTAssertThrowsError(try headers.validateAndFixTransportFraming(method: method, bodyLength: .fixed(length: 1))) } 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.validateAndFixTransportFraming(method: method, bodyLength: .fixed(length: 1))) } } @@ -336,4 +323,63 @@ class RequestValidationTests: XCTestCase { let req6 = try! HTTPClient.Request(url: "https://localhost/get", headers: ["host": "foo"]) XCTAssertEqual(try req6.createRequestHead().0.headers["host"].first, "foo") } + + func testTraceMethodIsNotAllowedToHaveAFixedLengthBody() { + var headers = HTTPHeaders() + XCTAssertThrowsError(try headers.validateAndFixTransportFraming(method: .TRACE, bodyLength: .fixed(length: 10))) + } + + func testTraceMethodIsNotAllowedToHaveADynamicLengthBody() { + var headers = HTTPHeaders() + XCTAssertThrowsError(try headers.validateAndFixTransportFraming(method: .TRACE, bodyLength: .dynamic)) + } + + func testTransferEncodingsArePreservedIfBodyLengthIsFixed() { + var headers: HTTPHeaders = [ + "Transfer-Encoding": "gzip, chunked", + ] + XCTAssertNoThrow(try headers.validateAndFixTransportFraming(method: .POST, bodyLength: .fixed(length: 1))) + XCTAssertEqual(headers, [ + "Transfer-Encoding": "gzip, chunked", + ]) + } + + func testTransferEncodingsArePreservedIfBodyLengthIsDynamic() { + var headers: HTTPHeaders = [ + "Transfer-Encoding": "gzip, chunked", + ] + XCTAssertNoThrow(try headers.validateAndFixTransportFraming(method: .POST, bodyLength: .dynamic)) + XCTAssertEqual(headers, [ + "Transfer-Encoding": "gzip, chunked", + ]) + } + + func testTransferEncodingValidation() { + XCTAssertNoThrow(try HTTPHeaders.validateTransferEncoding([String]())) + XCTAssertNoThrow(try HTTPHeaders.validateTransferEncoding(["Chunked"])) + XCTAssertNoThrow(try HTTPHeaders.validateTransferEncoding(["chunked"])) + XCTAssertNoThrow(try HTTPHeaders.validateTransferEncoding(["gzip", "chunked"])) + XCTAssertNoThrow(try HTTPHeaders.validateTransferEncoding(["deflat", "chunked"])) + + XCTAssertThrowsError(try HTTPHeaders.validateTransferEncoding(["gzip"])) { + XCTAssertEqual($0 as? HTTPClientError, .transferEncodingSpecifiedButChunkedIsNotTheFinalEncoding) + } + XCTAssertThrowsError(try HTTPHeaders.validateTransferEncoding(["chunked", "Chunked"])) { + XCTAssertEqual($0 as? HTTPClientError, .chunkedSpecifiedMultipleTimes) + } + XCTAssertThrowsError(try HTTPHeaders.validateTransferEncoding(["chunked", "chunked"])) { + XCTAssertEqual($0 as? HTTPClientError, .chunkedSpecifiedMultipleTimes) + } + XCTAssertThrowsError(try HTTPHeaders.validateTransferEncoding(["chunked", "gzip"])) { + XCTAssertEqual($0 as? HTTPClientError, .transferEncodingSpecifiedButChunkedIsNotTheFinalEncoding) + } + XCTAssertThrowsError(try HTTPHeaders.validateTransferEncoding(["chunked", "gzip", "chunked"])) { + let error = $0 as? HTTPClientError + XCTAssertTrue( + error == .chunkedSpecifiedMultipleTimes || + error == .transferEncodingSpecifiedButChunkedIsNotTheFinalEncoding, + "\(error as Any) must be \(HTTPClientError.chunkedSpecifiedMultipleTimes) or \(HTTPClientError.transferEncodingSpecifiedButChunkedIsNotTheFinalEncoding)" + ) + } + } } From 242e3929590af6973697fc4d3dfa406011f3f941 Mon Sep 17 00:00:00 2001 From: David Nadoba Date: Thu, 18 Nov 2021 14:07:47 +0100 Subject: [PATCH 2/6] fix typo --- Sources/AsyncHTTPClient/ConnectionPool/RequestBodyLength.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/AsyncHTTPClient/ConnectionPool/RequestBodyLength.swift b/Sources/AsyncHTTPClient/ConnectionPool/RequestBodyLength.swift index 9905070c3..648c74913 100644 --- a/Sources/AsyncHTTPClient/ConnectionPool/RequestBodyLength.swift +++ b/Sources/AsyncHTTPClient/ConnectionPool/RequestBodyLength.swift @@ -13,7 +13,7 @@ //===----------------------------------------------------------------------===// enum RequestBodyLength: Equatable { - /// size of the reuqest body is not known before starting the request + /// size of the request body is not known before starting the request case dynamic /// size of the request body is fixed and exactly `length` bytes case fixed(length: Int) From 311d927ccfbf488ca15ab58aef02bd213a3911b3 Mon Sep 17 00:00:00 2001 From: David Nadoba Date: Mon, 22 Nov 2021 14:41:15 +0100 Subject: [PATCH 3/6] always overwrite `Transport-Encoding` and `Content-Length` --- Sources/AsyncHTTPClient/HTTPClient.swift | 7 +- Sources/AsyncHTTPClient/HTTPHandler.swift | 2 +- .../AsyncHTTPClient/RequestValidation.swift | 119 +++++----------- .../HTTPClientTests+XCTest.swift | 2 +- .../HTTPClientTests.swift | 11 +- .../RequestValidationTests+XCTest.swift | 5 +- .../RequestValidationTests.swift | 130 +++++++----------- 7 files changed, 101 insertions(+), 175 deletions(-) diff --git a/Sources/AsyncHTTPClient/HTTPClient.swift b/Sources/AsyncHTTPClient/HTTPClient.swift index 4a181e388..20950c4bf 100644 --- a/Sources/AsyncHTTPClient/HTTPClient.swift +++ b/Sources/AsyncHTTPClient/HTTPClient.swift @@ -883,8 +883,8 @@ public struct HTTPClientError: Error, Equatable, CustomStringConvertible { case remoteConnectionClosed case cancelled case identityCodingIncorrectlyPresent + @available(*, deprecated) case chunkedSpecifiedMultipleTimes - case transferEncodingSpecifiedButChunkedIsNotTheFinalEncoding case invalidProxyResponse case contentLengthMissing case proxyAuthenticationRequired @@ -895,6 +895,7 @@ public struct HTTPClientError: Error, Equatable, CustomStringConvertible { case invalidHeaderFieldNames([String]) case bodyLengthMismatch case writeAfterRequestSent + @available(*, deprecated) case incompatibleHeaders case connectTimeout case socksHandshakeTimeout @@ -938,9 +939,8 @@ public struct HTTPClientError: Error, Equatable, CustomStringConvertible { /// Request contains invalid identity encoding. public static let identityCodingIncorrectlyPresent = HTTPClientError(code: .identityCodingIncorrectlyPresent) /// Request contains multiple chunks definitions. + @available(*, deprecated) public static let chunkedSpecifiedMultipleTimes = HTTPClientError(code: .chunkedSpecifiedMultipleTimes) - /// Request specifies `Transfer-Encoding` but `chunked` is not the final encoding - public static let transferEncodingSpecifiedButChunkedIsNotTheFinalEncoding = HTTPClientError(code: .transferEncodingSpecifiedButChunkedIsNotTheFinalEncoding) /// Proxy response was invalid. public static let invalidProxyResponse = HTTPClientError(code: .invalidProxyResponse) /// Request does not contain `Content-Length` header. @@ -962,6 +962,7 @@ public struct HTTPClientError: Error, Equatable, CustomStringConvertible { /// Body part was written after request was fully sent. public static let writeAfterRequestSent = HTTPClientError(code: .writeAfterRequestSent) /// Incompatible headers specified, for example `Transfer-Encoding` and `Content-Length`. + @available(*, deprecated) public static let incompatibleHeaders = HTTPClientError(code: .incompatibleHeaders) /// Creating a new tcp connection timed out public static let connectTimeout = HTTPClientError(code: .connectTimeout) diff --git a/Sources/AsyncHTTPClient/HTTPHandler.swift b/Sources/AsyncHTTPClient/HTTPHandler.swift index 8fb3003fe..dd3af0ec6 100644 --- a/Sources/AsyncHTTPClient/HTTPHandler.swift +++ b/Sources/AsyncHTTPClient/HTTPHandler.swift @@ -308,7 +308,7 @@ extension HTTPClient { head.headers.add(name: "host", value: host) } - let metadata = try head.headers.validateAndFixTransportFraming(method: self.method, bodyLength: .init(self.body)) + let metadata = try head.headers.validateAndSetTransportFraming(method: self.method, bodyLength: .init(self.body)) return (head, metadata) } diff --git a/Sources/AsyncHTTPClient/RequestValidation.swift b/Sources/AsyncHTTPClient/RequestValidation.swift index 97909e9b3..4a20b0b6a 100644 --- a/Sources/AsyncHTTPClient/RequestValidation.swift +++ b/Sources/AsyncHTTPClient/RequestValidation.swift @@ -16,79 +16,35 @@ import NIOCore import NIOHTTP1 extension HTTPHeaders { - mutating func validateAndFixTransportFraming( + mutating func validateAndSetTransportFraming( method: HTTPMethod, bodyLength: RequestBodyLength ) throws -> RequestFramingMetadata { - let contentLength = self.first(name: "Content-Length") - let encodings = self[canonicalForm: "Transfer-Encoding"] - - // "Transfer-Encoding" and "Content-Length" are not allowed to present at the same time (https://tools.ietf.org/html/rfc7230#section-3.3.1) - guard encodings.isEmpty || contentLength == nil else { - throw HTTPClientError.incompatibleHeaders - } - try self.validateFieldNames() - try Self.validateTransferEncoding(encodings) - - if contentLength != nil { - self.remove(name: "Content-Length") - } - if !encodings.isEmpty { - self.remove(name: "Transfer-Encoding") - } - - let connectionClose = self[canonicalForm: "connection"].lazy.map { $0.lowercased() }.contains("close") - switch bodyLength { - case .fixed(0): - // 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 { - case .GET, .HEAD, .DELETE, .CONNECT, .TRACE: - // A user agent SHOULD NOT send a Content-Length header field when the request - // message does not contain a payload body and the method semantics do not - // anticipate such a body. + if case .TRACE = method { + switch bodyLength { + case .fixed(length: 0): break - default: - // A user agent SHOULD send a Content-Length in a request message when - // no Transfer-Encoding is sent and the request method defines a meaning - // for an enclosed payload body. - self.add(name: "Content-Length", value: "0") - } - return .init(connectionClose: connectionClose, body: .fixedSize(0)) - case .fixed(let length): - if case .TRACE = method { - // A client MUST NOT send a message body in a TRACE request. - // https://tools.ietf.org/html/rfc7230#section-4.3.8 - throw HTTPClientError.traceRequestWithBody - } - if encodings.isEmpty { - self.add(name: "Content-Length", value: String(length)) - return .init(connectionClose: connectionClose, body: .fixedSize(length)) - } else { - self.add(name: "Transfer-Encoding", value: encodings.joined(separator: ", ")) - return .init(connectionClose: connectionClose, body: .stream) - } - case .dynamic: - if case .TRACE = method { + case .dynamic, .fixed: // A client MUST NOT send a message body in a TRACE request. // https://tools.ietf.org/html/rfc7230#section-4.3.8 throw HTTPClientError.traceRequestWithBody } + } - if encodings.isEmpty && contentLength == nil { - // if a user forgot to specify a Content-Length and Transfer-Encoding, we will set it for them - self.add(name: "Transfer-Encoding", value: "chunked") - } else { - self.add(name: "Transfer-Encoding", value: encodings.joined(separator: ", ")) - } + self.setTransportFraming(method: method, bodyLength: bodyLength) + let connectionClose = self[canonicalForm: "connection"].lazy.map { $0.lowercased() }.contains("close") + switch bodyLength { + case .dynamic: return .init(connectionClose: connectionClose, body: .stream) + case .fixed(let length): + return .init(connectionClose: connectionClose, body: .fixedSize(length)) } } - func validateFieldNames() throws { + private func validateFieldNames() throws { let invalidFieldNames = self.compactMap { (name, _) -> String? in let satisfy = name.utf8.allSatisfy { (char) -> Bool in switch char { @@ -124,32 +80,33 @@ extension HTTPHeaders { } } - static func validateTransferEncoding( - _ encodings: Encodings - ) throws where Encodings: Sequence, Encodings.Element: StringProtocol { - let encodings = encodings.map { $0.lowercased() } - - guard !encodings.contains("identity") else { - throw HTTPClientError.identityCodingIncorrectlyPresent - } + private mutating func setTransportFraming( + method: HTTPMethod, + bodyLength: RequestBodyLength + ) { + self.remove(name: "Content-Length") + self.remove(name: "Transfer-Encoding") - // If `Transfer-Encoding` is specified, `chunked` needs to be the last encoding and should not be specified multiple times - // https://datatracker.ietf.org/doc/html/rfc7230#section-3.3.1 - let chunkedEncodingCount = encodings.lazy.filter { $0 == "chunked" }.count - switch chunkedEncodingCount { - case 0: - if !encodings.isEmpty { - throw HTTPClientError.transferEncodingSpecifiedButChunkedIsNotTheFinalEncoding - } - case 1: - guard encodings.last == "chunked" else { - throw HTTPClientError.transferEncodingSpecifiedButChunkedIsNotTheFinalEncoding + switch bodyLength { + case .fixed(0): + // 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 { + case .GET, .HEAD, .DELETE, .CONNECT, .TRACE: + // A user agent SHOULD NOT send a Content-Length header field when the request + // message does not contain a payload body and the method semantics do not + // anticipate such a body. + break + default: + // A user agent SHOULD send a Content-Length in a request message when + // no Transfer-Encoding is sent and the request method defines a meaning + // for an enclosed payload body. + self.add(name: "Content-Length", value: "0") } - case 2...: - throw HTTPClientError.chunkedSpecifiedMultipleTimes - default: - // unreachable because `chunkedEncodingCount` is guaranteed to be positive - preconditionFailure() + case .fixed(let length): + self.add(name: "Content-Length", value: String(length)) + case .dynamic: + self.add(name: "Transfer-Encoding", value: "chunked") } } } diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift index 42395e2cb..13e8fa3fd 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift @@ -128,7 +128,7 @@ extension HTTPClientTests { ("testErrorAfterCloseWhileBackpressureExerted", testErrorAfterCloseWhileBackpressureExerted), ("testRequestSpecificTLS", testRequestSpecificTLS), ("testConnectionPoolSizeConfigValueIsRespected", testConnectionPoolSizeConfigValueIsRespected), - ("testRequestWithHeaderTransferEncodingIdentityFails", testRequestWithHeaderTransferEncodingIdentityFails), + ("testRequestWithHeaderTransferEncodingIdentityDoesNotFail", testRequestWithHeaderTransferEncodingIdentityDoesNotFail), ] } } diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index df5cd489b..1435265c5 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -2941,22 +2941,23 @@ class HTTPClientTests: XCTestCase { XCTAssertEqual(httpBin.createdConnections, poolSize) } - func testRequestWithHeaderTransferEncodingIdentityFails() { + func testRequestWithHeaderTransferEncodingIdentityDoesNotFail() { let group = MultiThreadedEventLoopGroup(numberOfThreads: 1) defer { XCTAssertNoThrow(try group.syncShutdownGracefully()) } let client = HTTPClient(eventLoopGroupProvider: .shared(group)) defer { XCTAssertNoThrow(try client.syncShutdown()) } - guard var request = try? Request(url: "http://localhost/get") else { + let httpBin = HTTPBin() + defer { XCTAssertNoThrow(try httpBin.shutdown()) } + + guard var request = try? Request(url: "http://127.0.0.1:\(httpBin.port)/get") else { return XCTFail("Expected to have a request here.") } request.headers.add(name: "X-Test-Header", value: "X-Test-Value") request.headers.add(name: "Transfer-Encoding", value: "identity") request.body = .string("1234") - XCTAssertThrowsError(try client.execute(request: request).wait()) { - XCTAssertEqual($0 as? HTTPClientError, .identityCodingIncorrectlyPresent) - } + XCTAssertNoThrow(try client.execute(request: request).wait()) } } diff --git a/Tests/AsyncHTTPClientTests/RequestValidationTests+XCTest.swift b/Tests/AsyncHTTPClientTests/RequestValidationTests+XCTest.swift index b56cb0790..4e384f92b 100644 --- a/Tests/AsyncHTTPClientTests/RequestValidationTests+XCTest.swift +++ b/Tests/AsyncHTTPClientTests/RequestValidationTests+XCTest.swift @@ -45,9 +45,8 @@ extension RequestValidationTests { ("testHostHeaderIsSetCorrectlyInCreateRequestHead", testHostHeaderIsSetCorrectlyInCreateRequestHead), ("testTraceMethodIsNotAllowedToHaveAFixedLengthBody", testTraceMethodIsNotAllowedToHaveAFixedLengthBody), ("testTraceMethodIsNotAllowedToHaveADynamicLengthBody", testTraceMethodIsNotAllowedToHaveADynamicLengthBody), - ("testTransferEncodingsArePreservedIfBodyLengthIsFixed", testTransferEncodingsArePreservedIfBodyLengthIsFixed), - ("testTransferEncodingsArePreservedIfBodyLengthIsDynamic", testTransferEncodingsArePreservedIfBodyLengthIsDynamic), - ("testTransferEncodingValidation", testTransferEncodingValidation), + ("testTransferEncodingsAreOverwritteIfBodyLengthIsFixed", testTransferEncodingsAreOverwritteIfBodyLengthIsFixed), + ("testTransferEncodingsAreOverwrittenIfBodyLengthIsDynamic", testTransferEncodingsAreOverwrittenIfBodyLengthIsDynamic), ] } } diff --git a/Tests/AsyncHTTPClientTests/RequestValidationTests.swift b/Tests/AsyncHTTPClientTests/RequestValidationTests.swift index 02678a453..96f692b3d 100644 --- a/Tests/AsyncHTTPClientTests/RequestValidationTests.swift +++ b/Tests/AsyncHTTPClientTests/RequestValidationTests.swift @@ -21,7 +21,7 @@ class RequestValidationTests: XCTestCase { func testContentLengthHeaderIsRemovedFromGETIfNoBody() { var headers = HTTPHeaders([("Content-Length", "0")]) var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: .GET, bodyLength: .fixed(length: 0))) + XCTAssertNoThrow(metadata = try headers.validateAndSetTransportFraming(method: .GET, bodyLength: .fixed(length: 0))) XCTAssertNil(headers.first(name: "Content-Length")) XCTAssertEqual(metadata?.body, .fixedSize(0)) } @@ -29,13 +29,13 @@ class RequestValidationTests: XCTestCase { func testContentLengthHeaderIsAddedToPOSTAndPUTWithNoBody() { var putHeaders = HTTPHeaders() var putMetadata: RequestFramingMetadata? - XCTAssertNoThrow(putMetadata = try putHeaders.validateAndFixTransportFraming(method: .PUT, bodyLength: .fixed(length: 0))) + XCTAssertNoThrow(putMetadata = try putHeaders.validateAndSetTransportFraming(method: .PUT, bodyLength: .fixed(length: 0))) XCTAssertEqual(putHeaders.first(name: "Content-Length"), "0") XCTAssertEqual(putMetadata?.body, .fixedSize(0)) var postHeaders = HTTPHeaders() var postMetadata: RequestFramingMetadata? - XCTAssertNoThrow(postMetadata = try postHeaders.validateAndFixTransportFraming(method: .POST, bodyLength: .fixed(length: 0))) + XCTAssertNoThrow(postMetadata = try postHeaders.validateAndSetTransportFraming(method: .POST, bodyLength: .fixed(length: 0))) XCTAssertEqual(postHeaders.first(name: "Content-Length"), "0") XCTAssertEqual(postMetadata?.body, .fixedSize(0)) } @@ -43,7 +43,7 @@ class RequestValidationTests: XCTestCase { func testContentLengthHeaderIsChangedIfBodyHasDifferentLength() { var headers = HTTPHeaders([("Content-Length", "0")]) var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: .PUT, bodyLength: .fixed(length: 200))) + XCTAssertNoThrow(metadata = try headers.validateAndSetTransportFraming(method: .PUT, bodyLength: .fixed(length: 200))) XCTAssertEqual(headers.first(name: "Content-Length"), "200") XCTAssertEqual(metadata?.body, .fixedSize(200)) } @@ -51,7 +51,7 @@ class RequestValidationTests: XCTestCase { func testTRACERequestMustNotHaveBody() { for header in [("Content-Length", "200"), ("Transfer-Encoding", "chunked")] { var headers = HTTPHeaders([header]) - XCTAssertThrowsError(try headers.validateAndFixTransportFraming(method: .TRACE, bodyLength: .fixed(length: 200))) { + XCTAssertThrowsError(try headers.validateAndSetTransportFraming(method: .TRACE, bodyLength: .fixed(length: 200))) { XCTAssertEqual($0 as? HTTPClientError, .traceRequestWithBody) } } @@ -62,7 +62,7 @@ class RequestValidationTests: XCTestCase { let allowedMethods: [HTTPMethod] = [.GET, .HEAD, .DELETE, .CONNECT] var headers = HTTPHeaders() for method in allowedMethods { - XCTAssertNoThrow(try headers.validateAndFixTransportFraming(method: method, bodyLength: .fixed(length: 100))) + XCTAssertNoThrow(try headers.validateAndSetTransportFraming(method: method, bodyLength: .fixed(length: 100))) } } @@ -72,7 +72,7 @@ class RequestValidationTests: XCTestCase { ("User Agent", "Haha"), ]) - XCTAssertThrowsError(try headers.validateAndFixTransportFraming(method: .GET, bodyLength: .fixed(length: 0))) { error in + XCTAssertThrowsError(try headers.validateAndSetTransportFraming(method: .GET, bodyLength: .fixed(length: 0))) { error in XCTAssertEqual(error as? HTTPClientError, HTTPClientError.invalidHeaderFieldNames(["User Agent"])) } } @@ -85,7 +85,7 @@ class RequestValidationTests: XCTestCase { ("!#$%&'*+-.^_`|~", "Haha"), ]) - XCTAssertNoThrow(try headers.validateAndFixTransportFraming(method: .GET, bodyLength: .fixed(length: 0))) + XCTAssertNoThrow(try headers.validateAndSetTransportFraming(method: .GET, bodyLength: .fixed(length: 0))) } func testMetadataDetectConnectionClose() { @@ -93,14 +93,14 @@ class RequestValidationTests: XCTestCase { ("Connection", "close"), ]) var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: .GET, bodyLength: .fixed(length: 0))) + XCTAssertNoThrow(metadata = try headers.validateAndSetTransportFraming(method: .GET, bodyLength: .fixed(length: 0))) XCTAssertEqual(metadata?.connectionClose, true) } func testMetadataDefaultIsConnectionCloseIsFalse() { var headers = HTTPHeaders([]) var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: .GET, bodyLength: .fixed(length: 0))) + XCTAssertNoThrow(metadata = try headers.validateAndSetTransportFraming(method: .GET, bodyLength: .fixed(length: 0))) XCTAssertEqual(metadata?.connectionClose, false) } @@ -114,7 +114,7 @@ class RequestValidationTests: XCTestCase { for method: HTTPMethod in [.GET, .HEAD, .DELETE, .CONNECT, .TRACE] { var headers: HTTPHeaders = .init() var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: method, bodyLength: .fixed(length: 0))) + XCTAssertNoThrow(metadata = try headers.validateAndSetTransportFraming(method: method, bodyLength: .fixed(length: 0))) XCTAssertTrue(headers["content-length"].isEmpty) XCTAssertTrue(headers["transfer-encoding"].isEmpty) XCTAssertEqual(metadata?.body, .fixedSize(0)) @@ -123,7 +123,7 @@ class RequestValidationTests: XCTestCase { for method: HTTPMethod in [.POST, .PUT] { var headers: HTTPHeaders = .init() var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: method, bodyLength: .fixed(length: 0))) + XCTAssertNoThrow(metadata = try headers.validateAndSetTransportFraming(method: method, bodyLength: .fixed(length: 0))) XCTAssertEqual(headers["content-length"].first, "0") XCTAssertFalse(headers["transfer-encoding"].contains("chunked")) XCTAssertEqual(metadata?.body, .fixedSize(0)) @@ -139,7 +139,7 @@ class RequestValidationTests: XCTestCase { for method: HTTPMethod in [.GET, .HEAD, .DELETE, .CONNECT] { var headers: HTTPHeaders = .init() var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: method, bodyLength: .fixed(length: 1))) + XCTAssertNoThrow(metadata = try headers.validateAndSetTransportFraming(method: method, bodyLength: .fixed(length: 1))) XCTAssertEqual(headers["content-length"].first, "1") XCTAssertTrue(headers["transfer-encoding"].isEmpty) XCTAssertEqual(metadata?.body, .fixedSize(1)) @@ -149,7 +149,7 @@ class RequestValidationTests: XCTestCase { for method: HTTPMethod in [.GET, .HEAD, .DELETE, .CONNECT] { var headers: HTTPHeaders = .init() var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: method, bodyLength: .dynamic)) + XCTAssertNoThrow(metadata = try headers.validateAndSetTransportFraming(method: method, bodyLength: .dynamic)) XCTAssertTrue(headers["content-length"].isEmpty) XCTAssertTrue(headers["transfer-encoding"].contains("chunked")) XCTAssertEqual(metadata?.body, .stream) @@ -159,7 +159,7 @@ class RequestValidationTests: XCTestCase { for method: HTTPMethod in [.POST, .PUT] { var headers: HTTPHeaders = .init() var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: method, bodyLength: .fixed(length: 1))) + XCTAssertNoThrow(metadata = try headers.validateAndSetTransportFraming(method: method, bodyLength: .fixed(length: 1))) XCTAssertEqual(headers["content-length"].first, "1") XCTAssertTrue(headers["transfer-encoding"].isEmpty) XCTAssertEqual(metadata?.body, .fixedSize(1)) @@ -169,7 +169,7 @@ class RequestValidationTests: XCTestCase { for method: HTTPMethod in [.POST, .PUT] { var headers: HTTPHeaders = .init() var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: method, bodyLength: .dynamic)) + XCTAssertNoThrow(metadata = try headers.validateAndSetTransportFraming(method: method, bodyLength: .dynamic)) XCTAssertTrue(headers["content-length"].isEmpty) XCTAssertTrue(headers["transfer-encoding"].contains("chunked")) XCTAssertEqual(metadata?.body, .stream) @@ -184,7 +184,7 @@ class RequestValidationTests: XCTestCase { for method: HTTPMethod in [.GET, .HEAD, .DELETE, .CONNECT, .TRACE] { var headers: HTTPHeaders = .init([("Content-Length", "1")]) var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: method, bodyLength: .fixed(length: 0))) + XCTAssertNoThrow(metadata = try headers.validateAndSetTransportFraming(method: method, bodyLength: .fixed(length: 0))) XCTAssertTrue(headers["content-length"].isEmpty) XCTAssertTrue(headers["transfer-encoding"].isEmpty) XCTAssertEqual(metadata?.body, .fixedSize(0)) @@ -193,7 +193,7 @@ class RequestValidationTests: XCTestCase { for method: HTTPMethod in [.POST, .PUT] { var headers: HTTPHeaders = .init([("Content-Length", "1")]) var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: method, bodyLength: .fixed(length: 0))) + XCTAssertNoThrow(metadata = try headers.validateAndSetTransportFraming(method: method, bodyLength: .fixed(length: 0))) XCTAssertEqual(headers["content-length"].first, "0") XCTAssertTrue(headers["transfer-encoding"].isEmpty) XCTAssertEqual(metadata?.body, .fixedSize(0)) @@ -208,7 +208,7 @@ class RequestValidationTests: XCTestCase { for method: HTTPMethod in [.GET, .HEAD, .DELETE, .CONNECT] { var headers: HTTPHeaders = .init([("Content-Length", "1")]) var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: method, bodyLength: .fixed(length: 1))) + XCTAssertNoThrow(metadata = try headers.validateAndSetTransportFraming(method: method, bodyLength: .fixed(length: 1))) XCTAssertEqual(headers["content-length"].first, "1") XCTAssertTrue(headers["transfer-encoding"].isEmpty) XCTAssertEqual(metadata?.body, .fixedSize(1)) @@ -217,7 +217,7 @@ class RequestValidationTests: XCTestCase { for method: HTTPMethod in [.POST, .PUT] { var headers: HTTPHeaders = .init([("Content-Length", "1")]) var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: method, bodyLength: .fixed(length: 1))) + XCTAssertNoThrow(metadata = try headers.validateAndSetTransportFraming(method: method, bodyLength: .fixed(length: 1))) XCTAssertEqual(headers["content-length"].first, "1") XCTAssertTrue(headers["transfer-encoding"].isEmpty) XCTAssertEqual(metadata?.body, .fixedSize(1)) @@ -232,7 +232,7 @@ class RequestValidationTests: XCTestCase { for method: HTTPMethod in [.GET, .HEAD, .DELETE, .CONNECT, .TRACE] { var headers: HTTPHeaders = .init([("Transfer-Encoding", "chunked")]) var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: method, bodyLength: .fixed(length: 0))) + XCTAssertNoThrow(metadata = try headers.validateAndSetTransportFraming(method: method, bodyLength: .fixed(length: 0))) XCTAssertTrue(headers["content-length"].isEmpty) XCTAssertFalse(headers["transfer-encoding"].contains("chunked")) XCTAssertEqual(metadata?.body, .fixedSize(0)) @@ -241,7 +241,7 @@ class RequestValidationTests: XCTestCase { for method: HTTPMethod in [.POST, .PUT] { var headers: HTTPHeaders = .init([("Transfer-Encoding", "chunked")]) var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: method, bodyLength: .fixed(length: 0))) + XCTAssertNoThrow(metadata = try headers.validateAndSetTransportFraming(method: method, bodyLength: .fixed(length: 0))) XCTAssertEqual(headers["content-length"].first, "0") XCTAssertFalse(headers["transfer-encoding"].contains("chunked")) XCTAssertEqual(metadata?.body, .fixedSize(0)) @@ -250,57 +250,54 @@ class RequestValidationTests: XCTestCase { // Method kind User sets Body Expectation // -------------------------------------------------------------------------------------- - // .GET, .HEAD, .DELETE, .CONNECT transfer-encoding: chunked not nil chunked - // other transfer-encoding: chunked not nil chunked + // .GET, .HEAD, .DELETE, .CONNECT transfer-encoding: chunked not nil no header + // other transfer-encoding: chunked not nil content-length func testTransferEncodingHeaderHasBody() throws { for method: HTTPMethod in [.GET, .HEAD, .DELETE, .CONNECT] { var headers: HTTPHeaders = .init([("Transfer-Encoding", "chunked")]) - var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: method, bodyLength: .fixed(length: 1))) - XCTAssertTrue(headers["content-length"].isEmpty) - XCTAssertTrue(headers["transfer-encoding"].contains("chunked")) - XCTAssertEqual(metadata?.body, .stream) + XCTAssertNoThrow(try headers.validateAndSetTransportFraming(method: method, bodyLength: .fixed(length: 1))) + XCTAssertEqual(headers, ["Content-Length": "1"]) } for method: HTTPMethod in [.POST, .PUT] { var headers: HTTPHeaders = .init([("Transfer-Encoding", "chunked")]) - var metadata: RequestFramingMetadata? - XCTAssertNoThrow(metadata = try headers.validateAndFixTransportFraming(method: method, bodyLength: .fixed(length: 1))) - XCTAssertTrue(headers["content-length"].isEmpty) - XCTAssertTrue(headers["transfer-encoding"].contains("chunked")) - XCTAssertEqual(metadata?.body, .stream) + XCTAssertNoThrow(try headers.validateAndSetTransportFraming(method: method, bodyLength: .fixed(length: 1))) + XCTAssertEqual(headers, ["Content-Length": "1"]) } } // Method kind User sets Body Expectation // --------------------------------------------------------------------------------------- - // .GET, .HEAD, .DELETE, .CONNECT, .TRACE CL & chunked (illegal) nil throws error - // other CL & chunked (illegal) nil throws error + // .GET, .HEAD, .DELETE, .CONNECT, .TRACE CL & chunked (illegal) nil no header + // other CL & chunked (illegal) nil content-length 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.validateAndFixTransportFraming(method: method, bodyLength: .fixed(length: 0))) + XCTAssertNoThrow(try headers.validateAndSetTransportFraming(method: method, bodyLength: .fixed(length: 0))) + XCTAssertEqual(headers, [:]) } for method: HTTPMethod in [.POST, .PUT] { var headers: HTTPHeaders = .init([("Content-Length", "1"), ("Transfer-Encoding", "chunked")]) - XCTAssertThrowsError(try headers.validateAndFixTransportFraming(method: method, bodyLength: .fixed(length: 0))) + XCTAssertNoThrow(try headers.validateAndSetTransportFraming(method: method, bodyLength: .fixed(length: 0))) + XCTAssertEqual(headers, ["Content-Length": "0"]) } } // 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 + // .TRACE CL & chunked (illegal) not nil throws error + // other CL & chunked (illegal) not nil content-length func testBothHeadersHasBody() throws { - for method: HTTPMethod in [.GET, .HEAD, .DELETE, .CONNECT, .TRACE] { + for method: HTTPMethod in [.TRACE] { var headers: HTTPHeaders = .init([("Content-Length", "1"), ("Transfer-Encoding", "chunked")]) - XCTAssertThrowsError(try headers.validateAndFixTransportFraming(method: method, bodyLength: .fixed(length: 1))) + XCTAssertThrowsError(try headers.validateAndSetTransportFraming(method: method, bodyLength: .fixed(length: 1))) } - for method: HTTPMethod in [.POST, .PUT] { + for method: HTTPMethod in [.GET, .HEAD, .DELETE, .CONNECT, .POST, .PUT] { var headers: HTTPHeaders = .init([("Content-Length", "1"), ("Transfer-Encoding", "chunked")]) - XCTAssertThrowsError(try headers.validateAndFixTransportFraming(method: method, bodyLength: .fixed(length: 1))) + XCTAssertNoThrow(try headers.validateAndSetTransportFraming(method: method, bodyLength: .fixed(length: 1))) + XCTAssertEqual(headers, ["Content-Length": "1"]) } } @@ -326,60 +323,31 @@ class RequestValidationTests: XCTestCase { func testTraceMethodIsNotAllowedToHaveAFixedLengthBody() { var headers = HTTPHeaders() - XCTAssertThrowsError(try headers.validateAndFixTransportFraming(method: .TRACE, bodyLength: .fixed(length: 10))) + XCTAssertThrowsError(try headers.validateAndSetTransportFraming(method: .TRACE, bodyLength: .fixed(length: 10))) } func testTraceMethodIsNotAllowedToHaveADynamicLengthBody() { var headers = HTTPHeaders() - XCTAssertThrowsError(try headers.validateAndFixTransportFraming(method: .TRACE, bodyLength: .dynamic)) + XCTAssertThrowsError(try headers.validateAndSetTransportFraming(method: .TRACE, bodyLength: .dynamic)) } - func testTransferEncodingsArePreservedIfBodyLengthIsFixed() { + func testTransferEncodingsAreOverwritteIfBodyLengthIsFixed() { var headers: HTTPHeaders = [ "Transfer-Encoding": "gzip, chunked", ] - XCTAssertNoThrow(try headers.validateAndFixTransportFraming(method: .POST, bodyLength: .fixed(length: 1))) + XCTAssertNoThrow(try headers.validateAndSetTransportFraming(method: .POST, bodyLength: .fixed(length: 1))) XCTAssertEqual(headers, [ - "Transfer-Encoding": "gzip, chunked", + "Content-Length": "1", ]) } - func testTransferEncodingsArePreservedIfBodyLengthIsDynamic() { + func testTransferEncodingsAreOverwrittenIfBodyLengthIsDynamic() { var headers: HTTPHeaders = [ "Transfer-Encoding": "gzip, chunked", ] - XCTAssertNoThrow(try headers.validateAndFixTransportFraming(method: .POST, bodyLength: .dynamic)) + XCTAssertNoThrow(try headers.validateAndSetTransportFraming(method: .POST, bodyLength: .dynamic)) XCTAssertEqual(headers, [ - "Transfer-Encoding": "gzip, chunked", + "Transfer-Encoding": "chunked", ]) } - - func testTransferEncodingValidation() { - XCTAssertNoThrow(try HTTPHeaders.validateTransferEncoding([String]())) - XCTAssertNoThrow(try HTTPHeaders.validateTransferEncoding(["Chunked"])) - XCTAssertNoThrow(try HTTPHeaders.validateTransferEncoding(["chunked"])) - XCTAssertNoThrow(try HTTPHeaders.validateTransferEncoding(["gzip", "chunked"])) - XCTAssertNoThrow(try HTTPHeaders.validateTransferEncoding(["deflat", "chunked"])) - - XCTAssertThrowsError(try HTTPHeaders.validateTransferEncoding(["gzip"])) { - XCTAssertEqual($0 as? HTTPClientError, .transferEncodingSpecifiedButChunkedIsNotTheFinalEncoding) - } - XCTAssertThrowsError(try HTTPHeaders.validateTransferEncoding(["chunked", "Chunked"])) { - XCTAssertEqual($0 as? HTTPClientError, .chunkedSpecifiedMultipleTimes) - } - XCTAssertThrowsError(try HTTPHeaders.validateTransferEncoding(["chunked", "chunked"])) { - XCTAssertEqual($0 as? HTTPClientError, .chunkedSpecifiedMultipleTimes) - } - XCTAssertThrowsError(try HTTPHeaders.validateTransferEncoding(["chunked", "gzip"])) { - XCTAssertEqual($0 as? HTTPClientError, .transferEncodingSpecifiedButChunkedIsNotTheFinalEncoding) - } - XCTAssertThrowsError(try HTTPHeaders.validateTransferEncoding(["chunked", "gzip", "chunked"])) { - let error = $0 as? HTTPClientError - XCTAssertTrue( - error == .chunkedSpecifiedMultipleTimes || - error == .transferEncodingSpecifiedButChunkedIsNotTheFinalEncoding, - "\(error as Any) must be \(HTTPClientError.chunkedSpecifiedMultipleTimes) or \(HTTPClientError.transferEncodingSpecifiedButChunkedIsNotTheFinalEncoding)" - ) - } - } } From eaf16eb3144df32227e5336ae80274f0b30644d1 Mon Sep 17 00:00:00 2001 From: David Nadoba Date: Mon, 22 Nov 2021 15:07:46 +0100 Subject: [PATCH 4/6] fix documentation --- Sources/AsyncHTTPClient/HTTPClient.swift | 8 ++++---- Sources/AsyncHTTPClient/HTTPHandler.swift | 7 ++++--- .../RequestValidationTests+XCTest.swift | 2 +- .../AsyncHTTPClientTests/RequestValidationTests.swift | 10 +++++++--- 4 files changed, 16 insertions(+), 11 deletions(-) diff --git a/Sources/AsyncHTTPClient/HTTPClient.swift b/Sources/AsyncHTTPClient/HTTPClient.swift index 20950c4bf..44854a4e0 100644 --- a/Sources/AsyncHTTPClient/HTTPClient.swift +++ b/Sources/AsyncHTTPClient/HTTPClient.swift @@ -883,7 +883,7 @@ public struct HTTPClientError: Error, Equatable, CustomStringConvertible { case remoteConnectionClosed case cancelled case identityCodingIncorrectlyPresent - @available(*, deprecated) + @available(*, deprecated, message: "AsyncHTTPClient now silently corrects this invalid header.") case chunkedSpecifiedMultipleTimes case invalidProxyResponse case contentLengthMissing @@ -895,7 +895,7 @@ public struct HTTPClientError: Error, Equatable, CustomStringConvertible { case invalidHeaderFieldNames([String]) case bodyLengthMismatch case writeAfterRequestSent - @available(*, deprecated) + @available(*, deprecated, message: "AsyncHTTPClient now silently corrects invalid headers.") case incompatibleHeaders case connectTimeout case socksHandshakeTimeout @@ -939,7 +939,7 @@ public struct HTTPClientError: Error, Equatable, CustomStringConvertible { /// Request contains invalid identity encoding. public static let identityCodingIncorrectlyPresent = HTTPClientError(code: .identityCodingIncorrectlyPresent) /// Request contains multiple chunks definitions. - @available(*, deprecated) + @available(*, deprecated, message: "AsyncHTTPClient now silently corrects this invalid header.") public static let chunkedSpecifiedMultipleTimes = HTTPClientError(code: .chunkedSpecifiedMultipleTimes) /// Proxy response was invalid. public static let invalidProxyResponse = HTTPClientError(code: .invalidProxyResponse) @@ -962,7 +962,7 @@ public struct HTTPClientError: Error, Equatable, CustomStringConvertible { /// Body part was written after request was fully sent. public static let writeAfterRequestSent = HTTPClientError(code: .writeAfterRequestSent) /// Incompatible headers specified, for example `Transfer-Encoding` and `Content-Length`. - @available(*, deprecated) + @available(*, deprecated, message: "AsyncHTTPClient now silently corrects invalid headers.") public static let incompatibleHeaders = HTTPClientError(code: .incompatibleHeaders) /// Creating a new tcp connection timed out public static let connectTimeout = HTTPClientError(code: .connectTimeout) diff --git a/Sources/AsyncHTTPClient/HTTPHandler.swift b/Sources/AsyncHTTPClient/HTTPHandler.swift index dd3af0ec6..666800ee0 100644 --- a/Sources/AsyncHTTPClient/HTTPHandler.swift +++ b/Sources/AsyncHTTPClient/HTTPHandler.swift @@ -43,7 +43,8 @@ extension HTTPClient { } } - /// Body size. if nil and `Transfer-Encoding` header is **not** set, `Transfer-Encoding` will be automatically be set to `chunked` + /// Body size. if nil,`Transfer-Encoding` will automatically be set to `chunked`. Otherwise a `Content-Length` + /// header is set with the given `length`. public var length: Int? /// Body chunk provider. public var stream: (StreamWriter) -> EventLoopFuture @@ -61,8 +62,8 @@ extension HTTPClient { /// Create and stream body using `StreamWriter`. /// /// - parameters: - /// - length: Body size. Request validation will be failed with `HTTPClientErrors.contentLengthMissing` if nil, - /// unless `Transfer-Encoding: chunked` header is set. + /// - length: Body size. If nil, `Transfer-Encoding` will be automatically be set to `chunked`. Otherwise a `Content-Length` + /// header is set with the given `length`. /// - stream: Body chunk provider. public static func stream(length: Int? = nil, _ stream: @escaping (StreamWriter) -> EventLoopFuture) -> Body { return Body(length: length, stream: stream) diff --git a/Tests/AsyncHTTPClientTests/RequestValidationTests+XCTest.swift b/Tests/AsyncHTTPClientTests/RequestValidationTests+XCTest.swift index 4e384f92b..3a93d70ec 100644 --- a/Tests/AsyncHTTPClientTests/RequestValidationTests+XCTest.swift +++ b/Tests/AsyncHTTPClientTests/RequestValidationTests+XCTest.swift @@ -45,7 +45,7 @@ extension RequestValidationTests { ("testHostHeaderIsSetCorrectlyInCreateRequestHead", testHostHeaderIsSetCorrectlyInCreateRequestHead), ("testTraceMethodIsNotAllowedToHaveAFixedLengthBody", testTraceMethodIsNotAllowedToHaveAFixedLengthBody), ("testTraceMethodIsNotAllowedToHaveADynamicLengthBody", testTraceMethodIsNotAllowedToHaveADynamicLengthBody), - ("testTransferEncodingsAreOverwritteIfBodyLengthIsFixed", testTransferEncodingsAreOverwritteIfBodyLengthIsFixed), + ("testTransferEncodingsAreOverwrittenIfBodyLengthIsFixed", testTransferEncodingsAreOverwrittenIfBodyLengthIsFixed), ("testTransferEncodingsAreOverwrittenIfBodyLengthIsDynamic", testTransferEncodingsAreOverwrittenIfBodyLengthIsDynamic), ] } diff --git a/Tests/AsyncHTTPClientTests/RequestValidationTests.swift b/Tests/AsyncHTTPClientTests/RequestValidationTests.swift index 96f692b3d..193a41b6b 100644 --- a/Tests/AsyncHTTPClientTests/RequestValidationTests.swift +++ b/Tests/AsyncHTTPClientTests/RequestValidationTests.swift @@ -323,15 +323,19 @@ class RequestValidationTests: XCTestCase { func testTraceMethodIsNotAllowedToHaveAFixedLengthBody() { var headers = HTTPHeaders() - XCTAssertThrowsError(try headers.validateAndSetTransportFraming(method: .TRACE, bodyLength: .fixed(length: 10))) + XCTAssertThrowsError(try headers.validateAndSetTransportFraming(method: .TRACE, bodyLength: .fixed(length: 10))) { + XCTAssertEqual($0 as? HTTPClientError, .traceRequestWithBody) + } } func testTraceMethodIsNotAllowedToHaveADynamicLengthBody() { var headers = HTTPHeaders() - XCTAssertThrowsError(try headers.validateAndSetTransportFraming(method: .TRACE, bodyLength: .dynamic)) + XCTAssertThrowsError(try headers.validateAndSetTransportFraming(method: .TRACE, bodyLength: .dynamic)) { + XCTAssertEqual($0 as? HTTPClientError, .traceRequestWithBody) + } } - func testTransferEncodingsAreOverwritteIfBodyLengthIsFixed() { + func testTransferEncodingsAreOverwrittenIfBodyLengthIsFixed() { var headers: HTTPHeaders = [ "Transfer-Encoding": "gzip, chunked", ] From 2e2140a696d0a15b6e09e5b2f446b18f1f94e6f1 Mon Sep 17 00:00:00 2001 From: David Nadoba Date: Mon, 22 Nov 2021 15:09:42 +0100 Subject: [PATCH 5/6] add `Hashable` conformance to `RequestBodyLength` --- Sources/AsyncHTTPClient/ConnectionPool/RequestBodyLength.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/AsyncHTTPClient/ConnectionPool/RequestBodyLength.swift b/Sources/AsyncHTTPClient/ConnectionPool/RequestBodyLength.swift index 648c74913..a963677dc 100644 --- a/Sources/AsyncHTTPClient/ConnectionPool/RequestBodyLength.swift +++ b/Sources/AsyncHTTPClient/ConnectionPool/RequestBodyLength.swift @@ -12,7 +12,7 @@ // //===----------------------------------------------------------------------===// -enum RequestBodyLength: Equatable { +enum RequestBodyLength: Hashable { /// size of the request body is not known before starting the request case dynamic /// size of the request body is fixed and exactly `length` bytes From 5713802f6f8f392b523ba91d6f8a09dd0d54ab54 Mon Sep 17 00:00:00 2001 From: David Nadoba Date: Mon, 22 Nov 2021 15:17:17 +0100 Subject: [PATCH 6/6] remove duplicate `be` from documentation --- Sources/AsyncHTTPClient/HTTPHandler.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/AsyncHTTPClient/HTTPHandler.swift b/Sources/AsyncHTTPClient/HTTPHandler.swift index 666800ee0..4da73531a 100644 --- a/Sources/AsyncHTTPClient/HTTPHandler.swift +++ b/Sources/AsyncHTTPClient/HTTPHandler.swift @@ -62,7 +62,7 @@ extension HTTPClient { /// Create and stream body using `StreamWriter`. /// /// - parameters: - /// - length: Body size. If nil, `Transfer-Encoding` will be automatically be set to `chunked`. Otherwise a `Content-Length` + /// - length: Body size. If nil, `Transfer-Encoding` will automatically be set to `chunked`. Otherwise a `Content-Length` /// header is set with the given `length`. /// - stream: Body chunk provider. public static func stream(length: Int? = nil, _ stream: @escaping (StreamWriter) -> EventLoopFuture) -> Body {