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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014M1jiEq9cHdE5SE5j6vFuE

diff --git a/web/Obj/brisk.phh b/web/Obj/brisk.phh
index 08adf95a9fb72d0c7be15f41d66db834b0b966cf..669cbdf427dd916eeafb3db3aa73388d545747b1 100644 (file)
--- 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 48556e6f75e63723ca1bec9cf8401325887a93d3..e54e1383629a735d5976304319eed3318c8fa13a 100644 (file)
--- 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 9468370211de8739335cbca32e45cb3e3e953d72..cd5b5834dfff5ac96cfa8cdcf37893ed0f4e411d 100644 (file)
--- 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 b020ba151741af9f26b417cb4f8b72c2badda367..66c6b2dac4f9b8bc788d44b08367f7c91960f8df 100644 (file)
--- 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("");