• js_socket: TLS send failures are indistinguishable from short writes

    From Rob Swindell@1:103/705 to GitLab issue in main/sbbs on Sat Jul 25 17:50:27 2026
    open https://gitlab.synchro.net/main/sbbs/-/issues/1199

    ## Summary

    `js_socket_sendsocket()` (`src/sbbs3/js_socket.cpp:381`) reports send failures two different ways depending on whether the socket is TLS-wrapped:

    * **Plaintext** (`p->session == -1`, `:388`) — returns `sendsocket()`'s result,
    i.e. `-1` on error.
    * **TLS** (`:389-416`) — returns a byte count for *every* outcome, including a
    session closed by the peer (`:404-410` returns `total`, typically `0`).

    So on a TLS session a caller cannot distinguish "connection closed" from "short write", and a routine client disconnect is indistinguishable from a real I/O error.

    ## Observed

    ```
    Sat Jul 25 17:31:36 2026 master/9fc5505cbc vert.synchro.net
    srvc 3408 WSS Error: SendToWebSocketClientVersion7 only sent 0 of 138 bytes
    ```

    A `wss://` client went away while the Terminal Server still had a 138-byte chunk queued for it. `exec/websocketservice.js` guards its write with `client.socket.is_connected`, which only reflects the TCP socket — the cryptlib
    session was already gone, `cryptPushData()` failed, and `Socket.send()` returned
    `0`.

    The literal `0` is the TLS signature: on the plaintext `WS` service a failed `sendsocket()` returns `-1`, `js_send()` leaves the return value `JSVAL_VOID` (`:1175-1178`), and the same message would have read `only sent undefined of 138 bytes`. The absence of a companion `TLS ... pushing data` error line points specifically at `CRYPT_ERROR_COMPLETE`, which `:405` deliberately doesn't log.

    The WSS symptom itself is addressed separately in `exec/websocketservice.js` (treat a short write as a disconnect and break the proxy loop rather than throwing). This issue covers the C side.

    ## Proposed changes

    **1. Return `-1` from the TLS branch on failure** — but only when nothing was copied. `:404-410` can be reached after a partial push, and that count must still be returned or the caller will resend already-delivered bytes.

    **2. Fix `js_socket_sendfilesocket()` in the same change — `:435`.** This is a
    prerequisite, not a follow-up:

    ```c
    ptrdiff_t wr = js_socket_sendsocket(p, buf + sent, rd - sent, false);
    if (wr > 0) { sent += wr; }
    else if (wr == SOCKET_ERROR && SOCKET_ERRNO == EWOULDBLOCK) { wr = 0; SLEEP(1); }
    else { ...return wr; }
    ```

    It is the only caller that inspects `SOCKET_ERROR`, and it consults `SOCKET_ERRNO` unconditionally. For a TLS session that errno belongs to cryptlib's own I/O on the underlying socket, where a leftover `EWOULDBLOCK`/`WSAEWOULDBLOCK` is routine. A `-1` return would then take the middle branch, reset `wr` to 0, sleep 1ms, and re-run `while (sent < rd)` with `sent` unadvanced — an infinite loop hanging the JS service thread. Today's `0`
    return falls into the final `else` and unwinds cleanly. Fix by gating the errno check on `p->session == -1`, or by having `js_socket_sendsocket()` distinguish "would block" from "closed" instead of overloading `-1`.

    **3. Cleanup: `js_sendbin()` declares `size_t wr` (`:1362`).** The `-1` from the
    plaintext path already lands there as `SIZE_MAX`; the `wr == (size_t)size` test at `:1401` then fails and the correct error path is taken — right answer, wrong
    reason. Make it `ptrdiff_t`.

    Verified as already handling `-1` correctly and needing no change:
    `js_send()` (`:1172`, `if (ret >= 0)`) and `js_sendline()` (`:1211`, `== len` with short-circuit).

    ## Do NOT surface `-1` to JavaScript

    `js_send()` leaves the return value `undefined` on error, and stock scripts are written against that convention. Changing `Socket.send()` to return `-1` would break:

    * `exec/imapservice.js:198` — `full_send()` tests `sret == undefined ||
    sret == 0`; `-1` slips through, `sent` goes negative, and `str.substr(-1)`
    re-sends the tail in a loop.
    * `exec/load/http.js:106` — `do_send()` tests `ret === 0 || null || false`;
    `-1` slips through and `sent` decreases monotonically, so
    `while (sent < str.length)` never terminates.
    * `exec/broker.js:922` (tests `bytes === null`) and `exec/load/ircd/core.js:587`
    (`if (sent > 0)`) degrade rather than hang — the transmit buffer stops
    draining.

    `Socket.sendline()` and `Socket.sendBin()` return booleans and are unaffected.

    — *Authored by Claude (Claude Code), on behalf of @rswindell*
    --- SBBSecho 3.37-Linux
    * Origin: Vertrauen - [vert/cvs/bbs].synchro.net (1:103/705)