Skip to content

Commit b2dc4ae

Browse files
fabianfettfabianfett
committed
Failing tests
1 parent b0180d1 commit b2dc4ae

File tree

4 files changed

+87
-15
lines changed

4 files changed

+87
-15
lines changed

Diff for: ‎Sources/AsyncHTTPClient/ConnectionPool/HTTPRequestStateMachine+Demand.swift

+42-11
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,29 @@ extension HTTPRequestStateMachine {
5151
buffer.append(body)
5252
self.state = .waitingForBytes(buffer)
5353

54-
case .waitingForRead,
55-
.waitingForDemand,
56-
.waitingForReadOrDemand:
57-
preconditionFailure("How can we receive a body part, after a channelReadComplete, but no read has been forwarded yet. Invalid state: \(self.state)")
54+
// For all the following cases, please note:
55+
// Normally these code paths should never be hit. However there is one way to trigger
56+
// this:
57+
//
58+
// In HTTP/1, if a server does not send a Content-Length or Transfer-Encoding header
59+
// we don't know how long the response will be. The response will be terminated by
60+
// a connection close. If a connection is closed, NIO does not wait for a `.read()`
61+
// call before forwarding the final `channelRead`s.
62+
63+
case .waitingForRead(var buffer):
64+
self.state = .modifying
65+
buffer.append(body)
66+
self.state = .waitingForRead(buffer)
67+
68+
case .waitingForDemand(var buffer):
69+
self.state = .modifying
70+
buffer.append(body)
71+
self.state = .waitingForDemand(buffer)
72+
73+
case .waitingForReadOrDemand(var buffer):
74+
self.state = .modifying
75+
buffer.append(body)
76+
self.state = .waitingForReadOrDemand(buffer)
5877

5978
case .modifying:
6079
preconditionFailure("Invalid state: \(self.state)")
@@ -134,15 +153,27 @@ extension HTTPRequestStateMachine {
134153
}
135154
}
136155

