Skip to content

Commit 619f10b

Browse files
committed
dropped writes at handler removal errors
unbuffering writes now flushes correctly if the buffer is flushed during unbuffering
1 parent 676e781 commit 619f10b

File tree

2 files changed

+42
-16
lines changed

2 files changed

+42
-16
lines changed

Sources/NIOExtras/HTTP1ProxyConnectHandler.swift

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -92,22 +92,13 @@ public final class NIOHTTP1ProxyConnectHandler: ChannelDuplexHandler, RemovableC
9292
self.failWithError(.noResult(), context: context)
9393

9494
case .completed:
95-
let hadMark = self.bufferedWrittenMessages.hasMark
96-
while self.bufferedWrittenMessages.hasMark {
97-
// write until mark
98-
let bufferedPart = self.bufferedWrittenMessages.removeFirst()
99-
context.write(bufferedPart.data, promise: bufferedPart.promise)
100-
}
101-
102-
// flush any messages up to the mark
103-
if hadMark {
104-
context.flush()
105-
}
106-
107-
// write remainder
108-
while let bufferedPart = self.bufferedWrittenMessages.popFirst() {
95+
while let (bufferedPart, isMarked) = self.bufferedWrittenMessages.popFirstCheckMarked() {
10996
context.write(bufferedPart.data, promise: bufferedPart.promise)
97+
if isMarked {
98+
context.flush()
99+
}
110100
}
101+
111102
}
112103

113104
context.leavePipeline(removalToken: removalToken)
@@ -122,9 +113,12 @@ public final class NIOHTTP1ProxyConnectHandler: ChannelDuplexHandler, RemovableC
122113
public func handlerRemoved(context: ChannelHandlerContext) {
123114
switch self.state {
124115
case .failed, .completed:
125-
// we don't expect there to be any buffered messages in these states
126-
assert(self.bufferedWrittenMessages.isEmpty)
116+
guard self.bufferedWrittenMessages.isEmpty else {
117+
self.failWithError(Error.droppedWrites(), context: context)
118+
return
119+
}
127120
break
121+
128122
case .initialized, .connectSent, .headReceived:
129123
self.failWithError(Error.noResult(), context: context)
130124
}
@@ -276,6 +270,7 @@ public final class NIOHTTP1ProxyConnectHandler: ChannelDuplexHandler, RemovableC
276270
case httpProxyHandshakeTimeout
277271
case noResult
278272
case channelUnexpectedlyInactive
273+
case droppedWrites
279274
}
280275

281276
final class Storage: Sendable {
@@ -331,6 +326,10 @@ public final class NIOHTTP1ProxyConnectHandler: ChannelDuplexHandler, RemovableC
331326
Error(error: .channelUnexpectedlyInactive, file: file, line: line)
332327
}
333328

329+
public static func droppedWrites(file: String = #file, line: UInt = #line) -> Error {
330+
Error(error: .droppedWrites, file: file, line: line)
331+
}
332+
334333
fileprivate var errorCode: Int {
335334
switch self.store.details {
336335
case .proxyAuthenticationRequired:
@@ -347,6 +346,8 @@ public final class NIOHTTP1ProxyConnectHandler: ChannelDuplexHandler, RemovableC
347346
return 5
348347
case .channelUnexpectedlyInactive:
349348
return 6
349+
case .droppedWrites:
350+
return 7
350351
}
351352
}
352353
}
@@ -388,6 +389,8 @@ extension NIOHTTP1ProxyConnectHandler.Error.Details: CustomStringConvertible {
388389
return "No Result"
389390
case .channelUnexpectedlyInactive:
390391
return "Channel Unexpectedly Inactive"
392+
case .droppedWrites:
393+
return "Handler Was Removed with Writes Left in the Buffer"
391394
}
392395
}
393396
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// This source file is part of the SwiftNIO open source project
4+
//
5+
// Copyright (c) YEARS Apple Inc. and the SwiftNIO project authors
6+
// Licensed under Apache License v2.0
7+
//
8+
// See LICENSE.txt for license information
9+
// See CONTRIBUTORS.txt for the list of SwiftNIO project authors
10+
//
11+
// SPDX-License-Identifier: Apache-2.0
12+
//
13+
//===----------------------------------------------------------------------===//
14+
15+
import NIOCore
16+
17+
extension MarkedCircularBuffer {
18+
@inlinable
19+
internal mutating func popFirstCheckMarked() -> (Element, Bool)? {
20+
let marked = self.markedElementIndex == self.startIndex
21+
return self.popFirst().map { ($0, marked) }
22+
}
23+
}

0 commit comments

Comments
 (0)