chat: both sides count the text as the wire carries it
The whole-branch review's W4-1. The panel counted 2048 UTF-8 bytes of the text; the server counted 2048 + 32 raw JSON bytes of the payload. Between those two rules sits a draft the panel calls legal and the server refuses: 2048 bytes of text with twenty-three quotes in it is 2071 raw bytes of payload, and the 413 that comes back is one nothing on the client could have foreseen or explained. One rule now, on both sides: the JSON-escaped length of the text, at most 2048 bytes. That is exactly what the canonical payload from the S4-2 fix puts on every recipient's stream, so the number the limit names is the number the wire carries. A quote, a backslash and a newline cost two bytes each; an angle bracket costs one, because SSE data is not HTML and neither side escapes it. The server measures the canonical form it just built; chat.js measures JSON.stringify(text) less its two quotes. The raw 2048 + 32 cap stays exactly where it was, as the outer guard that refuses an oversized body before any of this is reached. Tests at the boundary on both sides, each seen failing first: a quote, a newline and a backslash one byte past it (200 where 413 was wanted), and the panel's counter reading 1 where the wire carries 2. node --check, 180 node tests, make ci.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CzvnuKhLzudvq7jZ7SvVt2
4 files changed,
+90
−11
internal/meet/signal.go+12 −3internal/meet/static/chat.js+19 −8internal/meettest/chat_test.go+37 −0test/browser/chat.test.mjs+22 −0
diff --git a/internal/meet/signal.go b/internal/meet/signal.go| index a4bc997..6d0ad9b 100644 |
| --- a/internal/meet/signal.go |
| +++ b/internal/meet/signal.go |
| @@ -84,15 +84,24 @@ func validateFanout(typ string, raw json.RawMessage) (json.RawMessage, int, stri |
| if len(*p.Text) == 0 { |
| return nil, http.StatusBadRequest, "shape" |
| } |
| - if len(*p.Text) > maxChatBytes { |
| - return nil, http.StatusRequestEntityTooLarge, "size" |
| - } |
| canon, ok := canonicalJSON(struct { |
| Text string `json:"text"` |
| }{Text: *p.Text}) |
| if !ok { |
| return nil, http.StatusBadRequest, "shape" |
| } |
| + // §4.4's 2 KB, counted on the escaped text — the bytes this |
| + // payload actually puts on every recipient's stream, which is |
| + // what the limit is there to bound. Counting the decoded string |
| + // instead measured something nobody sends: a quote, a backslash |
| + // or a newline is one byte decoded and two on the wire, so a |
| + // 2048-byte text with twenty-three quotes in it passed the |
| + // client's own counter and came back 413 from a rule the client |
| + // could not apply (whole-branch review, W4-1). chat.js counts |
| + // the same thing, with JSON.stringify. |
| + if len(canon)-len(`{"text":""}`) > maxChatBytes { |
| + return nil, http.StatusRequestEntityTooLarge, "size" |
| + } |
| return canon, 0, "" |
| case hub.TypeShare: |
| if !exactKey(raw, "on") { |
diff --git a/internal/meet/static/chat.js b/internal/meet/static/chat.js| index 07918ee..06bcb02 100644 |
| --- a/internal/meet/static/chat.js |
| +++ b/internal/meet/static/chat.js |
| @@ -28,10 +28,11 @@ |
| (() => { |
| 'use strict'; |
| - // §4.4's limit, in bytes of UTF-8, not characters. The control's |
| - // maxlength caps characters, which is the courtesy the browser can |
| - // offer for free; this is the one the server actually measures, and |
| - // for anything outside ASCII the two disagree. |
| + // §4.4's limit, in bytes of the text as the wire carries it — not |
| + // characters. The control's maxlength caps characters, which is the |
| + // courtesy the browser can offer for free; this is the one the server |
| + // actually measures, and for anything outside ASCII, and for anything |
| + // JSON has to escape, the two disagree. |
| const MAX_BYTES = 2048; |
| // The log is a window, not a record. Five hundred lines is far past |
| @@ -47,11 +48,21 @@ |
| const SEND_FAILED = 'Could not send. Try again.'; |
| const TOO_LONG = 'Message is too long.'; |
| - // chatBytes is the byte counter: the length of text as UTF-8, which |
| - // is what §4.4's 2 KB counts and what the server will refuse above. |
| - // Lifted by name in test/browser/chat.test.mjs. |
| + // chatBytes is the byte counter: the length of the text as the wire |
| + // will carry it — its JSON-escaped UTF-8 form, without the quotes |
| + // around it — which is what §4.4's 2 KB counts and what the server |
| + // refuses above. Lifted by name in test/browser/chat.test.mjs. |
| + // |
| + // The escaped form, not the raw one: the server measures the canonical |
| + // JSON it re-encodes and fans out, where a quote, a backslash or a |
| + // newline is two bytes rather than one. Counting raw UTF-8 here let a |
| + // draft the panel called 2048 bytes come back 413 from a rule the |
| + // panel had no way to apply (whole-branch review, W4-1). Angle |
| + // brackets and ampersands are not escaped on either side — SSE data is |
| + // not HTML — so they cost one byte each here too. |
| function chatBytes(text) { |
| - return new TextEncoder().encode(String(text == null ? '' : text)).length; |
| + const s = JSON.stringify(String(text == null ? '' : text)); |
| + return new TextEncoder().encode(s).length - 2; |
| } |
| // chatLine builds one line of the log and nothing else: no document |
diff --git a/internal/meettest/chat_test.go b/internal/meettest/chat_test.go| index e7f2011..8340817 100644 |
| --- a/internal/meettest/chat_test.go |
| +++ b/internal/meettest/chat_test.go |
| @@ -395,6 +395,43 @@ func TestChatDoesNotExpandOnTheWire(t *testing.T) { |
| } |
| } |
| +// The whole-branch review's W4-1: the panel counted 2048 UTF-8 bytes of |
| +// the text and the server counted 2080 raw JSON bytes of the payload, so |
| +// a 2048-byte draft with twenty-three quotes in it passed the panel and |
| +// came back 413 — a refusal nothing on the client could have foreseen. |
| +// Both sides count one thing now: the JSON-escaped length of the text, |
| +// which is exactly what the canonical payload puts on the wire. The raw |
| +// cap stays as the outer guard, refusing an oversized body before any of |
| +// this is reached. |
| +func TestChatCapCountsTheEscapedText(t *testing.T) { |
| + a, sqid, pa, _ := chatPair(t) |
| + |
| + cases := []struct { |
| + name string |
| + text string |
| + status int |
| + }{ |
| + {"a quote at the boundary", strings.Repeat("a", chatCap-2) + `\"`, http.StatusOK}, |
| + {"a quote one byte past it", strings.Repeat("a", chatCap-1) + `\"`, http.StatusRequestEntityTooLarge}, |
| + {"a newline at the boundary", strings.Repeat("a", chatCap-2) + `\n`, http.StatusOK}, |
| + {"a newline one byte past it", strings.Repeat("a", chatCap-1) + `\n`, http.StatusRequestEntityTooLarge}, |
| + {"a backslash one byte past it", strings.Repeat("a", chatCap-1) + `\\`, http.StatusRequestEntityTooLarge}, |
| + {"twenty-three quotes inside 2048 bytes of text", |
| + strings.Repeat("a", chatCap-23) + strings.Repeat(`\"`, 23), http.StatusRequestEntityTooLarge}, |
| + {"angle brackets cost one byte each, not six", |
| + strings.Repeat("<", chatCap), http.StatusOK}, |
| + } |
| + for _, c := range cases { |
| + t.Run(c.name, func(t *testing.T) { |
| + body := `{"peer":"` + pa + `","type":"chat","payload":{"text":"` + c.text + `"}}` |
| + res, respBody := a.signalRaw(sqid, body) |
| + if res.StatusCode != c.status { |
| + t.Fatalf("%s: %d %s, want %d", c.name, res.StatusCode, respBody, c.status) |
| + } |
| + }) |
| + } |
| +} |
| + |
| // The wire is line-oriented and the payload is a person's own text, so |
| // the one thing a chat must never be able to do is end its own frame |
| // and start another. It cannot: JSON escapes a newline inside a string |
diff --git a/test/browser/chat.test.mjs b/test/browser/chat.test.mjs| index 19113fd..c7401d4 100644 |
| --- a/test/browser/chat.test.mjs |
| +++ b/test/browser/chat.test.mjs |
| @@ -332,6 +332,28 @@ test('the byte counter counts UTF-8 bytes, not characters', () => { |
| assert.equal('🙂'.length, 2, 'a surrogate pair is two characters and four bytes'); |
| }); |
| +test('the counter measures the text as the wire will carry it', () => { |
| + // The whole-branch review's W4-1. The server measures the canonical |
| + // JSON form — the bytes it re-encodes and fans out — so a quote, a |
| + // backslash and a newline each cost two bytes there. A panel counting |
| + // raw UTF-8 would let a 2048-byte draft with twenty-three quotes in it |
| + // go, and the 413 that came back would be one nobody in the panel |
| + // could have foreseen. |
| + assert.equal(chatBytes('"'), 2); |
| + assert.equal(chatBytes('\\'), 2); |
| + assert.equal(chatBytes('\n'), 2); |
| + assert.equal(chatBytes('\t'), 2); |
| + assert.equal(chatBytes('a"b'), 4); |
| + // A control character with no short escape spells out in full. |
| + assert.equal(chatBytes(''), 6); |
| + // And these are not escaped on either side: SSE data is not HTML. |
| + assert.equal(chatBytes('<b>&</b>'), 8); |
| + assert.equal(chatBytes('a'.repeat(MAX_BYTES - 2) + '"'), MAX_BYTES, |
| + 'the boundary the server accepts'); |
| + assert.ok(chatBytes('a'.repeat(MAX_BYTES - 1) + '"') > MAX_BYTES, |
| + 'one byte past it, and the panel must say so before the server does'); |
| +}); |
| + |
| test('maxlength is not the limit: 2048 characters can be far more than 2048 bytes', () => { |
| // Exactly at the cap in ASCII, and the control would allow it. |
| assert.equal(chatBytes('a'.repeat(MAX_BYTES)), MAX_BYTES); |