Skip to content

Commit

Permalink
Avoid sending window update messages if locally closed (#143) (#144)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
gaetanzanella authored Apr 20, 2023
1 parent c56abab commit 7733e7e
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 1 deletion.
2 changes: 1 addition & 1 deletion Sources/NIOSSH/Child Channels/SSHChildChannel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
48 changes: 48 additions & 0 deletions Tests/NIOSSHTests/ChildChannelMultiplexerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 7733e7e

Please sign in to comment.