apps/wolfssh, examples/client: resend stdin a send did not take - #1280
Open
yosuke-wolfssl wants to merge 1 commit into
Open
yosuke-wolfssl wants to merge 1 commit into
yosuke-wolfssl wants to merge 1 commit into
Conversation
- readInput() in apps/wolfssh runs FlushQueuedSend() when wolfSSH_stream_send() returns WS_WANT_WRITE without taking the buffer, then sends the same buffer again; a failed flush still ends in "Couldn't send data". - readInput() in examples/client retries WS_WANT_WRITE as it retries WS_REKEYING, for up to SEND_RETRY_TIMEOUT (10) seconds per buffer. - The comment on the queued check in apps/wolfssh describes the positive-return case.
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1280
Scan targets checked: wolfssh-src, wolfssh-bugs
Coverage: 2 of 2 in-scope changed file(s) opened by the reviewer
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
Review tier: Lite
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.
Problem
wolfSSH_stream_send()can returnWS_WANT_WRITEwithout taking any of the caller's buffer: when earlier output is still queued and the socket won't take it,SendChannelData()tries to flush that queue first and returns before framing the new data. Both client stdin readers treated this as fatal. They printed "Couldn't send data" and stopped reading stdin, while the session stayed up.examples/client -Nhits this on master: nothing flushes a queued send there, so the next send's flush-first check returnsWS_WANT_WRITE.apps/wolfsshrarely hits it today, because itsqueuedcheck flushes after each send. A follow-up library fix forSendChannelData(), which currently reports the caller's bytes as taken when it queued nothing, will make it returnWS_WANT_WRITEin those cases. This PR has to land first.Fix (
apps/wolfssh/wolfssh.c,examples/client/client.c)Both readers now keep the buffer and send it again:
WS_WANT_WRITEwith nothing takenapps/wolfsshFlushQueuedSend()drives the worker until the queue drains, then resendsFLUSH_QUEUE_TIMEOUTpassesexamples/clientWS_REKEYING; the resend flushes the queue firstSEND_RETRY_TIMEOUT) of waiting on the socket; a rekey restarts the waitBoth still print "Couldn't send data" when they give up. The
apps/wolfsshcomment on thequeuedcheck now describes the case it covers: a positive return with data still queued.Verification
make check: 13/13 pass. gcc-13-Werror: clean across 6 configs.WOLFSSH_TEST_BLOCKbuild with forced write blocks after connect, 40000 bytes on stdin, 3/3 runs each. "Library fix" is the follow-up above, applied locally:apps/wolfsshexamples/client -NNot in this PR (already there before this change, will be fixed separately): Windows
client_test()frees the session without stoppingreadInput(); both readers treat a short positive return as fully sent.