Skip to content

Commit 89ec767

Browse files
committed
Drain request when not done reading
1 parent 0362155 commit 89ec767

2 files changed

Lines changed: 74 additions & 13 deletions

File tree

Sources/NIOHTTPServer/HTTPRequestConcludingAsyncReader.swift

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public struct HTTPRequestConcludingAsyncReader: ConcludingAsyncReader, ~Copyable
3838
/// The type of errors that can occur during reading operations.
3939
public typealias ReadFailure = any Error
4040

41-
/// The HTTP trailer fields captured at the end of the request.
41+
/// The shared reader state that holds the iterator and captures trailers.
4242
fileprivate var state: ReaderState
4343

4444
struct RequestBodyStateMachine {
@@ -62,14 +62,9 @@ public struct HTTPRequestConcludingAsyncReader: ConcludingAsyncReader, ~Copyable
6262
private var state: State
6363
private var readerState: ReaderState
6464

65-
/// The iterator that provides HTTP request parts from the underlying channel.
66-
/// Taken from ReaderState once at the start of reading, and returned when reading completes.
67-
private var iterator: NIOAsyncChannelInboundStream<HTTPRequestPart>.AsyncIterator?
68-
6965
init(readerState: ReaderState) {
7066
self.state = .readingBody(.noExcess)
7167
self.readerState = readerState
72-
self.iterator = readerState.takeIterator()
7368
}
7469

7570
enum ReadResult {
@@ -90,26 +85,45 @@ public struct HTTPRequestConcludingAsyncReader: ConcludingAsyncReader, ~Copyable
9085

9186
case .noExcess:
9287
// There is no excess from previous reads. We obtain the next element from the stream.
93-
let requestPart = try await self.iterator?.next(isolation: #isolation)
88+
// Take the iterator from ReaderState, read one part, and put it back.
89+
// This ensures the iterator is always recoverable even if the reader
90+
// is dropped without consuming .end.
91+
guard var iterator = self.readerState.takeIterator() else {
92+
throw RequestBodyReadError.requestEndedBeforeReceivingEnd
93+
}
94+
95+
let requestPart: HTTPRequestPart?
96+
do {
97+
requestPart = try await iterator.next(isolation: #isolation)
98+
} catch {
99+
// Put the iterator back before propagating the error.
100+
nonisolated(unsafe) let iter = iterator
101+
self.readerState.putIterator(iter)
102+
throw error
103+
}
94104

95105
switch requestPart {
96106
case .head:
107+
nonisolated(unsafe) let iter = iterator
108+
self.readerState.putIterator(iter)
97109
fatalError("Unexpectedly received a request head.")
98110

99111
case .none:
112+
// Stream ended without .end — don't put iterator back.
100113
throw RequestBodyReadError.requestEndedBeforeReceivingEnd
101114

102115
case .body(let element):
116+
nonisolated(unsafe) let iter = iterator
117+
self.readerState.putIterator(iter)
103118
bodyElement = element
104119

105120
case .end(let trailers):
106121
self.state = .finished
107-
nonisolated(unsafe) let iterator = self.iterator.take()
122+
nonisolated(unsafe) let iter = iterator
123+
self.readerState.putIterator(iter)
108124
self.readerState.wrapped.withLock { state in
109125
state.finishedReading = true
110126
state.trailers = trailers
111-
let disconnected = Disconnected(value: iterator)
112-
state.iterator = disconnected.take()
113127
}
114128
return .requestFinished
115129
}
@@ -136,8 +150,6 @@ public struct HTTPRequestConcludingAsyncReader: ConcludingAsyncReader, ~Copyable
136150
var requestBodyStateMachine: RequestBodyStateMachine
137151

138152
/// Initializes a new request body reader.
139-
///
140-
/// Takes the iterator from ReaderState so the state machine owns it for the entire read cycle.
141153
fileprivate init(readerState: ReaderState) {
142154
self.requestBodyStateMachine = .init(readerState: readerState)
143155
self.state = readerState
@@ -197,6 +209,15 @@ public struct HTTPRequestConcludingAsyncReader: ConcludingAsyncReader, ~Copyable
197209
return iterator
198210
}
199211
}
212+
213+
func putIterator(
214+
_ iterator: consuming sending NIOAsyncChannelInboundStream<HTTPRequestPart>.AsyncIterator
215+
) {
216+
var disconnected = Disconnected(value: Optional(iterator))
217+
self.wrapped.withLock { state in
218+
state.iterator = disconnected.swap(newValue: nil)
219+
}
220+
}
200221
}
201222

202223
/// The underlying reader type for the HTTP request body.

Sources/NIOHTTPServer/NIOHTTPServer.swift

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,47 @@ public struct NIOHTTPServer: HTTPServer {
273273
throw error
274274
}
275275

276-
return readerState.takeIterator()
276+
// If the handler didn't properly conclude the response, the HTTP codec
277+
// is in an inconsistent state and the connection cannot be reused.
278+
if !writerState.wrapped.withLock({ $0.finishedWriting }) {
279+
self.logger.debug("Handler did not conclude the response. Closing connection.")
280+
return nil
281+
}
282+
283+
// Recover the iterator for potential connection reuse.
284+
guard var recoveredIterator = readerState.takeIterator() else {
285+
// The handler started reading the request body but didn't finish.
286+
// The iterator was consumed by the reader and not returned.
287+
return nil
288+
}
289+
290+
// If the handler didn't fully consume the request body, drain the remaining
291+
// parts so the iterator is positioned at the start of the next request.
292+
// Errors during draining are not propagated — if the drain fails, we simply
293+
// cannot reuse this connection.
294+
if !readerState.wrapped.withLock({ $0.finishedReading }) {
295+
do {
296+
drainLoop: while true {
297+
switch try await recoveredIterator.next(isolation: #isolation) {
298+
case .head:
299+
self.logger.debug(
300+
"Unexpectedly received request head while draining unconsumed request body."
301+
)
302+
return nil
303+
case .body:
304+
continue drainLoop
305+
case .end:
306+
break drainLoop
307+
case .none:
308+
return nil
309+
}
310+
}
311+
} catch {
312+
return nil
313+
}
314+
}
315+
316+
return recoveredIterator
277317
}
278318

279319
/// Fail the listening address promise if the server is shutting down before it began listening.

0 commit comments

Comments
 (0)