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; a receive that reported
  anything but WS_WANT_READ keeps ssh->error. 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
  failure path: a null send callback, a bad buffer state, the
  connection-reset/close/general cases, and an out-of-bounds
  read. Its header comment states that contract.
- tests cover the flush on an idle receive, the owed flush across
  calls, a receive failure and a hard send failure each landing in
  ssh->error, and that a disconnect on the same pass leaves queued
  output unsent.

Author: yosuke-wolfssl

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

@@ -3614,46 +3614,29 @@ 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) {
-        int sendRet = WS_SUCCESS;
-
-        if (ssh->outputBuffer.length != 0)
-            sendRet = wolfSSH_SendPacket(ssh);
+    /* 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;
 
-        /* If send is back-pressured, immediately try another receive to pick
-         * up potential window-adjusts and then return the send status. */
-        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)
-                ret = sendRet;
-            else
-                ret = recv2;
-        }
-        else {
-            /* Preserve meaningful receive status when send succeeded. */
-            if (sendRet != WS_SUCCESS)
+        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) {
+                /* Only WS_WANT_READ means DoReceive() had nothing to
+                 * report; anything else outranks wolfSSH_SendPacket(). */
+                ssh->error = rxErr;
+            }
         }
     }
-#endif /* WOLFSSH_TEST_BLOCK */
 
     /* WS_EXTDATA reports the channel too, so a multi-channel caller can route
      * the drain to wolfSSH_ChannelIdReadExt(). */
@@ -4074,8 +4057,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,
@@ -4135,8 +4118,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