HTTP: assert that user doesn't set CL & TE:chunked (#1556)

Motivation:

NIO users need to understand the semantic of the underlying protocol.
Therefore NIO does not (and in many case cannot) check all the
invariants. Generally however, we try to be helpful and assert a bunch
of things that a user has (hopefully) checked. That doesn't affect
performance and simplifies development for users.

Modifications:

- assert that request/response heads don't set content-length +
  transfer-encoding: chunked

Result:

Developers of libraries like AHC will have an easier time.
This commit is contained in:
Johannes Weiss 2020-06-15 12:36:19 +01:00 committed by GitHub
parent 5268726898
commit 7b93c22836
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 57 additions and 4 deletions

View File

@ -16,4 +16,5 @@ let crashTestSuites: [String: Any] = [
"EventLoopCrashTests": EventLoopCrashTests(),
"ByteBufferCrashTests": ByteBufferCrashTests(),
"SystemCrashTests": SystemCrashTests(),
"HTTPCrashTests": HTTPCrashTests(),
]

View File

@ -0,0 +1,43 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the SwiftNIO open source project
//
// Copyright (c) 2020 Apple Inc. and the SwiftNIO project authors
// Licensed under Apache License v2.0
//
// See LICENSE.txt for license information
// See CONTRIBUTORS.txt for the list of SwiftNIO project authors
//
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//
import NIO
import NIOHTTP1
struct HTTPCrashTests {
let testEncodingChunkedAndContentLengthForRequestsCrashes = CrashTest(
regex: "^Assertion failed: illegal HTTP sent: HTTPRequestHead .* contains both a content-length and transfer-encoding:chunked",
{
let channel = EmbeddedChannel(handler: HTTPRequestEncoder())
_ = try? channel.writeAndFlush(
HTTPClientRequestPart.head(
HTTPRequestHead(version: .init(major: 1, minor: 1),
method: .POST,
uri: "/",
headers: ["content-Length": "1",
"transfer-Encoding": "chunked"]))).wait()
})
let testEncodingChunkedAndContentLengthForResponseCrashes = CrashTest(
regex: "^Assertion failed: illegal HTTP sent: HTTPResponseHead .* contains both a content-length and transfer-encoding:chunked",
{
let channel = EmbeddedChannel(handler: HTTPResponseEncoder())
_ = try? channel.writeAndFlush(
HTTPServerResponsePart.head(
HTTPResponseHead(version: .init(major: 1, minor: 1),
status: .ok,
headers: ["content-Length": "1",
"transfer-Encoding": "chunked"]))).wait()
})
}

View File

@ -65,7 +65,9 @@ private final class GrepHandler: ChannelInboundHandler {
func channelRead(context: ChannelHandlerContext, data: NIOAny) {
let line = self.unwrapInboundIn(data)
if line.lowercased().starts(with: "fatal error: ") || line.lowercased().starts(with: "precondition failed: ") {
if line.lowercased().starts(with: "fatal error: ") ||
line.lowercased().starts(with: "precondition failed: ") ||
line.lowercased().starts(with: "assertion failed: ") {
self.promise.succeed(line)
context.close(promise: nil)
}

View File

@ -143,7 +143,11 @@ public final class HTTPRequestEncoder: ChannelOutboundHandler, RemovableChannelH
public func write(context: ChannelHandlerContext, data: NIOAny, promise: EventLoopPromise<Void>?) {
switch self.unwrapOutboundIn(data) {
case .head(var request):
self.isChunked = sanitizeTransportHeaders(hasBody: request.method.hasRequestBody, headers: &request.headers, version: request.version) == .chunked
assert(!(request.headers.contains(name: "content-length") &&
request.headers[canonicalForm: "transfer-encoding"].contains("chunked"[...])),
"illegal HTTP sent: \(request) contains both a content-length and transfer-encoding:chunked")
self.isChunked = sanitizeTransportHeaders(hasBody: request.method.hasRequestBody,
headers: &request.headers, version: request.version) == .chunked
writeHead(wrapOutboundOut: self.wrapOutboundOut, writeStartLine: { buffer in
buffer.write(request: request)
@ -177,8 +181,11 @@ public final class HTTPResponseEncoder: ChannelOutboundHandler, RemovableChannel
public func write(context: ChannelHandlerContext, data: NIOAny, promise: EventLoopPromise<Void>?) {
switch self.unwrapOutboundIn(data) {
case .head(var response):
self.isChunked = sanitizeTransportHeaders(hasBody: response.status.mayHaveResponseBody ? .yes : .no, headers: &response.headers, version: response.version) == .chunked
assert(!(response.headers.contains(name: "content-length") &&
response.headers[canonicalForm: "transfer-encoding"].contains("chunked"[...])),
"illegal HTTP sent: \(response) contains both a content-length and transfer-encoding:chunked")
self.isChunked = sanitizeTransportHeaders(hasBody: response.status.mayHaveResponseBody ? .yes : .no,
headers: &response.headers, version: response.version) == .chunked
writeHead(wrapOutboundOut: self.wrapOutboundOut, writeStartLine: { buffer in
buffer.write(response: response)