ssh, internal: flush the worker's queued output on every call

- wolfSSH_worker() calls wolfSSH_SendPacket() whenever
  ssh->outputBuffer holds bytes and the session is not
  disconnected, in place of doing so only for WS_SUCCESS,
  WS_WANT_READ or WS_CHAN_RXD. A failed flush reaches the return
  only when the receive reported nothing; ssh->error keeps the
  receive's code when the receive reported anything but
  WS_WANT_READ and the flush left bytes queued to retry. Drops
  the second DoReceive(), its WS_WINDOW_FULL case, and the
  WOLFSSH_TEST_BLOCK fork.
- wolfSSH_SendPacket() records its code in ssh->error on every
  transport failure path, and the comments on _ChannelRead(),
  _ChannelReadExt() and test_ChannelReadExtHardFailureReported()
  no longer describe the former contract.
- The echoserver shell loop and the Windows wolfsshd shell loop
  treat a WS_WANT_WRITE from wolfSSH_worker() as non-fatal.
- tests cover the flush on an idle receive, the owed flush across
  calls, what ssh->error holds after a receive failure, a hard
  send failure, channel data alongside a failed send, a discarded
  buffer and an out-of-bounds send, and that queued output stays
  unsent on the pass that receives a disconnect.

Author: yosuke-wolfssl

apps/wolfsshd/wolfsshd.c

@@ -1361,7 +1361,7 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
                          * peer. Both want fixing where they can be tested. */
                         continue;
                     }
-                    else if (rc != WS_WANT_READ) {
+                    else if (rc != WS_WANT_READ && rc != WS_WANT_WRITE) {
                         break;
                     }
                 }

examples/echoserver/echoserver.c

@@ -1205,7 +1205,7 @@ static int ssh_worker(thread_ctx_t* threadCtx)
                          * above, which has already run this pass. */
                         continue;
                     }
-                    else if (rc != WS_WANT_READ) {
+                    else if (rc != WS_WANT_READ && rc != WS_WANT_WRITE) {
                         #ifdef SHELL_DEBUG
                             printf("Break:read sshFd returns %d: errno =%x\n",
                                     cnt_r, errno);

src/internal.c

@@ -4290,13 +4290,15 @@ static int GetInputLine(WOLFSSH* ssh, byte** pEol)
 }
 
 
-/* returns WS_SUCCESS on success */
+/* returns WS_SUCCESS on success. Transport failures record their code in
+ * ssh->error */
 int wolfSSH_SendPacket(WOLFSSH* ssh)
 {
     WLOG(WS_LOG_DEBUG, "Entering wolfSSH_SendPacket()");
 
     if (ssh->ctx->ioSendCb == NULL) {
         WLOG(WS_LOG_DEBUG, "Your IO Send callback is null, please set");
+        ssh->error = WS_SOCKET_ERROR_E;
         return WS_SOCKET_ERROR_E;
     }
 
@@ -4307,6 +4309,7 @@ int wolfSSH_SendPacket(WOLFSSH* ssh)
         if (ssh->outputBuffer.length > ssh->outputBuffer.bufferSz ||
                 ssh->outputBuffer.length < ssh->outputBuffer.idx) {
             WLOG(WS_LOG_ERROR, "Bad buffer state");
+            ssh->error = WS_BUFFER_E;
             return WS_BUFFER_E;
         }
 
@@ -4332,11 +4335,13 @@ int wolfSSH_SendPacket(WOLFSSH* ssh)
                 case WS_CBIO_ERR_GENERAL:
                     ShrinkBuffer(&ssh->outputBuffer, 1);
             }
+            ssh->error = WS_SOCKET_ERROR_E;
             return WS_SOCKET_ERROR_E;
         }
 
         if ((word32)sent > ssh->outputBuffer.length) {
             WLOG(WS_LOG_DEBUG, "wolfSSH_SendPacket() out of bounds read");
+            ssh->error = WS_SEND_OOB_READ_E;
             return WS_SEND_OOB_READ_E;
         }
 

src/ssh.c

@@ -3688,49 +3688,30 @@ int wolfSSH_worker(WOLFSSH* ssh, word32* channelId)
         return WS_FATAL_ERROR;
     }
 
-#ifdef WOLFSSH_TEST_BLOCK
-    /* In forced non-blocking test mode, keep legacy ordering (send before
-     * receive) to match the harness expectations and avoid synthetic spins. */
-    if (ret == WS_SUCCESS) {
-        if (ssh->outputBuffer.length != 0)
-            ret = wolfSSH_SendPacket(ssh);
-    }
-    if (ret == WS_SUCCESS)
-        ret = DoReceive(ssh);
-#else
     /* Always service inbound data first so window updates can unblock sends. */
     if (ret == WS_SUCCESS) {
         ret = DoReceive(ssh);
     }
 
