Skip to content

Commit be3b2d5

Browse files
yosuke-wolfsslyosuke-wolfssl
committed
ssh: flush the worker's queued output and report what it is waiting on
- wolfSSH_worker() calls wolfSSH_SendPacket() whenever ssh->outputBuffer holds bytes, in place of doing so only for WS_SUCCESS, WS_WANT_READ or WS_CHAN_RXD. It takes ssh->error into rxErr before the send and restores it unless the send status wins, replaces DoReceive()'s WS_FATAL_ERROR with ssh->error when that holds a specific code, and writes *channelId before the send. Drops the second DoReceive(), its WS_WINDOW_FULL case, and the WOLFSSH_TEST_BLOCK fork. - GetInputData() returns its condition in place of assigning ssh->error and returning WS_FATAL_ERROR; DoReceive() records it at both call sites. - SendChannelData() and SendChannelExtendedData() capture the send status into sendRet and record it inside the existing ret == WS_SUCCESS block, which now also gates plainSz. Both set plainSz as well when the flush of already-queued output returns WS_WANT_WRITE. - _ChannelRead(), _ChannelReadExt() and SendPendingChannelWindowAdjust() drop their savedError capture and restore, keeping the assignment on failure. - wolfSSH_shutdown() reads ret in its teardown-send guards. - echoserver, wolfsshd, sftpclient, client and scpclient gate on the worker's and shutdown's return values, and sftpclient and the wolfssh app drop 12 now-dead WS_FATAL_ERROR translations. - echoserver reads wolfSSH_get_error() once after the shutdown drain for its end-of-connection report; wolfsshd's drain loops on WS_SUCCESS, the wants, WS_CHAN_RXD, WS_EXTDATA and WS_REKEYING. - tests cover the worker's flush and reported status and assert the specific codes it now returns; TestShutdownKeepsFlushWantWrite() checks wolfSSH_shutdown()'s return alone.
1 parent 9731cfe commit be3b2d5

10 files changed

Lines changed: 422 additions & 181 deletions

File tree

‎apps/wolfssh/wolfssh.c‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -325,10 +325,6 @@ static int FlushQueuedSend(WOLFSSH* ssh, wolfSSL_Mutex* lock)
325325
wc_LockMutex(lock);
326326
}
327327
ret = wolfSSH_worker(ssh, NULL);
328-
if (ret == WS_FATAL_ERROR) {
329-
/* the session holds the detail behind a fatal error */
330-
ret = wolfSSH_get_error(ssh);
331-
}
332328
if (lock != NULL) {
333329
wc_UnLockMutex(lock);
334330
}
@@ -1353,9 +1349,6 @@ static THREAD_RETURN WOLFSSH_THREAD wolfSSH_Client(void* args)
13531349