137-
mutating func end() -> CircularBuffer<ByteBuffer> {
156+
enum ConnectionAction {
157+
case none
158+
case close
159+
}
160+
161+
mutating func end() -> (CircularBuffer<ByteBuffer>, ConnectionAction) {
138162
switch self.state {
139163
case .waitingForBytes(let buffer):
140-
return buffer
141-
142-
case .waitingForReadOrDemand,
143-
.waitingForRead,
144-
.waitingForDemand:
145-
preconditionFailure("How can we receive a body end, after a channelReadComplete, but no read has been forwarded yet. Invalid state: \(self.state)")
164+
return (buffer, .none)
165+
166+
case .waitingForReadOrDemand(let buffer),
167+
.waitingForRead(let buffer),
168+
.waitingForDemand(let buffer):
169+
// Normally this code path should never be hit. However there is one way to trigger
170+
// this:
171+
//
172+
// In HTTP/1, if a server does not send a Content-Length or Transfer-Encoding header
173+
// we don't know how long the response will be. The response will be terminated by
174+
// a connection close. If a connection is closed, NIO does not wait for a `.read()`
175+
// call before forwarding the final `channelRead`s.
176+
return (buffer, .close)
146177

147178
case .modifying:
148179
preconditionFailure("Invalid state: \(self.state)")

Diff for: ‎Sources/AsyncHTTPClient/ConnectionPool/HTTPRequestStateMachine.swift

+9-4
Original file line numberDiff line numberDiff line change
@@ -523,7 +523,7 @@ struct HTTPRequestStateMachine {
523523
where head.status.code < 300:
524524

525525
return self.avoidingStateMachineCoW { state -> Action in
526-
let remainingBuffer = responseStreamState.end()
526+
let (remainingBuffer, _) = responseStreamState.end()
527527
state = .running(
528528
.streaming(expectedBodyLength: expectedBodyLength, sentBodyBytes: sentBodyBytes, producer: producerState),
529529
.endReceived
@@ -536,16 +536,21 @@ struct HTTPRequestStateMachine {
536536
assert(producerState == .paused, "Expected to have paused the request body stream, when the head was received. Invalid state: \(self.state)")
537537

538538
return self.avoidingStateMachineCoW { state -> Action in
539-
let remainingBuffer = responseStreamState.end()
539+
let (remainingBuffer, _) = responseStreamState.end()
540540
state = .finished
541541
return .succeedRequest(.close, remainingBuffer)
542542
}
543543

544544
case .running(.endSent, .receivingBody(_, var responseStreamState)):
545545
return self.avoidingStateMachineCoW { state -> Action in
546-
let remainingBuffer = responseStreamState.end()
546+
let (remainingBuffer, action) = responseStreamState.end()
547547
state = .finished
548-
return .succeedRequest(.none, remainingBuffer)
548+
switch action {
549+
case .none:
550+
return .succeedRequest(.none, remainingBuffer)
551+
case .close:
552+
return .succeedRequest(.close, remainingBuffer)
553+
}
549554
}
550555

551556
case .running(_, .endReceived), .finished:

Diff for: ‎Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests+XCTest.swift

+2
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ extension HTTPRequestStateMachineTests {
5050
("testReadTimeoutThatFiresToLateIsIgnored", testReadTimeoutThatFiresToLateIsIgnored),
5151
("testCancellationThatIsInvokedToLateIsIgnored", testCancellationThatIsInvokedToLateIsIgnored),
5252
("testErrorWhileRunningARequestClosesTheStream", testErrorWhileRunningARequestClosesTheStream),
53+
("testCanReadHTTP1_0ResponseWithoutBody", testCanReadHTTP1_0ResponseWithoutBody),
54+
("testCanReadHTTP1_0ResponseWithBody", testCanReadHTTP1_0ResponseWithBody),
5355
]
5456
}
5557
}

Diff for: ‎Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests.swift

+34
Original file line numberDiff line numberDiff line change
@@ -451,6 +451,40 @@ class HTTPRequestStateMachineTests: XCTestCase {
451451
XCTAssertEqual(state.errorHappened(NIOSSLError.uncleanShutdown), .failRequest(NIOSSLError.uncleanShutdown, .close))
452452
XCTAssertEqual(state.requestCancelled(), .wait, "A cancellation that happens to late is ignored")
453453
}
454+
455+
func testCanReadHTTP1_0ResponseWithoutBody() {
456+
var state = HTTPRequestStateMachine(isChannelWritable: true)
457+
let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/")
458+
let metadata = RequestFramingMetadata(connectionClose: false, body: .none)
459+
XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false))
460+
461+
let responseHead = HTTPResponseHead(version: .http1_0, status: .internalServerError)
462+
XCTAssertEqual(state.channelRead(.head(responseHead)), .forwardResponseHead(responseHead, pauseRequestBodyStream: false))
463+
XCTAssertEqual(state.demandMoreResponseBodyParts(), .wait)
464+
XCTAssertEqual(state.channelReadComplete(), .wait)
465+
XCTAssertEqual(state.read(), .read)
466+
XCTAssertEqual(state.channelReadComplete(), .wait)
467+
XCTAssertEqual(state.channelRead(.end(nil)), .succeedRequest(.close, []))
468+
XCTAssertEqual(state.channelInactive(), .wait)
469+
}
470+
471+
func testCanReadHTTP1_0ResponseWithBody() {
472+
var state = HTTPRequestStateMachine(isChannelWritable: true)
473+
let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/")
474+
let metadata = RequestFramingMetadata(connectionClose: false, body: .none)
475+
XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false))
476+
477+
let responseHead = HTTPResponseHead(version: .http1_0, status: .internalServerError)
478+
let body = ByteBuffer(string: "foo bar")
479+
XCTAssertEqual(state.channelRead(.head(responseHead)), .forwardResponseHead(responseHead, pauseRequestBodyStream: false))
480+
XCTAssertEqual(state.demandMoreResponseBodyParts(), .wait)
481+
XCTAssertEqual(state.channelReadComplete(), .wait)
482+
XCTAssertEqual(state.read(), .read)
483+
XCTAssertEqual(state.channelReadComplete(), .wait)
484+
XCTAssertEqual(state.channelRead(.body(body)), .wait)
485+
XCTAssertEqual(state.channelRead(.end(nil)), .succeedRequest(.close, [body]))
486+
XCTAssertEqual(state.channelInactive(), .wait)
487+
}
454488
}
455489

456490
extension HTTPRequestStateMachine.Action: Equatable {

0 commit comments

Comments
 (0)