-    /* If receive only wanted read or delivered channel data, still try to
-     * flush any pending outbound packets. */
-    if (ret == WS_SUCCESS || ret == WS_WANT_READ || ret == WS_CHAN_RXD
-            || ret == WS_EOF) {
-        int sendRet = WS_SUCCESS;
-
-        if (ssh->outputBuffer.length != 0)
-            sendRet = wolfSSH_SendPacket(ssh);
-
-        /* If send is back-pressured, immediately try another receive to pick
-         * up potential window-adjusts and then return the send status. The
-         * send status wins; a peer EOF stays latched on the channel. */
-        if (sendRet == WS_WANT_WRITE || sendRet == WS_WINDOW_FULL) {
-            int recv2 = DoReceive(ssh);
-            if (recv2 == WS_SUCCESS || recv2 == WS_WANT_READ || recv2 == WS_CHAN_RXD
-                    || recv2 == WS_EOF)
-                ret = sendRet;
-            else
-                ret = recv2;
-        }
-        else {
-            /* Preserve meaningful receive status when send succeeded. */
-            if (sendRet != WS_SUCCESS)
+    /* Flush queued output whatever DoReceive() made of the socket, since an
+     * idle receive reports WS_FATAL_ERROR. !ssh->disconnected gates it. */
+    if (ssh != NULL && !ssh->disconnected && ssh->outputBuffer.length != 0) {
+        int rxErr = ssh->error;
+        int sendRet;
+
+        sendRet = wolfSSH_SendPacket(ssh);
+        if (sendRet != WS_SUCCESS) {
+            if (ret == WS_SUCCESS) {
                 ret = sendRet;
-            /* else leave ret as prior receive result (SUCCESS/WANT_READ/CHAN_RXD). */
+            }
+            else if (rxErr != WS_WANT_READ
+                    && ssh->outputBuffer.length != 0) {
+                /* ret still reports the receive, so put the receive's status
+                 * back */
+                ssh->error = rxErr;
+            }
         }
     }
-#endif /* WOLFSSH_TEST_BLOCK */
 
     /* DoChannelClose() bundles the reply inside DoReceive(), and callers
      * treat the close as terminal, so flush it here. The close stays the
@@ -4163,8 +4144,8 @@ static int _ChannelRead(WOLFSSH_CHANNEL* channel, byte* buf, word32 bufSz)
         }
     }
     else {
-        /* SendPacket() records only WS_WANT_WRITE, so a hard failure has to
-         * be recorded here; rewriting WS_WANT_WRITE is deliberate. */
+        /* The window adjust failed. ssh->error carries the code, and the
+         * log skips a WS_WANT_WRITE, which only asks for a retry. */
         ssh->error = updateResult;
         if (updateResult != WS_WANT_WRITE) {
             WLOG(WS_LOG_ERROR,
@@ -4224,8 +4205,8 @@ static int _ChannelReadExt(WOLFSSH_CHANNEL* channel, byte* buf, word32 bufSz)
                 ssh->error = savedError;
         }
         else {
-            /* SendPacket() sets ssh->error only for WS_WANT_WRITE, so hard
-             * failures must be recorded here or they stay hidden. */
+            /* The window adjust failed. ssh->error carries the code, and
+             * the log skips a WS_WANT_WRITE, which only asks for a retry. */
             ssh->error = adjustResult;
             if (adjustResult != WS_WANT_WRITE) {
                 WLOG(WS_LOG_ERROR,

tests/regress.c

@@ -4499,11 +4499,19 @@ static void TestWorkerReportsDisconnect(void)
     wolfSSH_SetIOReadCtx(ssh, &io);
     wolfSSH_SetIOWriteCtx(ssh, &io);
 
+    /* Queued output, so the flush on this pass has something to push. */
+    ssh->outputBuffer.length = 1;
+    ssh->outputBuffer.idx = 0;
+    ssh->outputBuffer.buffer[0] = 0;
+
     AssertIntEQ(wolfSSH_worker(ssh, NULL), WS_FATAL_ERROR);
     AssertIntEQ(wolfSSH_get_error(ssh), WS_DISCONNECT);
     AssertTrue(ssh->disconnected);
     AssertTrue(ssh->isKeying != 0);
-    io.outSz = 0;
+
+    /* The queued byte stays put: the session ended on this very pass. */
+    AssertIntEQ(io.outSz, 0);
+    AssertTrue(wolfSSH_OutputPending(ssh));
 
     /* The message behind it is still queued, and every further pass reports
      * the disconnect rather than the WS_SUCCESS of a skipped dispatch or the