ssh: flush any queued output at teardown - #1219
Open
ejohnstown wants to merge 1 commit into
Open
Conversation
wolfSSH_shutdown() flushes whatever a short send left in the output buffer, not just a queued disconnect. A rejected auth's USERAUTH_FAILURE has no channel, and DoChannelClose() retires the channel as soon as the close is bundled, so neither had anything left to carry the retry. - The gate still refuses a flush once the peer has disconnected, unless our own disconnect is the thing queued. - ssh.h documents the widened flush and the WS_WANT_WRITE a short one leaves wolfSSH_shutdown() returning. - Tests cover the rejected auth with no channel and the close whose channel DoChannelClose() already retired. - A flush that short-sends on a live channel leaves the teardown queued behind it, and the retry adds no second EOF or close.
Contributor
There was a problem hiding this comment.
Pull request overview
Updates wolfSSH_shutdown() teardown behavior so it will flush any already-queued outbound bytes left behind by a short send (not only a queued local disconnect), while still preventing post-disconnect traffic unless the only queued output is the local disconnect.
Changes:
- Generalize the shutdown-time “flush queued disconnect” gate into a “flush queued output” gate, with special-casing to suppress non-disconnect output after the peer has disconnected.
- Extend regression coverage for queued output flush scenarios: rejected auth with no channel, channel-close queued after channel retirement, and a live-channel short-send during shutdown.
- Document the broader shutdown flush behavior in the public header.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/ssh.c |
Generalizes queued-output flushing at teardown and in the post-disconnect send path. |
wolfssh/ssh.h |
Expands wolfSSH_shutdown() API comment to describe the broader flush semantics and retry behavior. |
tests/regress.c |
Adds regression tests covering newly supported queued-output flush scenarios and short-send retry behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
wolfSSH_shutdown() flushes whatever a short send left in the output buffer,
not just a queued disconnect. A rejected auth's USERAUTH_FAILURE has no
channel, and a channel close is retired the moment it is bundled, so neither
had anything left to carry the retry.
own disconnect is the thing queued.
already retired, and a flush that short-sends on a live channel.