open
https://gitlab.synchro.net/main/sbbs/-/issues/1188
`Socket.recvline()` treats its `timeout` as an absolute deadline measured from the moment it's called. If that deadline expires *mid-line* — after at least one byte has been received but before the terminating LF — it returns the **truncated fragment as an ordinary string**, indistinguishable from a complete line. The caller has no way to tell "here is your line" from "I gave up partway through".
### Where
`src/sbbs3/js_socket.cpp`, `js_recvline()`:
```c
switch (js_sock_read_check(p, start, timeout, i)) {
case 1: // time-out */
case 2: // disconnected
if (i) { // some data was received before the error/disconnection
len = 0; // so break the loop
continue;
}
// no data received, so just return null
```
With `i == 0` the function correctly returns `null`. With `i > 0` it falls out of the loop and returns whatever partial bytes it has. `js_sock_read_check()` only tests the deadline when the socket is *not* readable, so buffered data always wins — the truncation only bites when a peer's response straddles the deadline.
Introduced in 13d22506ca (`fires-11-domain`, 2018-08-14), which added the partial-return to break an infinite error/log loop on a TLS read error. Returning the data was reasonable for that bug; the problem is that a *timeout* is reported to the caller as success.
### Observed impact
This is the root cause of two distinct spamc/SpamAssassin error families in `data/error.log` on `cvs.synchro.net` (51 occurrences over ~2 weeks). `exec/load/salib.js` called `sock.recvline()` bare, so it took the 30-second default, while `spamd` doesn't respond until it has finished scanning — which under load, with network tests/DNSBLs, routinely exceeds 30s:
- deadline expires before any byte → `null` → `!ERROR No lines read from spamd` (35x)
- deadline expires mid-line → `"S"`, the first byte of `SPAMD/1.1 0 EX_OK` → `!ERROR Unable to parse line 'S` (17x)
### Reproduction
Against a stub spamd, driving `recvline()` with no arguments (i.e. `maxlen=512, timeout=30`):
| Stub timing | Result |
|---|---|
| first byte at +31s (past deadline) | `No lines read from spamd` |
| first byte at +29s, remainder at +33s (**straddles** the deadline) | `Unable to parse line 'S` |
| first byte at +5s, remainder at +8s (same gap, inside deadline) | parses fine — `SPAMD/1.1 0 EX_OK`, 3 lines |
The third row is the control: an identical mid-line gap succeeds when the deadline hasn't passed, isolating the deadline expiry — not the gap — as the cause.
### Scope
The `salib.js` side is fixed separately by passing an explicit timeout (600s, matching `spamc(1)`'s own `-t` default). But `recvline()`'s contract affects **every caller in the tree**, and any of them can silently receive a truncated line from a slow peer.
Options, roughly in increasing order of blast radius:
1. Return `null` on a mid-line timeout (arguably correct, but changes behavior for all callers and could resurrect the infinite-loop class of bug 13d22506ca fixed).
2. Keep returning the partial line but expose the truncation — e.g. set a socket property alongside the existing `last_error` so callers can opt into checking.
3. Document the current behavior as the contract and leave it, on the grounds that callers should pass sane timeouts.
I lean toward (2): it's additive, doesn't regress 13d22506ca, and gives careful callers a way to be correct. Flagging for a maintainer decision rather than patching unilaterally, since (1) in particular is a tree-wide behavior change.
— *Authored by Claude (Claude Code), on behalf of @rswindell*
--- SBBSecho 3.37-Linux
* Origin: Vertrauen - [vert/cvs/bbs].synchro.net (1:103/705)