Skip to content

Commit 61a80a2

Browse files
artemredkinartemredkin
authored
assume chunked on a stream with no length (#247)
Motivation: Streams length parameter is optional to allow cases were stream length is not known in advance, but we do not support this in request validation. This PR aims to address that. Modifications: Modifies request validation to default to chunked encoding if body length is zero or to passed in content-length header Adds a test Result: Closes #218
1 parent aac4357 commit 61a80a2

File tree

6 files changed

+237
-38
lines changed

6 files changed

+237
-38
lines changed

‎Sources/AsyncHTTPClient/HTTPClient.swift‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -967,6 +967,7 @@ public struct HTTPClientError: Error, Equatable, CustomStringConvertible {
967967
case traceRequestWithBody
968968
case invalidHeaderFieldNames([String])
969969
case bodyLengthMismatch
970+
case incompatibleHeaders
970971
}
971972

972973
private var code: Code
@@ -1019,4 +1020,6 @@ public struct HTTPClientError: Error, Equatable, CustomStringConvertible {
10191020
public static func invalidHeaderFieldNames(_ names: [String]) -> HTTPClientError { return HTTPClientError(code: .invalidHeaderFieldNames(names)) }
10201021
/// Body length is not equal to `Content-Length`.
10211022
public static let bodyLengthMismatch = HTTPClientError(code: .bodyLengthMismatch)
1023+
/// Incompatible headers specified, for example `Transfer-Encoding` and `Content-Length`.
1024+
public static let incompatibleHeaders = HTTPClientError(code: .incompatibleHeaders)
10221025
}

‎Sources/AsyncHTTPClient/RequestValidation.swift‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@ import NIOHTTP1
1818
extension HTTPHeaders {
1919
mutating func validate(method: HTTPMethod, body: HTTPClient.Body?) throws {
2020
// validate transfer encoding and content length (https://tools.ietf.org/html/rfc7230#section-3.3.1)
21+
if self.contains(name: "Transfer-Encoding"), self.contains(name: "Content-Length") {
22+
throw HTTPClientError.incompatibleHeaders
23+
}
24+
2125
var transferEncoding: String?
2226
var contentLength: Int?
2327
let encodings = self[canonicalForm: "Transfer-Encoding"].map { $0.lowercased() }
@@ -27,11 +31,11 @@ extension HTTPHeaders {
2731
}
2832

2933
self.remove(name: "Transfer-Encoding")
30-
self.remove(name: "Content-Length")
3134

3235
try self.validateFieldNames()
3336

3437
guard let body = body else {
38+
self.remove(name: "Content-Length")
3539
// if we don't have a body we might not need to send the Content-Length field
3640
// https://tools.ietf.org/html/rfc7230#section-3.3.2
3741
switch method {
@@ -60,11 +64,15 @@ extension HTTPHeaders {
6064
}
6165

6266
if encodings.isEmpty {
63-
guard let length = body.length else {
64-
throw HTTPClientError.contentLengthMissing
67+
if let length = body.length {
68+
self.remove(name: "Content-Length")
69+
contentLength = length
70+
} else if !self.contains(name: "Content-Length") {
71+
transferEncoding = "chunked"
6572
}
66-
contentLength = length
6773
} else {
74+
self.remove(name: "Content-Length")
75+
6876
transferEncoding = encodings.joined(separator: ", ")
6977
if !encodings.contains("chunked") {
7078
guard let length = body.length else {

‎Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ extension HTTPClientTests {
114114
("testNothingIsLoggedAtInfoOrHigher", testNothingIsLoggedAtInfoOrHigher),
115115
("testAllMethodsLog", testAllMethodsLog),
116116
("testClosingIdleConnectionsInPoolLogsInTheBackground", testClosingIdleConnectionsInPoolLogsInTheBackground),
117+
("testUploadStreamingNoLength", testUploadStreamingNoLength),
117118
("testConnectErrorPropagatedToDelegate", testConnectErrorPropagatedToDelegate),
118119
("testDelegateCallinsTolerateRandomEL", testDelegateCallinsTolerateRandomEL),
119120
("testContentLengthTooLongFails", testContentLengthTooLongFails),

‎Tests/AsyncHTTPClientTests/HTTPClientTests.swift‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2374,6 +2374,37 @@ class HTTPClientTests: XCTestCase {
23742374
self.defaultClient = nil // so it doesn't get shut down again.
23752375
}
23762376

2377+
func testUploadStreamingNoLength() throws {
2378+
let server = NIOHTTP1TestServer(group: self.serverGroup)
2379+
let client = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup))
2380+
defer {
2381+
XCTAssertNoThrow(try client.syncShutdown())
2382+
XCTAssertNoThrow(try server.stop())
2383+
}
2384+
2385+
var request = try HTTPClient.Request(url: "http://localhost:\(server.serverPort)/")
2386+
request.body = .stream { writer in
2387+
writer.write(.byteBuffer(ByteBuffer(string: "1234")))
2388+
}
2389+
2390+
let future = client.execute(request: request)
2391+
2392+
switch try server.readInbound() {
2393+
case .head(let head):
2394+
XCTAssertEqual(head.headers["transfer-encoding"], ["chunked"])
2395+
default:
2396+
XCTFail("Unexpected part")
2397+
}
2398+
2399+
XCTAssertNoThrow(try server.readInbound()) // .body
2400+
XCTAssertNoThrow(try server.readInbound()) // .end
2401+
2402+
XCTAssertNoThrow(try server.writeOutbound(.head(.init(version: .init(major: 1, minor: 1), status: .ok))))
2403+
XCTAssertNoThrow(try server.writeOutbound(.end(nil)))
2404+
2405+
XCTAssertNoThrow(try future.wait())
2406+
}
2407+
23772408
func testConnectErrorPropagatedToDelegate() throws {
23782409
class TestDelegate: HTTPClientResponseDelegate {
23792410
typealias Response = Void

‎Tests/AsyncHTTPClientTests/RequestValidationTests+XCTest.swift‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,18 @@ extension RequestValidationTests {
2828
("testContentLengthHeaderIsRemovedFromGETIfNoBody", testContentLengthHeaderIsRemovedFromGETIfNoBody),
2929
("testContentLengthHeaderIsAddedToPOSTAndPUTWithNoBody", testContentLengthHeaderIsAddedToPOSTAndPUTWithNoBody),
3030
("testContentLengthHeaderIsChangedIfBodyHasDifferentLength", testContentLengthHeaderIsChangedIfBodyHasDifferentLength),
31-
("testChunkedEncodingDoesNotHaveContentLengthHeader", testChunkedEncodingDoesNotHaveContentLengthHeader),
3231
("testTRACERequestMustNotHaveBody", testTRACERequestMustNotHaveBody),
3332
("testGET_HEAD_DELETE_CONNECTRequestCanHaveBody", testGET_HEAD_DELETE_CONNECTRequestCanHaveBody),
3433
("testInvalidHeaderFieldNames", testInvalidHeaderFieldNames),
3534
("testValidHeaderFieldNames", testValidHeaderFieldNames),
36-
("testMultipleContentLengthOnNilStreamLength", testMultipleContentLengthOnNilStreamLength),
35+
("testNoHeadersNoBody", testNoHeadersNoBody),
36+
("testNoHeadersHasBody", testNoHeadersHasBody),
37+
("testContentLengthHeaderNoBody", testContentLengthHeaderNoBody),
38+
("testContentLengthHeaderHasBody", testContentLengthHeaderHasBody),
39+
("testTransferEncodingHeaderNoBody", testTransferEncodingHeaderNoBody),
40+
("testTransferEncodingHeaderHasBody", testTransferEncodingHeaderHasBody),
41+
("testBothHeadersNoBody", testBothHeadersNoBody),
42+
("testBothHeadersHasBody", testBothHeadersHasBody),
3743
]
3844
}
3945
}

‎Tests/AsyncHTTPClientTests/RequestValidationTests.swift‎

Lines changed: 182 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -42,32 +42,14 @@ class RequestValidationTests: XCTestCase {
4242
XCTAssertEqual(headers.first(name: "Content-Length"), "200")
4343
}
4444

45-
func testChunkedEncodingDoesNotHaveContentLengthHeader() {
46-
var headers = HTTPHeaders([
47-
("Content-Length", "200"),
48-
("Transfer-Encoding", "chunked"),
49-
])
50-
var buffer = ByteBufferAllocator().buffer(capacity: 200)
51-
buffer.writeBytes([UInt8](repeating: 12, count: 200))
52-
XCTAssertNoThrow(try headers.validate(method: .PUT, body: .byteBuffer(buffer)))
53-
54-
// https://tools.ietf.org/html/rfc7230#section-3.3.2
55-
// A sender MUST NOT send a Content-Length header field in any message
56-
// that contains a Transfer-Encoding header field.
57-
58-
XCTAssertNil(headers.first(name: "Content-Length"))
59-
XCTAssertEqual(headers.first(name: "Transfer-Encoding"), "chunked")
60-
}
61-
6245
func testTRACERequestMustNotHaveBody() {
63-
var headers = HTTPHeaders([
64-
("Content-Length", "200"),
65-
("Transfer-Encoding", "chunked"),
66-
])
67-
var buffer = ByteBufferAllocator().buffer(capacity: 200)
68-
buffer.writeBytes([UInt8](repeating: 12, count: 200))
69-
XCTAssertThrowsError(try headers.validate(method: .TRACE, body: .byteBuffer(buffer))) {
70-
XCTAssertEqual($0 as? HTTPClientError, .traceRequestWithBody)
46+
for header in [("Content-Length", "200"), ("Transfer-Encoding", "chunked")] {
47+
var headers = HTTPHeaders([header])
48+
var buffer = ByteBufferAllocator().buffer(capacity: 200)
49+
buffer.writeBytes([UInt8](repeating: 12, count: 200))
50+
XCTAssertThrowsError(try headers.validate(method: .TRACE, body: .byteBuffer(buffer))) {
51+
XCTAssertEqual($0 as? HTTPClientError, .traceRequestWithBody)
52+
}
7153
}
7254
}
7355

@@ -105,13 +87,181 @@ class RequestValidationTests: XCTestCase {
10587
XCTAssertNoThrow(try headers.validate(method: .GET, body: nil))
10688
}
10789

108-
func testMultipleContentLengthOnNilStreamLength() {
109-
var headers = HTTPHeaders([("Content-Length", "1"), ("Content-Length", "2")])
110-
var buffer = ByteBufferAllocator().buffer(capacity: 10)
111-
buffer.writeBytes([UInt8](repeating: 12, count: 10))
112-
let body: HTTPClient.Body = .stream { writer in
113-
writer.write(.byteBuffer(buffer))
90+
// MARK: - Content-Length/Transfer-Encoding Matrix
91+
92+
// Method kind User sets Body Expectation
93+
// ----------------------------------------------------------------------------------
94+
// .GET, .HEAD, .DELETE, .CONNECT, .TRACE nothing nil Neither CL nor chunked
95+
// other nothing nil CL=0
96+
func testNoHeadersNoBody() throws {
97+
for method: HTTPMethod in [.GET, .HEAD, .DELETE, .CONNECT, .TRACE] {
98+
var headers: HTTPHeaders = .init()
99+
XCTAssertNoThrow(try headers.validate(method: method, body: nil))
100+
XCTAssertTrue(headers["content-length"].isEmpty)
101+
XCTAssertTrue(headers["transfer-encoding"].isEmpty)
102+
}
103+
104+
for method: HTTPMethod in [.POST, .PUT] {
105+
var headers: HTTPHeaders = .init()
106+
XCTAssertNoThrow(try headers.validate(method: method, body: nil))
107+
XCTAssertEqual(headers["content-length"].first, "0")
108+
XCTAssertFalse(headers["transfer-encoding"].contains("chunked"))
109+
}
110+
}
111+
112+
// Method kind User sets Body Expectation
113+
// --------------------------------------------------------------------------------------
114+
// .GET, .HEAD, .DELETE, .CONNECT, .TRACE nothing not nil CL or chunked
115+
// other nothing not nil CL or chunked
116+
func testNoHeadersHasBody() throws {
117+
// Body length is known
118+
for method: HTTPMethod in [.GET, .HEAD, .DELETE, .CONNECT] {
119+
var headers: HTTPHeaders = .init()
120+
XCTAssertNoThrow(try headers.validate(method: method, body: .byteBuffer(ByteBuffer(bytes: [0]))))
121+
XCTAssertEqual(headers["content-length"].first, "1")
122+
XCTAssertTrue(headers["transfer-encoding"].isEmpty)
123+
}
124+
125+
// Body length is _not_ known
126+
for method: HTTPMethod in [.GET, .HEAD, .DELETE, .CONNECT] {
127+
var headers: HTTPHeaders = .init()
128+
let body: HTTPClient.Body = .stream { writer in
129+
writer.write(.byteBuffer(ByteBuffer(bytes: [0])))
130+
}
131+
XCTAssertNoThrow(try headers.validate(method: method, body: body))
132+
XCTAssertTrue(headers["content-length"].isEmpty)
133+
XCTAssertTrue(headers["transfer-encoding"].contains("chunked"))
134+
}
135+
136+
// Body length is known
137+
for method: HTTPMethod in [.POST, .PUT] {
138+
var headers: HTTPHeaders = .init()
139+
XCTAssertNoThrow(try headers.validate(method: method, body: .byteBuffer(ByteBuffer(bytes: [0]))))
140+
XCTAssertEqual(headers["content-length"].first, "1")
141+
XCTAssertTrue(headers["transfer-encoding"].isEmpty)
142+
}
143+
144+
// Body length is _not_ known
145+
for method: HTTPMethod in [.POST, .PUT] {
146+
var headers: HTTPHeaders = .init()
147+
let body: HTTPClient.Body = .stream { writer in
148+
writer.write(.byteBuffer(ByteBuffer(bytes: [0])))
149+
}
150+
XCTAssertNoThrow(try headers.validate(method: method, body: body))
151+
XCTAssertTrue(headers["content-length"].isEmpty)
152+
XCTAssertTrue(headers["transfer-encoding"].contains("chunked"))
153+
}
154+
}
155+
156+
// Method kind User sets Body Expectation
157+
// ------------------------------------------------------------------------------
158+
// .GET, .HEAD, .DELETE, .CONNECT, .TRACE content-length nil Neither CL nor chunked
159+
// other content-length nil CL=0
160+
func testContentLengthHeaderNoBody() throws {
161+
for method: HTTPMethod in [.GET, .HEAD, .DELETE, .CONNECT, .TRACE] {
162+
var headers: HTTPHeaders = .init([("Content-Length", "1")])
163+
XCTAssertNoThrow(try headers.validate(method: method, body: nil))
164+
XCTAssertTrue(headers["content-length"].isEmpty)
165+
XCTAssertTrue(headers["transfer-encoding"].isEmpty)
166+
}
167+
168+
for method: HTTPMethod in [.POST, .PUT] {
169+
var headers: HTTPHeaders = .init([("Content-Length", "1")])
170+
XCTAssertNoThrow(try headers.validate(method: method, body: nil))
171+
XCTAssertEqual(headers["content-length"].first, "0")
172+
XCTAssertTrue(headers["transfer-encoding"].isEmpty)
173+
}
174+
}
175+
176+
// Method kind User sets Body Expectation
177+
// --------------------------------------------------------------------------
178+
// .GET, .HEAD, .DELETE, .CONNECT content-length not nil CL=1
179+
// other content-length nit nil CL=1
180+
func testContentLengthHeaderHasBody() throws {
181+
for method: HTTPMethod in [.GET, .HEAD, .DELETE, .CONNECT] {
182+
var headers: HTTPHeaders = .init([("Content-Length", "1")])
183+
XCTAssertNoThrow(try headers.validate(method: method, body: .byteBuffer(ByteBuffer(bytes: [0]))))
184+
XCTAssertEqual(headers["content-length"].first, "1")
185+
XCTAssertTrue(headers["transfer-encoding"].isEmpty)
186+
}
187+
188+
for method: HTTPMethod in [.POST, .PUT] {
189+
var headers: HTTPHeaders = .init([("Content-Length", "1")])
190+
XCTAssertNoThrow(try headers.validate(method: method, body: .byteBuffer(ByteBuffer(bytes: [0]))))
191+
XCTAssertEqual(headers["content-length"].first, "1")
192+
XCTAssertTrue(headers["transfer-encoding"].isEmpty)
193+
}
194+
}
195+
196+
// Method kind User sets Body Expectation
197+
// ------------------------------------------------------------------------------------------
198+
// .GET, .HEAD, .DELETE, .CONNECT, .TRACE transfer-encoding: chunked nil nil
199+
// other transfer-encoding: chunked nil nil
200+
func testTransferEncodingHeaderNoBody() throws {
201+
for method: HTTPMethod in [.GET, .HEAD, .DELETE, .CONNECT, .TRACE] {
202+
var headers: HTTPHeaders = .init([("Transfer-Encoding", "chunked")])
203+
XCTAssertNoThrow(try headers.validate(method: method, body: nil))
204+
XCTAssertTrue(headers["content-length"].isEmpty)
205+
XCTAssertFalse(headers["transfer-encoding"].contains("chunked"))
206+
}
207+
208+
for method: HTTPMethod in [.POST, .PUT] {
209+
var headers: HTTPHeaders = .init([("Transfer-Encoding", "chunked")])
210+
XCTAssertNoThrow(try headers.validate(method: method, body: nil))
211+
XCTAssertEqual(headers["content-length"].first, "0")
212+
XCTAssertFalse(headers["transfer-encoding"].contains("chunked"))
213+
}
214+
}
215+
216+
// Method kind User sets Body Expectation
217+
// --------------------------------------------------------------------------------------
218+
// .GET, .HEAD, .DELETE, .CONNECT transfer-encoding: chunked not nil chunked
219+
// other transfer-encoding: chunked not nil chunked
220+
func testTransferEncodingHeaderHasBody() throws {
221+
for method: HTTPMethod in [.GET, .HEAD, .DELETE, .CONNECT] {
222+
var headers: HTTPHeaders = .init([("Transfer-Encoding", "chunked")])
223+
XCTAssertNoThrow(try headers.validate(method: method, body: .byteBuffer(ByteBuffer(bytes: [0]))))
224+
XCTAssertTrue(headers["content-length"].isEmpty)
225+
XCTAssertTrue(headers["transfer-encoding"].contains("chunked"))
226+
}
227+
228+
for method: HTTPMethod in [.POST, .PUT] {
229+
var headers: HTTPHeaders = .init([("Transfer-Encoding", "chunked")])
230+
XCTAssertNoThrow(try headers.validate(method: method, body: .byteBuffer(ByteBuffer(bytes: [0]))))
231+
XCTAssertTrue(headers["content-length"].isEmpty)
232+
XCTAssertTrue(headers["transfer-encoding"].contains("chunked"))
233+
}
234+
}
235+
236+
// Method kind User sets Body Expectation
237+
// ---------------------------------------------------------------------------------------
238+
// .GET, .HEAD, .DELETE, .CONNECT, .TRACE CL & chunked (illegal) nil throws error
239+
// other CL & chunked (illegal) nil throws error
240+
func testBothHeadersNoBody() throws {
241+
for method: HTTPMethod in [.GET, .HEAD, .DELETE, .CONNECT, .TRACE] {
242+
var headers: HTTPHeaders = .init([("Content-Length", "1"), ("Transfer-Encoding", "chunked")])
243+
XCTAssertThrowsError(try headers.validate(method: method, body: nil))
244+
}
245+
246+
for method: HTTPMethod in [.POST, .PUT] {
247+
var headers: HTTPHeaders = .init([("Content-Length", "1"), ("Transfer-Encoding", "chunked")])
248+
XCTAssertThrowsError(try headers.validate(method: method, body: nil))
249+
}
250+
}
251+
252+
// Method kind User sets Body Expectation
253+
// -------------------------------------------------------------------------------------------
254+
// .GET, .HEAD, .DELETE, .CONNECT, .TRACE CL & chunked (illegal) not nil throws error
255+
// other CL & chunked (illegal) not nil throws error
256+
func testBothHeadersHasBody() throws {
257+
for method: HTTPMethod in [.GET, .HEAD, .DELETE, .CONNECT, .TRACE] {
258+
var headers: HTTPHeaders = .init([("Content-Length", "1"), ("Transfer-Encoding", "chunked")])
259+
XCTAssertThrowsError(try headers.validate(method: method, body: .byteBuffer(ByteBuffer(bytes: [0]))))
260+
}
261+
262+
for method: HTTPMethod in [.POST, .PUT] {
263+
var headers: HTTPHeaders = .init([("Content-Length", "1"), ("Transfer-Encoding", "chunked")])
264+
XCTAssertThrowsError(try headers.validate(method: method, body: .byteBuffer(ByteBuffer(bytes: [0]))))
114265
}
115-
XCTAssertThrowsError(try headers.validate(method: .PUT, body: body))
116266
}
117267
}

0 commit comments

Comments
 (0)