Skip to content

Expose pong() so an idle connection can be kept alive - #33

Open
tommedema wants to merge 4 commits into
lexiforest:mainfrom
tommedema:feat/ws-pong
Open

Expose pong() so an idle connection can be kept alive#33
tommedema wants to merge 4 commits into
lexiforest:mainfrom
tommedema:feat/ws-pong

Conversation

@tommedema

Copy link
Copy Markdown

Stacked on #30, #31 and #32 (this branch contains all three).

The problem

A WebSocket consumer that only receives gets silently disconnected.

libcurl answers a server PING by queueing a PONG, and in CONNECT_ONLY mode nothing is written to the socket until the application calls curl_ws_send. Polling recv() does not flush it. So a client that sits and reads — the normal shape for a subscription — never actually delivers a pong.

Measured against a server that pings every 20s and enforces a pong deadline: an idle-but-actively-reading client stays up at 30s and 60s and is gone by 90s. There is no error until the next receive, so it looks like the server just stopped sending.

Why a new method

There was no way out through the public API:

  • sendPong was private.
  • ping() works, but provokes a server PONG that then has to be filtered back out of the message stream — and it puts a frame on the wire that the client being impersonated would not send.

An unsolicited pong is the standard unidirectional heartbeat (RFC 6455 §5.5.3) and is the quietest frame that flushes the queue: no reply, no stream pollution.

const ws = await wsConnect(url);
const keepalive = setInterval(() => void ws.pong().catch(() => {}), 20_000);

Full suite: 161 passed, 1 suite skipped (live fingerprints).

Opening a WebSocket made the process die with SIGSEGV during teardown, after
the script had already run to completion and produced all of its output.

`performConnect` ran `curl_easy_perform` through Koffi's `.async()`, which
dispatches to a libuv worker thread. Driving libcurl from one leaves per-thread
state behind whose pthread TSD destructor lives in the libcurl image. At exit
the worker thread is torn down, `_pthread_tsd_cleanup` calls that destructor
after the image has gone, and the process faults:

    EXC_BAD_ACCESS (SIGSEGV), KERN_INVALID_ADDRESS
    ???  ->  _pthread_tsd_cleanup  ->  _pthread_exit  ->  _pthread_start

It is not WebSocket-specific: a plain HTTPS GET issued through `.async()`
faults the same way. WebSockets are simply the only path that used it.

Making the connect synchronous fixes the crash but deadlocks any caller whose
server shares the process — which is what the tests in this repository do, and
why `.async()` was chosen. So the handshake is driven with the multi interface
instead: `curl_multi_perform` returns as soon as there is nothing to do right
now, so the connect advances across event-loop turns without ever leaving the
main thread. Both constraints hold at once. The handle stays attached to the
multi for the life of the socket, since removing it would drop the connection
that CONNECT_ONLY exists to keep.

`curl_easy_perform_async` had no other caller and was never exported from the
package entry, so it is removed rather than left as a trap.

The regression test spawns a child process, because nothing observable from
inside the process can catch a fault that happens after the last line runs —
only the exit status can. It fails on the unfixed code and passes on the fix.

With the crash gone, the WebSocket suite skipped in fbd0ac9 is re-enabled; it
passes, along with the rest of the suite.
`wsConnect` accepted `headers`, `cookies`, `impersonate`, `proxy` and `verify`
and silently dropped every one of them. `headers` got as far as being built into
a curl header list that was then discarded:

    const headerList = headers.toCurlHeaders();
    // Note: Would need SList here for actual implementation

and the rest were never read at all — the constructor only ever set URL,
CONNECT_ONLY and TIMEOUT.

The effect is worse than a missing feature, because it fails silently and only
on the wire. A caller who passes a session cookie gets an unauthenticated
handshake. A caller who passes `impersonate` gets plain libcurl's TLS
fingerprint while believing they are presenting a browser's, which is precisely
the guarantee the package exists to provide — and the HTTP side of the same
session does honour it, so one process can end up presenting two different
fingerprints to the same host.

`Curl` already had `setHeaders`, `impersonate` and the relevant options; they
just were not called. Impersonation is applied first, because with default
headers enabled it installs the browser's own header list and would otherwise
replace the caller's; the caller's headers go on afterwards so they win. That
ordering is why `defaultHeaders` is now accepted here too, matching
`RequestOptions`.

Verified against a server that records the upgrade request, since the wire is
the only place the difference shows. The tests fail on the previous code.
`curl_ws_recv` returns its frame metadata through a
`const struct curl_ws_frame **meta` out-parameter. It was declared to Koffi as a
plain input `void **`, so Koffi marshalled a copy of the array it was handed and
never wrote the pointer back. `frame` was therefore always null and
`frameToMessage` fell through to its TEXT default for every frame.

The consequences were not cosmetic:

  - BINARY frames arrived as text.
  - CLOSE frames arrived as an ordinary two-byte text message. `closed` stayed
    false, `closeEvent` stayed null, no close was echoed, and a consumer went on
    polling a socket the server had already closed — for as long as it cared to.
    A server-initiated close was effectively invisible.
  - The auto-PONG branch keyed on the PING type could never run.

Marking the parameter `_Out_` is the whole fix. Verified against a server that
emits one frame per opcode in a known order: before, text/binary/close all came
back as `text` and the third was two bytes of raw close code; after, they are
reported as text, binary, and a `WebSocketClosed` carrying code 1001.
libcurl answers a server ping by queueing a pong, and in CONNECT_ONLY mode
nothing reaches the socket until the application sends something. A consumer
that only ever receives therefore never delivers a pong, and a server that
enforces a pong deadline closes the connection — measured against one such
server, an idle-but-actively-reading client is dropped between 60 and 90
seconds, with no error until the next receive.

There was no way out of that through the public API: `sendPong` was private and
`ping()` provokes a reply that then has to be filtered back out of the message
stream. An unsolicited pong is the standard unidirectional heartbeat (RFC 6455
section 5.5.3) and is the quietest frame that flushes the queue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant