From 5dd4d6b82e6237f7f53d3efcefdf2b43da22a5bf Mon Sep 17 00:00:00 2001 From: Matteo Nastasi Date: Sun, 13 Sep 2026 13:31:09 +0200 Subject: [PATCH] fix truncated chunk left behind by a short write When fwrite() could not place the whole response on the socket, the part still to be sent was sliced out of the wrong string: $wret is an offset into $response, which carries the http headers and the chunk framing ahead of the content, but the slice was taken from $content. The client was then served a chunk shorter than its declared length and without its terminator, followed by bytes with no framing at all: the javascript parser lost the @BEGIN@/@END@ boundaries and never found them again, which is the comet stream falling apart. sac-a-push.phh already did this right; brisk.phh and briskin5.phh did not. Reproduced on a socket pair whose send buffer is smaller than the payload: with the old slice the chunk declared 324012 bytes and only 323908 followed, with the new one the framing stays consistent and the content comes out whole. It takes a response above the ~200 KB of the socket buffer to show up, which is why a full room (240 players, ~18 KB of bootstrap) never triggered it. Two more defects of the same family: - compress_chunk() re-wrote the whole input after a short write into the deflate stream, instead of the part that was still missing, so the client inflated a chunk carrying duplicated content. - chunked_fini() returned "0\r\n" without the CRLF that closes the trailer section, that is an unterminated last chunk. It has no callers today (stream_close() does the work), but it was the same mistake waiting to be made again. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014M1jiEq9cHdE5SE5j6vFuE --- web/Obj/brisk.phh | 9 ++++++++- web/Obj/user.phh | 6 +++++- web/Obj/zlibstream.phh | 7 ++++++- web/briskin5/Obj/briskin5.phh | 9 ++++++++- 4 files changed, 27 insertions(+), 4 deletions(-) diff --git a/web/Obj/brisk.phh b/web/Obj/brisk.phh index 08adf95..669cbdf 100644 --- a/web/Obj/brisk.phh +++ b/web/Obj/brisk.phh @@ -2876,7 +2876,14 @@ class Brisk $wret = @fwrite($new_socket, $response, $response_l); if ($wret < $response_l) { printf("TROUBLES WITH FWRITE: %d\n", $wret); - $user->rd_cache_set(mb_substr($content, $wret, $response_l - $wret, "ASCII")); + /* The remainder has to be sliced out of $response, not of $content: + $wret is an offset into $response, which carries the http headers + and the chunk framing ahead of the content (and, on a compressed + stream, the deflated bytes instead of the plain ones). Resuming + from $content dropped the header bytes and left the chunk shorter + than its declared length and without its terminator, which is what + made the javascript parser lose the framing for good. */ + $user->rd_cache_set(mb_substr($response, $wret, $response_l - $wret, "ASCII")); } else { $user->rd_cache_set(""); diff --git a/web/Obj/user.phh b/web/Obj/user.phh index 48556e6..e54e138 100644 --- a/web/Obj/user.phh +++ b/web/Obj/user.phh @@ -993,7 +993,11 @@ function chunked_content($content) function chunked_fini() { - return sprintf("0\r\n"); + /* The last chunk is "0" CRLF plus the CRLF that closes the (empty) + trailer section: without the second one the response stays open and + whoever parses it downstream waits for a continuation that never + comes. See stream_close(), which emits the same sequence. */ + return sprintf("0\r\n\r\n"); } diff --git a/web/Obj/zlibstream.phh b/web/Obj/zlibstream.phh index 9468370..cd5b583 100644 --- a/web/Obj/zlibstream.phh +++ b/web/Obj/zlibstream.phh @@ -79,7 +79,12 @@ class ZLibStream { for ($to_be_proc = $s_in_l, $max_fail = 0 ; $to_be_proc > 0 && $max_fail < 2 ; $max_fail++) { if ($to_be_proc > 0) { $max_fail = 0; - if (($ct = fwrite($this->s[0], $s_in)) == FALSE) + /* Only the part that is still missing gets written: passing + $s_in whole again after a short write fed the already + compressed bytes to the deflater a second time, and the + client inflated a chunk with duplicated content in it. */ + if (($ct = fwrite($this->s[0], mb_substr($s_in, $s_in_l - $to_be_proc, + $to_be_proc, 'ASCII'))) == FALSE) return FALSE; $to_be_proc -= $ct; diff --git a/web/briskin5/Obj/briskin5.phh b/web/briskin5/Obj/briskin5.phh index b020ba1..66c6b2d 100644 --- a/web/briskin5/Obj/briskin5.phh +++ b/web/briskin5/Obj/briskin5.phh @@ -1580,7 +1580,14 @@ class Bin5 { $wret = @fwrite($new_socket, $response, $response_l); if ($wret < $response_l) { printf("TROUBLES WITH FWRITE: %d\n", $wret); - $user->rd_cache_set(mb_substr($content, $wret, $response_l - $wret, "ASCII")); + /* The remainder has to be sliced out of $response, not of $content: + $wret is an offset into $response, which carries the http headers + and the chunk framing ahead of the content (and, on a compressed + stream, the deflated bytes instead of the plain ones). Resuming + from $content dropped the header bytes and left the chunk shorter + than its declared length and without its terminator, which is what + made the javascript parser lose the framing for good. */ + $user->rd_cache_set(mb_substr($response, $wret, $response_l - $wret, "ASCII")); } else { $user->rd_cache_set(""); -- 2.47.3