From 33e9da4567e42c6e2724e6ede082bd47f9a21319 Mon Sep 17 00:00:00 2001 From: Raphael Hiesgen Date: Mon, 6 Jul 2026 14:56:07 +0100 Subject: [PATCH 1/3] Add test to reproduce misbehaving stream shutdown --- Tests/SwiftNetworkTests/QUICTestHarness.swift | 80 +++++++++++++++++++ .../SwiftNetworkQUICHarnessTests.swift | 4 + 2 files changed, 84 insertions(+) diff --git a/Tests/SwiftNetworkTests/QUICTestHarness.swift b/Tests/SwiftNetworkTests/QUICTestHarness.swift index 42aaba7..1c4230c 100644 --- a/Tests/SwiftNetworkTests/QUICTestHarness.swift +++ b/Tests/SwiftNetworkTests/QUICTestHarness.swift @@ -1436,6 +1436,86 @@ final class QUICTestHarness { stop() } + /// A clean close of a send side that never wrote any data must be encoded as a + /// zero-length `STREAM` frame with the FIN bit set (RFC 9000 §3.1 / §19.8), not + /// a `RESET_STREAM`. + /// + /// Reproduces the asymmetric scenario: the client opens a bidi stream and sends + /// `"ping"` + FIN — its send side went `.ready → .send`. The server reads + /// `"ping"` then closes the stream having never written, so its send side is still + /// `.ready` and the close is clean (`outboundApplicationError == nil`). + /// The client's receive side must reach a clean end-of-stream. + func runEmptyFinCleanCloseSendsFinNotReset(timeout: TimeInterval = 4.0) { + do { + try quicHandshake() + } catch { + XCTFail("Handshake failed: \(error)") + return + } + + guard let clientStream = createNewStream(identifier: "C1") else { + XCTFail("Failed to create client stream") + return + } + + let serverFlowExpectation = XCTestExpectation(description: "Server sees new flow") + let serverClosedExpectation = XCTestExpectation( + description: "Server reads ping and closes the stream without writing" + ) + // Only used to bound the wait for a inbound abort. A clean close of + // a send side that never wrote must not reset the peer, so this must never + // actually be fulfilled. + let clientAbortExpectation = XCTestExpectation(description: "Client inbound abort (must not happen)") + + var serverStream: StreamUpperHarness? + var clientResetError: String? + + // The client's receive side must reach a clean end-of-stream. + clientStream.waitForInboundAborted { error in + clientResetError = error?.description ?? "no error" + clientAbortExpectation.fulfill() + } + + context.async { + self.state?.serverHarness.waitForNewFlow { + guard let stream = self.state?.serverHarness.upperHarnesses.last else { + XCTFail("Server flow missing") + serverFlowExpectation.fulfill() + return + } + serverStream = stream + serverFlowExpectation.fulfill() + + // Read the client's "ping" (present as of the new-flow event), then + // close the stream having never written. The send side is still + // `.ready`, so this clean close (no application error) must emit a + // zero-length STREAM+FIN, not RESET_STREAM. + while stream.read() != nil {} + stream.stop() + serverClosedExpectation.fulfill() + } + } + + // Open the stream with a real payload + FIN so the server learns the flow. + context.async { + let wrote = clientStream.write(Array("ping".utf8), sendFIN: true) + XCTAssertTrue(wrote, "Client failed to write ping") + } + + wait(for: [serverFlowExpectation], timeout: timeout) + wait(for: [serverClosedExpectation], timeout: timeout) + // Give other errors some time to arrive. + _ = XCTWaiter.wait(for: [clientAbortExpectation], timeout: 1.0) + XCTAssertNil( + clientResetError, + "Client received RESET_STREAM(\(clientResetError ?? "")) instead of a clean FIN — a clean close " + + "of a send side that never wrote must be a zero-length STREAM+FIN (RFC 9000 §3.1 / §19.8)." + ) + + _ = serverStream // silence unused-warning; harness keeps strong ref + stop() + } + func runQUICServerTestForPendingBidirectional( identifier: String = #function, dataBlock: [UInt8], diff --git a/Tests/SwiftNetworkTests/SwiftNetworkQUICHarnessTests.swift b/Tests/SwiftNetworkTests/SwiftNetworkQUICHarnessTests.swift index 90a2d56..e2f6e94 100644 --- a/Tests/SwiftNetworkTests/SwiftNetworkQUICHarnessTests.swift +++ b/Tests/SwiftNetworkTests/SwiftNetworkQUICHarnessTests.swift @@ -370,6 +370,10 @@ final class SwiftNetworkQUICHarnessTests: NetTestCase { QUICTestHarness().runQUICResetStreamFirstFrameOnNewStream() } + func testQUICEmptyFinCleanCloseSendsFinNotReset() { + QUICTestHarness().runEmptyFinCleanCloseSendsFinNotReset() + } + func testQUICResetStreamDoesNotAffectOppositeDirection() { QUICTestHarness().runQUICTest( streamCount: 1, From d9be0541194ee29b4a26127b019f4d58658688a8 Mon Sep 17 00:00:00 2001 From: Raphael Hiesgen Date: Mon, 6 Jul 2026 16:01:43 +0100 Subject: [PATCH 2/3] Move stream to send state for clean shutdown Streams that never transmitted data when closing cleanly might still be in the ready state, which will lead to a RESET_STREAM instead of a clean shutdown. --- .../SwiftNetwork/QUIC/QUICConnection.swift | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/Sources/SwiftNetwork/QUIC/QUICConnection.swift b/Sources/SwiftNetwork/QUIC/QUICConnection.swift index 8ff817c..3376def 100644 --- a/Sources/SwiftNetwork/QUIC/QUICConnection.swift +++ b/Sources/SwiftNetwork/QUIC/QUICConnection.swift @@ -3827,11 +3827,21 @@ public final class QUICConnection: ManyToManyApplicationStreamProtocol, } if !state.isTerminal { - // Send a FIN if we are closing a stream without an error. - // Otherwise, send RESET_STREAM. - // Technically, 0 can be valid application error code, but - // it isn't for HTTP/3, so we can assume it's not set. - if stream.sendState == .send && stream.outboundApplicationError == nil { + // A clean close (no application error) is a FIN, regardless of whether + // any data was sent. A send side still in `.ready` (nothing ever sent) + // advances to `.send` so the servicing path emits a zero-length STREAM + // frame with the FIN bit set (RFC 9000 §3.1 / §19.8). Only send + // RESET_STREAM when an application error is set (e.g. a received + // STOP_SENDING) or a partially buffered stream is aborted. + if stream.outboundApplicationError == nil + && (stream.sendState == .send || stream.sendState == .ready) + { + if stream.sendState == .ready { + stream.sendState.change(logIDString: stream.logPrefix, to: .send) + // Keep the write side open until the zero-length FIN has been + // serviced (and ACKed); closing now would drop the queued FIN. + closeWrite = false + } markStreamFinished(stream: stream) } else if stream.sendState == .ready || (stream.sendState == .send && !stream.sendBuffer.hasLast From ba894a945a926768d1971b5f367073bd0913a3ef Mon Sep 17 00:00:00 2001 From: Raphael Hiesgen Date: Tue, 7 Jul 2026 18:55:22 +0100 Subject: [PATCH 3/3] Verify stream state after close --- Tests/SwiftNetworkTests/QUICTestHarness.swift | 38 +++++++++++++++++-- 1 file changed, 34 insertions(+), 4 deletions(-) diff --git a/Tests/SwiftNetworkTests/QUICTestHarness.swift b/Tests/SwiftNetworkTests/QUICTestHarness.swift index 1c4230c..6087dac 100644 --- a/Tests/SwiftNetworkTests/QUICTestHarness.swift +++ b/Tests/SwiftNetworkTests/QUICTestHarness.swift @@ -1462,10 +1462,10 @@ final class QUICTestHarness { let serverClosedExpectation = XCTestExpectation( description: "Server reads ping and closes the stream without writing" ) - // Only used to bound the wait for a inbound abort. A clean close of - // a send side that never wrote must not reset the peer, so this must never - // actually be fulfilled. + // Expecting a clean close, so this should not be fulfilled. let clientAbortExpectation = XCTestExpectation(description: "Client inbound abort (must not happen)") + // The client must observe a clean end-of-stream. + let clientClosedExpectation = XCTestExpectation(description: "Client observes clean close") var serverStream: StreamUpperHarness? var clientResetError: String? @@ -1475,6 +1475,18 @@ final class QUICTestHarness { clientResetError = error?.description ?? "no error" clientAbortExpectation.fulfill() } + clientStream.waitForDisconnected { + clientClosedExpectation.fulfill() + } + + // Client drains its receive side so the server's FIN is consumed and the + // stream can reach a clean close. + var clientReadHandler: ((Bool) -> Void)? = nil + defer { clientReadHandler = nil } + clientReadHandler = { _ in + while clientStream.read() != nil {} + clientStream.waitForInboundDataAvailable { clientReadHandler?($0) } + } context.async { self.state?.serverHarness.waitForNewFlow { @@ -1500,17 +1512,35 @@ final class QUICTestHarness { context.async { let wrote = clientStream.write(Array("ping".utf8), sendFIN: true) XCTAssertTrue(wrote, "Client failed to write ping") + clientReadHandler?(true) } wait(for: [serverFlowExpectation], timeout: timeout) wait(for: [serverClosedExpectation], timeout: timeout) // Give other errors some time to arrive. - _ = XCTWaiter.wait(for: [clientAbortExpectation], timeout: 1.0) + _ = XCTWaiter.wait(for: [clientAbortExpectation], timeout: 0.5) XCTAssertNil( clientResetError, "Client received RESET_STREAM(\(clientResetError ?? "")) instead of a clean FIN — a clean close " + "of a send side that never wrote must be a zero-length STREAM+FIN (RFC 9000 §3.1 / §19.8)." ) + // And confirm that the client saw the FIN. + wait(for: [clientClosedExpectation], timeout: timeout) + + #if canImport(SwiftNetwork) + // The server's send side must have finished via FIN, i.e. the state should be + // in .dataSent, or .dataReceived. + if let serverConnection = state?.serverInstance, + let serverFlow = serverConnection.multiplexedFlows.values.first + { + XCTAssertTrue( + serverFlow.sendState == .dataSent || serverFlow.sendState == .dataReceived, + "Server send side must reach a clean FIN terminal state, got \(serverFlow.sendState)" + ) + } else { + XCTFail("Server stream flow missing; cannot verify send state") + } + #endif _ = serverStream // silence unused-warning; harness keeps strong ref stop()