fix(websocket): drain before closing an overflowed peer

CHANGELOG.md

@@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **A WebSocket peer flooded past the inbound cap, was closed 1013, and never saw the code (#305).** `ws_session_feed` returns -1 on `recv_overflow`, the connection layer closes the socket, and the peer — which overflowed the cap by outrunning the reader — still has unread bytes in its receive buffer. A socket closed on top of unread bytes is reset rather than finished, and the reset discards the 1013 this side has already written, so the client reads nothing and learns nothing about why its connection ended. This is the shape #287 named for HTTP/1, where a 413 was lost the same way and #288 answered with a lingering close; the WebSocket overflow path had none. The overflow teardown opens the same drain, carried by the deadline tick that already walks the connection list, so no timer is added. Only a plaintext upgraded HTTP/1 connection arms it. Over HTTP/2 the session is one stream among several, and stalling the connection to drain one stream would punish the rest. Over TLS the drain does not exist: it is fed from the plaintext read paths, and the TLS read FSM has no hook into them, so `wss` loses its 1013 exactly as before — and `https` its 413, which #288 did not close either. Arming it there would spend the deadline and end in the same reset, so `http_connection_linger_begin` refuses a TLS connection, and the gap is an open item in `dev/PLAN.md` rather than a claim. Evidence: `websocket/035` reads `client saw close: NULL` on 5 of 10 runs before and 0 of 20 after; with the client's write cut to 13 frames — just past the 8 KiB cap, nothing left unread — it passes either way, which is the cause. The test itself hid this: `usleep()` and `microtime()` in its FILE section make run-tests retry it and report only the retry, so a failure on one run in two never reached suite output. Both calls are gone.
 - **A TCP syslog sink wrote nothing on Windows, and its socket had two owners everywhere (#293).** Every record was counted as dropped: `log_io_type_for_fd` carried its whole body under `#ifndef PHP_WIN32`, so a socket was driven through the file path and `uv_fs_write` refused the SOCKET with EBADF. With the type detection in place the records arrive and uncover what the file path was hiding — the sink handed the descriptor to `ZEND_ASYNC_IO_CREATE` with `ZEND_ASYNC_IO_PRESERVE_FD` and kept the stream, on the assumption that the flag leaves the descriptor with it. The flag reaches only a descriptor the reactor would otherwise close itself; a socket is adopted by a libuv stream handle that closes it on teardown regardless, so the stream closed it a second time when the caller's resource went. Windows answered that with `socket operation on non-socket`, POSIX with a silent EBADF — the worse half, because the descriptor number is reused and a late close lands on another connection. Ownership now follows the handle type: a file descriptor stays with the stream under `PRESERVE_FD`, a socket belongs to the io and the stream is marked `PHP_STREAM_FLAG_NO_CLOSE`. Duplicating the socket instead was tried and is not a fix — closing the copy sends no FIN, so the collector never sees EOF, and `uv_tcp_open` puts the shared file description into non-blocking mode behind the stream's back. Two defects a review found on the way are fixed with it: `php_stream_cast` writes a `php_socket_t` through the pointer it is given, and the `int` that carried it took an eight-byte write past its own storage on 64-bit Windows; and a Windows socket the io cannot adopt — a datagram one, or the AF_UNIX that libuv will not take — was left on the file path, where it looks like a working sink and drops every record, so the sink refuses to start instead. Evidence: `core/023` fails 3 of 3 runs before and passes 5 of 5 after; the Windows suite goes from 3 failures to 2.
 - **The Windows build answered every request with identity encoding, whatever the client offered (#291).** Compression was compiled, zlib was linked and `isCompressionEnabled()` said true; `http_accept_encoding_select` returned `HTTP_CODEC_IDENTITY` all the same. It picks a codec inside `#ifdef HAVE_HTTP_COMPRESSION`, and `http_compression_negotiate.c` is pure C by design — the unit suite links it alone — so it includes no PHP header. On POSIX the macro arrives through `config.h`; on Windows it lives in `main/config.w32.h`, which a translation unit sees only through `php.h`, and the block compiled out. `http_compression.c` gates the brotli and zstd registry entries the same way, so those backends would have been built and never reached. `config.w32` passes the three macros on the command line beside the `AC_DEFINE`, as it already does for `HAVE_LLHTTP`. Evidence: a 4200-byte `text/html` body asked for with `Accept-Encoding: gzip` comes back `encoding=gzip` and `Content-Length: 69` where it was 4200 uncompressed, on the buffered and the streamed route alike; `compression/074` fails on its first assertion before and passes after. The rest of the compression group skips on Windows, where the zlib extension the tests decode with is absent, so that one assertion was the whole coverage.
 - **A streaming HTTP/2 handler that threw could lose the whole response, status line included (#285).** The peer read `status=0` on a stream whose two chunks the handler had written, while RST_STREAM(INTERNAL_ERROR) arrived and every later stream on the connection was served. `h2_stream_append_chunk` ends with `http2_session_emit`, which skips the send while a writev is in flight and leaves the frames to the write completion's re-drive; `h2_stream_abort` then submitted RST_STREAM, and nghttp2 drops what it has queued for a stream it moves to the closing state — the HEADERS commit among it. The ring-exists check that guards the reset reads a non-NULL `chunk_queue` as "the HEADERS are on the wire", and a skipped emit breaks that. The abort now flushes the session through `http2_session_emit_now` before it raises `streaming_ended` and submits the reset, so everything the handler wrote reaches the peer ahead of the reset, on every platform. Evidence: `h2/053` fails 3 of 3 runs on Windows before the fix with an empty body and passes 5 of 5 after; the same test with a 100 ms delay before the first `write()` passes without the fix, which is the race. The test asserted `body=alpha` — one platform's timing, the second chunk lost to the same race — and asserts `body=alphabeta` now.

dev/PLAN.md

@@ -1656,7 +1656,7 @@ a release yet except the first two, which are fixed.
 
 ## A WebSocket close is lost when the peer is still sending (#303 fallout)
 
-- [ ] **The 1013 close never reaches a client that overflowed the inbound
+- [x] **The 1013 close never reaches a client that overflowed the inbound
   cap.** `websocket/035-recv-queue-overflow` fails on Windows with
   `client saw close: NULL` while the handler side reports 1013 correctly, so
   the decision is right and the frame does not arrive. The shape is the one
@@ -1667,10 +1667,36 @@ a release yet except the first two, which are fixed.
   has written. #288 gave HTTP/1 a lingering close for exactly this; the
   WebSocket overflow path has none.
 
-  Not proved yet, and the cheap proof is the same one that settled #287: cut
-  the client's write to just past the cap, leaving nothing unread, and see
-  whether the close arrives. Windows shows it because a reset there is
-  immediate; Linux may be hiding it behind timing rather than avoiding it.
+  The cheap proof is the one that settled #287, and it settled this too: run
+  outside run-tests, the test reads `NULL` on 5 of 10 runs; cut the client's
+  write to 13 frames — just past the cap, nothing left unread — and it is 0 of
+  10, while 60 frames is 7 of 10. The condition is the unread tail, not the
+  volume. Windows shows it because a reset there is immediate; Linux may be
+  hiding it behind timing rather than avoiding it.
+
+  Fixed in #305 for a plaintext ws connection: the overflow teardown arms the
+  same drain #288 built, on an upgraded H1 connection only — over H2 the session
+  shares the connection with other streams. 0 of 20 after, and the whole phpt
+  suite is 259 executed, 0 failed. Over wss it is still lost; see the TLS item
+  below.
 
   Found while making the group build on Windows at all (#303), where it is the
   single failure out of 61 executed WebSocket tests.
+
+- [ ] **The lingering close has no TLS path, so wss and https keep losing what a
+  reset discards.** The drain is fed from the plaintext read paths alone —
+  `http_connection_linger_note_inbound` is reached from `http_connection.c:1324`
+  and `:1429`, and `http_connection_tls.c` does not mention the drain at all. A
+  TLS connection that armed it would wait with nothing being read and close on
+  the same unread bytes at the end, so `linger_begin` refuses there rather than
+  spending the deadline for nothing. That refusal is the honest state and not
+  the fix: a 413 over https and a 1013 over wss are lost exactly as #287
+  described. Closing it means giving the TLS read FSM the drop-and-refresh hook
+  the plaintext paths already have. Found by the review of #305.
+
+- [ ] **The permessage-deflate teardown has the same shape and no proof.**
+  `pmce_error` queues a 1009 and returns -1 down the same path, so a peer whose
+  compressed message overflows the cap should lose its close for the same
+  reason. No run of it has been made to fail, so it is left as it stands rather
+  than fixed on the resemblance. The proof, if it is wanted, is the one above:
+  a bomb just past the cap against a flood well past it.

fuzz/fuzz_ws_frame.c

@@ -80,6 +80,13 @@ void http_connection_destroy_if_idle_deferred(http_connection_t *conn)
     (void)conn;
 }
 
+/* The drain the inbound-cap overflow opens on a connection it owns. conn is
+ * NULL here, so the overflow path skips it. */
+void http_connection_linger_begin(http_connection_t *conn)
+{
+    (void)conn;
+}
+
 bool http_connection_send(http_connection_t *conn, const char *data, size_t len)
 {
     (void)conn; (void)data; (void)len;

src/core/http_connection.c

@@ -330,6 +330,9 @@ void http_connection_destroy_if_idle_deferred(http_connection_t *conn)
  * nothing about why its upload ended. nginx answers this with lingering_close,
  * Apache with ap_lingering_close.
  *
+ * A WebSocket peer flooding past the inbound FIFO cap reaches the same reset,
+ * and loses its CLOSE 1013 to it.
+ *
  * The drain is the read that is already armed: what arrives is thrown away, and
  * the destroy waits. HTTP_LINGER_IDLE_MS measures silence from the peer and
  * every chunk buys another window of it; HTTP_LINGER_MAX_MS caps the sum, so a
@@ -350,13 +353,21 @@ static uint64_t http_connection_linger_next_deadline(const http_connection_t *co
     return idle_until < conn->linger_until_ms ? idle_until : conn->linger_until_ms;
 }
 
-static void http_connection_linger_begin(http_connection_t *conn)
+void http_connection_linger_begin(http_connection_t *conn)
 {
-    if (conn->linger_close || !conn->parse_error_handled
+    if (conn->linger_close
         || conn->io == NULL || conn->write_failed || conn->write_timed_out) {
         return;
     }
 
+#ifdef HAVE_OPENSSL
+    /* Only the plaintext read paths drop what arrives, so on TLS the wait would
+     * run with nothing being read and close on the same unread bytes. */
+    if (conn->tls != NULL) {
+        return;
+    }
+#endif
+
     conn->linger_close = 1;
     conn->read_buffer_len = 0;
     conn->keep_alive = false;

src/core/http_connection.h

@@ -428,6 +428,14 @@ http_connection_t *http_connection_create(php_socket_t socket_fd,
                                           struct http_server_object *server);
 void http_connection_destroy(http_connection_t *conn);
 
+/* Opens a lingering close: the connection keeps reading and throws away what
+ * arrives, and destroy waits until the peer falls silent or the drain hits its
+ * cap. For a caller that has written something the peer has not read while it
+ * is still sending — without the wait the close resets the socket and takes
+ * that with it. Ignored on a TLS connection, on one whose write side is gone,
+ * and on one already draining; http_connection_linger_end cuts the wait short. */
+void http_connection_linger_begin(http_connection_t *conn);
+
 /* Ends a lingering close, so the next destroy closes instead of waiting out the
  * drain. For a caller tearing the connection down whatever the peer is doing —
  * the drain is finished by the deadline tick, and a caller that outlives the

src/websocket/ws_session.c

@@ -1341,8 +1341,19 @@ int ws_session_feed(ws_session_t *session, const uint8_t *data, size_t len)
     }
 
     /* Inbound FIFO overflowed its byte cap: the 1013 close queued in
-     * on_msg_recv was flushed by the drive above; tear the transport down. */
+     * on_msg_recv was flushed by the drive above; tear the transport down.
+     *
+     * The peer outran the reader to get here, so it is still sending and what
+     * it has already put on the wire is unread — the drain keeps the close from
+     * being discarded by the reset that closing on top of those bytes sends.
+     * Only an upgraded H1 connection owns its socket; over H2 the session is
+     * one stream among several, and RFC 8441 ends that stream instead. */
     if (session->recv_overflow) {
+        if (session->conn != NULL
+            && session->conn->protocol_type == HTTP_PROTOCOL_WEBSOCKET) {
+            http_connection_linger_begin(session->conn);
+        }
+
         return -1;
     }
 #ifdef HAVE_HTTP_COMPRESSION

tests/phpt/websocket/035-recv-queue-overflow.phpt

@@ -64,8 +64,10 @@ function ws_client_text_frame(string $payload): string {
     return $hdr . $mask . $masked;
 }
 
+/* No usleep() or microtime() below: run-tests retries a FILE section that calls
+ * either and reports only the retry, which hides a close arriving one run in two. */
 $client = spawn(function () use ($port, $server) {
-    usleep(20000);
+    delay(20);
     $fp = stream_socket_client("tcp://127.0.0.1:$port", $errno, $errstr, 2);
 
     fwrite($fp,
@@ -91,9 +93,13 @@ $client = spawn(function () use ($port, $server) {
      * before the cap are still being drained — so the stream is walked frame
      * by frame rather than sampled at byte 0. */
     $close_code = null;
-    $deadline = microtime(true) + 5;
+    /* Attempts, not a clock: a clock here means microtime(). Only an empty read
+     * waits, and the timeout below caps it, so 60 attempts is about 13 s against
+     * a peer that never answers. */
+    stream_set_timeout($fp, 0, 200000);
+    $attempts_left = 60;
 
-    while (microtime(true) < $deadline) {
+    while ($attempts_left-- > 0) {
         $frame = ws_read_frame($fp);
 
         if ($frame === null) {