From 7733e7e8ffb68eb83fedf35550c29882d8fda404 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=A9tan=20Zanella?= Date: Thu, 20 Apr 2023 10:06:01 +0200 Subject: [PATCH] Avoid sending window update messages if locally closed (#143) (#144) Motivation: Currently the library crashes if a window update is required and the connection is closed locally. Modifications: Check the connection state before sending a window update message. --- .../Child Channels/SSHChildChannel.swift | 2 +- .../ChildChannelMultiplexerTests.swift | 48 +++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/Sources/NIOSSH/Child Channels/SSHChildChannel.swift b/Sources/NIOSSH/Child Channels/SSHChildChannel.swift index e2188e8..29077b3 100644 --- a/Sources/NIOSSH/Child Channels/SSHChildChannel.swift +++ b/Sources/NIOSSH/Child Channels/SSHChildChannel.swift @@ -647,7 +647,7 @@ private extension SSHChildChannel { switch data { case .data(let data): // We only futz with the window manager if the channel is not already closed. - if !self.didClose, let increment = self.windowManager.unbufferBytes(data.data.readableBytes) { + if !self.didClose, !self.state.sentClose, let increment = self.windowManager.unbufferBytes(data.data.readableBytes) { let update = SSHMessage.ChannelWindowAdjustMessage(recipientChannel: self.state.remoteChannelIdentifier!, bytesToAdd: UInt32(increment)) self.processOutboundMessage(.channelWindowAdjust(update), promise: nil) } diff --git a/Tests/NIOSSHTests/ChildChannelMultiplexerTests.swift b/Tests/NIOSSHTests/ChildChannelMultiplexerTests.swift index 284bba9..57a8ebb 100644 --- a/Tests/NIOSSHTests/ChildChannelMultiplexerTests.swift +++ b/Tests/NIOSSHTests/ChildChannelMultiplexerTests.swift @@ -1274,6 +1274,54 @@ final class ChildChannelMultiplexerTests: XCTestCase { self.assertChannelClose(harness.flushedMessages.last, recipientChannel: 1) } + func testWeDontResizeTheWindowAfterLocalClosing() throws { + let harness = self.harnessForbiddingInboundChannels() + defer { + harness.finish() + } + + var childChannel: Channel? + harness.multiplexer.createChildChannel(channelType: .session) { channel, _ in + childChannel = channel + return channel.setOption(ChannelOptions.autoRead, value: false) + } + + guard let childChannel = childChannel else { + XCTFail("Did not create child channel") + return + } + + // Activate channel. + let channelID = self.assertChannelOpen(harness.flushedMessages.first) + XCTAssertNoThrow(try harness.multiplexer.receiveMessage(self.openConfirmation(originalChannelID: channelID!, peerChannelID: 1))) + XCTAssertEqual(harness.flushedMessages.count, 1) + + // The default window size is 1<<24 bytes. Sadly, we need a buffer that size. + let buffer = ByteBuffer.bigBuffer + + // We close locally the channel. + childChannel.close(promise: nil) + XCTAssertEqual(harness.flushedMessages.count, 2) + + // But, for some reason, we are still receiving data that requires a window adjustment. + XCTAssertNoThrow(try harness.multiplexer.receiveMessage( + self.data( + peerChannelID: channelID!, + data: buffer.getSlice(at: buffer.readerIndex, length: 1)! + ) + )) + XCTAssertNoThrow(try harness.multiplexer.receiveMessage( + self.data( + peerChannelID: channelID!, + data: buffer.getSlice(at: buffer.readerIndex, length: 1 << 23)! + ) + )) + + // This should not trigger outbound messages. + childChannel.read() + XCTAssertEqual(harness.flushedMessages.count, 2) + } + func testRespectingMaxMessageSize() throws { let harness = self.harnessForbiddingInboundChannels() defer {