13541350
if (ret == WS_SUCCESS) {
13551351
ret = wolfSSH_worker(ssh, NULL);
1356-
if (ret == WS_FATAL_ERROR) {
1357-
ret = wolfSSH_get_error(ssh);
1358-
}
13591352
if (ret == WS_WANT_WRITE) {
13601353
/* The close messages are already out, whatever the drain
13611354
* still wants to send is a reply to the peer. */

‎apps/wolfsshd/wolfsshd.c‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2623,15 +2623,12 @@ static void* HandleConnection(void* arg)
26232623
ret = 0;
26242624
}
26252625

2626-
error = wolfSSH_get_error(ssh);
2627-
if (error != WS_SOCKET_ERROR_E &&
2628-
(error == WS_WANT_READ || error == WS_WANT_WRITE)) {
2626+
if (ret == WS_WANT_READ || ret == WS_WANT_WRITE) {
26292627
int maxAttempt = 10; /* make 10 attempts max before giving up */
26302628
int attempt;
26312629

26322630
for (attempt = 0; attempt < maxAttempt; attempt++) {
26332631
ret = wolfSSH_worker(ssh, NULL);
2634-
error = wolfSSH_get_error(ssh);
26352632

26362633
/* peer successfully closed down gracefully */
26372634
if (ret == WS_CHANNEL_CLOSED) {
@@ -2645,9 +2642,11 @@ static void* HandleConnection(void* arg)
26452642
break;
26462643
}
26472644

2648-
if (ret == WS_FATAL_ERROR &&
2649-
(error != WS_WANT_READ &&
2650-
error != WS_WANT_WRITE)) {
2645+
/* Keep draining while the socket blocks or the peer is still
2646+
* talking. Anything else is a failure worth giving up on. */
2647+
if (ret != WS_SUCCESS && ret != WS_WANT_READ &&
2648+
ret != WS_WANT_WRITE && ret != WS_CHAN_RXD &&
2649+
ret != WS_EXTDATA && ret != WS_REKEYING) {
26512650
break;
26522651
}
26532652
#ifdef _WIN32

‎examples/client/client.c‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1209,7 +1209,8 @@ THREAD_RETURN WOLFSSH_THREAD client_test(void* args)
12091209
}
12101210
ret = wolfSSH_worker(ssh, NULL);
12111211
if (ret != WS_SUCCESS && ret != WS_SOCKET_ERROR_E &&
1212-
ret != WS_CHANNEL_CLOSED) {
1212+
ret != WS_CHANNEL_CLOSED &&
1213+
ret != WS_WANT_READ && ret != WS_WANT_WRITE) {
12131214
ClientFreeBuffers(pubKeyName, privKeyName, NULL);
12141215
wolfSSH_free(ssh);
12151216
wolfSSH_CTX_free(ctx);
@@ -1226,7 +1227,8 @@ THREAD_RETURN WOLFSSH_THREAD client_test(void* args)
12261227
wolfSSH_free(ssh);
12271228
wolfSSH_CTX_free(ctx);
12281229
if (ret != WS_SUCCESS && ret != WS_SOCKET_ERROR_E &&
1229-
ret != WS_CHANNEL_CLOSED) {
1230+
ret != WS_CHANNEL_CLOSED &&
1231+
ret != WS_WANT_READ && ret != WS_WANT_WRITE) {
12301232
err_sys("Closing client stream failed");
12311233
}
12321234

‎examples/echoserver/echoserver.c‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1644,9 +1644,7 @@ static THREAD_RETURN WOLFSSH_THREAD server_worker(void* vArgs)
16441644
ret = 0;
16451645
}
16461646

1647-
error = wolfSSH_get_error(threadCtx->ssh);
1648-
if (error != WS_SOCKET_ERROR_E &&
1649-
(error == WS_WANT_READ || error == WS_WANT_WRITE)) {
1647+
if (ret == WS_WANT_READ || ret == WS_WANT_WRITE) {
16501648
int maxAttempt = 10; /* make 10 attempts max before giving up */
16511649
int attempt;
16521650

@@ -1681,6 +1679,10 @@ static THREAD_RETURN WOLFSSH_THREAD server_worker(void* vArgs)
16811679
}
16821680
}
16831681

1682+
/* The report below names how the connection ended, and the shutdown
1683+
* drain refreshes error only on the paths that enter it. */
1684+
error = wolfSSH_get_error(threadCtx->ssh);
1685+
16841686
if (threadCtx->fd != -1) {
16851687
WCLOSESOCKET(threadCtx->fd);
16861688
threadCtx->fd = -1;

‎examples/scpclient/scpclient.c‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,8 @@ THREAD_RETURN WOLFSSH_THREAD scp_client(void* args)
326326
}
327327
else {
328328
ret = wolfSSH_worker(ssh, NULL);
329-
if (ret != WS_SUCCESS && ret != WS_CHANNEL_CLOSED) {
329+
if (ret != WS_SUCCESS && ret != WS_CHANNEL_CLOSED &&
330+
ret != WS_WANT_READ && ret != WS_WANT_WRITE) {
330331
WLOG(WS_LOG_DEBUG,
331332
"Failed to listen for close messages from the peer.");
332333
}

‎examples/sftpclient/sftpclient.c‎

Lines changed: 1 addition & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -661,9 +661,6 @@ static int doCmds(func_args* args)
661661
do {
662662
while (ret == WS_REKEYING || ssh->error == WS_REKEYING) {
663663
ret = wolfSSH_worker(ssh, NULL);
664-
if (ret != WS_SUCCESS && ret == WS_FATAL_ERROR) {
665-
ret = wolfSSH_get_error(ssh);
666-
}
667664
}
668665

669666
ret = wolfSSH_SFTP_Get(ssh, pt, to, resume, &myStatusCb);
@@ -772,9 +769,6 @@ static int doCmds(func_args* args)
772769
do {
773770
while (ret == WS_REKEYING || ssh->error == WS_REKEYING) {
774771
ret = wolfSSH_worker(ssh, NULL);
775-
if (ret != WS_SUCCESS && ret == WS_FATAL_ERROR) {
776-
ret = wolfSSH_get_error(ssh);
777-
}
778772
}
779773

780774
ret = wolfSSH_SFTP_Put(ssh, pt, to, resume, &myStatusCb);
@@ -866,9 +860,6 @@ static int doCmds(func_args* args)
866860
do {
867861
while (ret == WS_REKEYING || ssh->error == WS_REKEYING) {
868862
ret = wolfSSH_worker(ssh, NULL);
869-
if (ret != WS_SUCCESS && ret == WS_FATAL_ERROR) {
870-
ret = wolfSSH_get_error(ssh);
871-
}
872863
}
873864

874865
ret = wolfSSH_SFTP_STAT(ssh, pt, &atrb);
@@ -920,9 +911,6 @@ static int doCmds(func_args* args)
920911
do {
921912
while (ret == WS_REKEYING || ssh->error == WS_REKEYING) {
922913
ret = wolfSSH_worker(ssh, NULL);
923-
if (ret != WS_SUCCESS && ret == WS_FATAL_ERROR) {
924-
ret = wolfSSH_get_error(ssh);
925-
}
926914
}
927915

928916
ret = wolfSSH_SFTP_CHMOD(ssh, path, mode);
@@ -987,9 +975,6 @@ static int doCmds(func_args* args)
987975
do {
988976
while (ret == WS_REKEYING || ssh->error == WS_REKEYING) {
989977
ret = wolfSSH_worker(ssh, NULL);
990-
if (ret != WS_SUCCESS && ret == WS_FATAL_ERROR) {
991-
ret = wolfSSH_get_error(ssh);
992-
}
993978
}
994979
ret = wolfSSH_SFTP_Open(ssh, path,
995980
WOLFSSH_FXF_WRITE | WOLFSSH_FXF_CREAT |
@@ -1003,9 +988,6 @@ static int doCmds(func_args* args)
1003988
do {
1004989
while (ret == WS_REKEYING || ssh->error == WS_REKEYING) {
1005990
ret = wolfSSH_worker(ssh, NULL);
1006-
if (ret != WS_SUCCESS && ret == WS_FATAL_ERROR) {
1007-
ret = wolfSSH_get_error(ssh);
1008-
}
1009991
}
1010992
ret = wolfSSH_SFTP_Close(ssh, handle, handleSz);
1011993
err = wolfSSH_get_error(ssh);
@@ -1060,9 +1042,6 @@ static int doCmds(func_args* args)
10601042
do {
10611043
while (ret == WS_REKEYING || ssh->error == WS_REKEYING) {
10621044
ret = wolfSSH_worker(ssh, NULL);
1063-
if (ret != WS_SUCCESS && ret == WS_FATAL_ERROR) {
1064-
ret = wolfSSH_get_error(ssh);
1065-
}
10661045
}
10671046

10681047
ret = wolfSSH_SFTP_RMDIR(ssh, pt);
@@ -1117,9 +1096,6 @@ static int doCmds(func_args* args)
11171096
do {
11181097
while (ret == WS_REKEYING || ssh->error == WS_REKEYING) {
11191098
ret = wolfSSH_worker(ssh, NULL);
1120-
if (ret != WS_SUCCESS && ret == WS_FATAL_ERROR) {
1121-
ret = wolfSSH_get_error(ssh);
1122-
}
11231099
}
11241100

11251101
ret = wolfSSH_SFTP_Remove(ssh, pt);
@@ -1210,9 +1186,6 @@ static int doCmds(func_args* args)
12101186
do {
12111187
while (ret == WS_REKEYING || ssh->error == WS_REKEYING) {
12121188
ret = wolfSSH_worker(ssh, NULL);
1213-
if (ret != WS_SUCCESS && ret == WS_FATAL_ERROR) {
1214-
ret = wolfSSH_get_error(ssh);
1215-
}
12161189
}
12171190

12181191
ret = wolfSSH_SFTP_Rename(ssh, pt, to);
@@ -1348,9 +1321,6 @@ static int doCmds(func_args* args)
13481321
do {
13491322
while (ret == WS_REKEYING || ssh->error == WS_REKEYING) {
13501323
ret = wolfSSH_worker(ssh, NULL);
1351-
if (ret != WS_SUCCESS && ret == WS_FATAL_ERROR) {
1352-
ret = wolfSSH_get_error(ssh);
1353-
}
13541324
}
13551325

13561326
current = wolfSSH_SFTP_LS(ssh, workingDir);
@@ -1821,9 +1791,7 @@ THREAD_RETURN WOLFSSH_THREAD sftpclient_test(void* args)
18211791
ret = 0;
18221792
}
18231793

1824-
err = wolfSSH_get_error(ssh);
1825-
if (err != WS_SOCKET_ERROR_E &&
1826-
(err == WS_WANT_READ || err == WS_WANT_WRITE)) {
1794+
if (ret == WS_WANT_READ || ret == WS_WANT_WRITE) {
18271795
int maxAttempt = 10; /* make 10 attempts max before giving up */
18281796
int attempt;
18291797

0 commit comments

Comments
 (0)