diff --git a/Sources/AsyncHTTPClient/HTTPClient.swift b/Sources/AsyncHTTPClient/HTTPClient.swift index 548f038ad..f736d9579 100644 --- a/Sources/AsyncHTTPClient/HTTPClient.swift +++ b/Sources/AsyncHTTPClient/HTTPClient.swift @@ -754,6 +754,7 @@ public struct HTTPClientError: Error, Equatable, CustomStringConvertible { case redirectLimitReached case redirectCycleDetected case uncleanShutdown + case traceRequestWithBody } private var code: Code @@ -798,4 +799,6 @@ public struct HTTPClientError: Error, Equatable, CustomStringConvertible { public static let redirectCycleDetected = HTTPClientError(code: .redirectCycleDetected) /// Unclean shutdown public static let uncleanShutdown = HTTPClientError(code: .uncleanShutdown) + /// A body was sent in a request with method TRACE + public static let traceRequestWithBody = HTTPClientError(code: .traceRequestWithBody) } diff --git a/Sources/AsyncHTTPClient/HTTPHandler.swift b/Sources/AsyncHTTPClient/HTTPHandler.swift index 921c1e678..b77ab02d1 100644 --- a/Sources/AsyncHTTPClient/HTTPHandler.swift +++ b/Sources/AsyncHTTPClient/HTTPHandler.swift @@ -771,7 +771,7 @@ extension TaskHandler: ChannelDuplexHandler { } do { - try headers.validate(body: request.body) + try headers.validate(method: request.method, body: request.body) } catch { promise?.fail(error) context.fireErrorCaught(error) diff --git a/Sources/AsyncHTTPClient/RequestValidation.swift b/Sources/AsyncHTTPClient/RequestValidation.swift index be5d7f10f..0802da0ad 100644 --- a/Sources/AsyncHTTPClient/RequestValidation.swift +++ b/Sources/AsyncHTTPClient/RequestValidation.swift @@ -16,7 +16,7 @@ import NIO import NIOHTTP1 extension HTTPHeaders { - mutating func validate(body: HTTPClient.Body?) throws { + 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) var transferEncoding: String? var contentLength: Int? @@ -29,33 +29,55 @@ extension HTTPHeaders { self.remove(name: "Transfer-Encoding") self.remove(name: "Content-Length") - if let body = body { - guard (encodings.filter { $0 == "chunked" }.count <= 1) else { - throw HTTPClientError.chunkedSpecifiedMultipleTimes + guard let body = body else { + // 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. + return + 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 } + } + + 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 { + guard (encodings.filter { $0 == "chunked" }.count <= 1) else { + throw HTTPClientError.chunkedSpecifiedMultipleTimes + } + + if encodings.isEmpty { + guard let length = body.length else { + throw HTTPClientError.contentLengthMissing + } + contentLength = length + } else { + transferEncoding = encodings.joined(separator: ", ") + if !encodings.contains("chunked") { guard let length = body.length else { throw HTTPClientError.contentLengthMissing } contentLength = length - } else { - transferEncoding = encodings.joined(separator: ", ") - if !encodings.contains("chunked") { - guard let length = body.length else { - throw HTTPClientError.contentLengthMissing - } - contentLength = length - } } - } else { - contentLength = 0 } + // add headers if required if let enc = transferEncoding { self.add(name: "Transfer-Encoding", value: enc) - } - if let length = contentLength { + } 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)) } } diff --git a/Tests/AsyncHTTPClientTests/RequestValidationTests+XCTest.swift b/Tests/AsyncHTTPClientTests/RequestValidationTests+XCTest.swift new file mode 100644 index 000000000..b6145a4b2 --- /dev/null +++ b/Tests/AsyncHTTPClientTests/RequestValidationTests+XCTest.swift @@ -0,0 +1,35 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the AsyncHTTPClient open source project +// +// Copyright (c) 2018-2019 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 +// +//===----------------------------------------------------------------------===// +// +// RequestValidationTests+XCTest.swift +// +import XCTest + +/// +/// NOTE: This file was generated by generate_linux_tests.rb +/// +/// Do NOT edit this file directly as it will be regenerated automatically when needed. +/// + +extension RequestValidationTests { + static var allTests: [(String, (RequestValidationTests) -> () throws -> Void)] { + return [ + ("testContentLengthHeaderIsRemovedFromGETIfNoBody", testContentLengthHeaderIsRemovedFromGETIfNoBody), + ("testContentLengthHeaderIsAddedToPOSTAndPUTWithNoBody", testContentLengthHeaderIsAddedToPOSTAndPUTWithNoBody), + ("testContentLengthHeaderIsChangedIfBodyHasDifferentLength", testContentLengthHeaderIsChangedIfBodyHasDifferentLength), + ("testChunkedEncodingDoesNotHaveContentLengthHeader", testChunkedEncodingDoesNotHaveContentLengthHeader), + ("testTRACERequestMustNotHaveBody", testTRACERequestMustNotHaveBody), + ] + } +} diff --git a/Tests/AsyncHTTPClientTests/RequestValidationTests.swift b/Tests/AsyncHTTPClientTests/RequestValidationTests.swift new file mode 100644 index 000000000..72dfa3da8 --- /dev/null +++ b/Tests/AsyncHTTPClientTests/RequestValidationTests.swift @@ -0,0 +1,73 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the AsyncHTTPClient open source project +// +// Copyright (c) 2018-2019 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 +// +//===----------------------------------------------------------------------===// + +@testable import AsyncHTTPClient +import NIO +import NIOHTTP1 +import XCTest + +class RequestValidationTests: XCTestCase { + func testContentLengthHeaderIsRemovedFromGETIfNoBody() { + var headers = HTTPHeaders([("Content-Length", "0")]) + XCTAssertNoThrow(try headers.validate(method: .GET, body: .none)) + XCTAssertNil(headers.first(name: "Content-Length")) + } + + func testContentLengthHeaderIsAddedToPOSTAndPUTWithNoBody() { + var putHeaders = HTTPHeaders() + XCTAssertNoThrow(try putHeaders.validate(method: .PUT, body: .none)) + XCTAssertEqual(putHeaders.first(name: "Content-Length"), "0") + + var postHeaders = HTTPHeaders() + XCTAssertNoThrow(try postHeaders.validate(method: .POST, body: .none)) + XCTAssertEqual(postHeaders.first(name: "Content-Length"), "0") + } + + func testContentLengthHeaderIsChangedIfBodyHasDifferentLength() { + var headers = HTTPHeaders([("Content-Length", "0")]) + var buffer = ByteBufferAllocator().buffer(capacity: 200) + buffer.writeBytes([UInt8](repeating: 12, count: 200)) + XCTAssertNoThrow(try headers.validate(method: .PUT, body: .byteBuffer(buffer))) + 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) + } + } +} diff --git a/Tests/LinuxMain.swift b/Tests/LinuxMain.swift index b46ac9e2f..93e594a06 100644 --- a/Tests/LinuxMain.swift +++ b/Tests/LinuxMain.swift @@ -29,5 +29,6 @@ import XCTest testCase(HTTPClientCookieTests.allTests), testCase(HTTPClientInternalTests.allTests), testCase(HTTPClientTests.allTests), + testCase(RequestValidationTests.allTests), ]) #endif