Fix chunked write() and beginWrite() calls

[notaphplover]

Summary

Fixes uNetworking/uWebSockets#1939: HTTP/1.1 chunked write() / beginWrite() / end() did not emit complete chunks as defined by RFC 9112 §7.1.

write() prefixed a CRLF and omitted the chunk-data trailer. That leading CRLF was reused as (1) the header/body separator on the first call and (2) the previous chunk’s trailer on later calls. beginWrite() already wrote the header terminator, so the first write() added a second CRLF. A spec-compliant parser then treated 0x0D as a chunk-size digit (chunk hex-length char not a hex digit: 0xd).

On a long-lived stream that never calls end() (SSE), the last write() had no following operation to supply the deferred trailer, so the last event was not a complete chunk and was not delivered.

This PR makes each write() emit a complete chunk, makes beginWrite() only terminate headers, and makes end() emit only last-chunk + empty trailer-section + final CRLF.

Spec (why the old bytes were illegal)

RFC 9112 §2.1 — headers end with one empty line:

HTTP-message = start-line CRLF
               *( field-line CRLF )
               CRLF
               [ message-body ]

RFC 9112 §7.1 — the body, when Transfer-Encoding: chunked, is:

chunked-body   = *chunk
                 last-chunk
                 trailer-section
                 CRLF

chunk          = chunk-size [ chunk-ext ] CRLF
                 chunk-data CRLF
last-chunk     = 1*("0") [ chunk-ext ] CRLF
trailer-section = *( field-line CRLF )

A data chunk is therefore HEXDIG+ CRLF chunk-data CRLF. The chunk-data CRLF is part of this chunk, not a prefix of the next write.

last-chunk with an empty trailer is exactly 0 CRLF + (no field-lines) + CRLF → 0\r\n\r\n.

Older grammar, same bytes: RFC 7230 §4.1 (chunk-data CRLF, last-chunk, trailer-part CRLF).

What was on the wire

Before (deferred trailer / “CRLF before”):

write("foo")  →  CRLF  3  CRLF  foo
write("bar")  →  CRLF  3  CRLF  bar
end()         →  CRLF  0  CRLF  CRLF

A finished write(); write(); end() happened to look legal because the next call’s leading CRLF closed the previous chunk. The current chunk was never closed until the next write() or end().

beginWrite(); write("foo"); end() (issue 1939):

headers CRLF          ← beginWrite() (correct header terminator)
CRLF  3  CRLF  foo    ← write() still prefixes CRLF
CRLF  0  CRLF  CRLF

That extra CRLF is a chunk-size line whose first byte is CR (0x0D). Curl is correct to reject it.

Never-ending / SSE: beginWrite(); write(event N); and wait. Event N is size CRLF data with no chunk-data CRLF. A compliant HTTP/1.1 decoder must not deliver those bytes until the trailer arrives (RFC 9112 §7.1.1: chunk-data length is chunk-size, then CRLF). The SSE parser never sees a finished frame.

After:

beginWrite()     →  headers CRLF          // §2.1 empty line, once
write("foo")     →  3 CRLF foo CRLF       // complete chunk
write("bar")     →  3 CRLF bar CRLF
end()            →  0 CRLF CRLF           // last-chunk + empty trailer

write(); end() without beginWrite() is byte-identical to the old successful path. beginWrite(); write(); end() now matches that path (13 bytes of body for "foo", not 15).

Additional Context

Why the trailer is unconditional (backpressure)

AsyncSocket::write’s failed flag means backpressure, not “bytes were dropped”. Non-optional writes always accept the buffer: leftover bytes are appended and the call returns {length, true} (src/AsyncSocket.h, already-buffered path and short us_socket_write).

If writeChunk() skipped the trailer when failed was true, a 64 KiB write() that filled the socket would queue 10000 CRLF <data> with no CRLF, then the next write("foo") / end() would start a new chunk. A decoder reading size 0x10000 would then see 3 instead of CRLF.

The trailer Super::write("\r\n", 2) in that state also appends to the same per-socket buffer. write() still returns false so callers back off.

What about tests/ChunkedEncoding.cpp?

This file does not change src/ChunkedEncoding.h. It only fixes the encoder used by the existing parser brute-force test.

The fixture listed two concatenated chunked bodies, with "" meaning last-chunk. It encoded each empty element as:

hex(0) CRLF  +  data("")  +  CRLF  +  extra CRLF
= 0\r\n\r\n\r\n

That is last-chunk 0\r\n + empty trailer CRLF + a surplus CRLF.

RFC 9112 §7.1 last-chunk + empty trailer-section + final CRLF is 0\r\n\r\n. The parser (already, on master) finishes the first chunked-body there and correctly leaves the surplus CRLF.

Byte count on the old fixture (why CI printed remaining chunk:76):

Piece	Bytes
22 CRLF + 34 + CRLF	40
f CRLF + 15 + CRLF	20
0\r\n\r\n\r\n	7
first body	67
second body (same shape)	74
total	141

Parser consumes last-chunk + trailer = 5 bytes of the 0\r\n\r\n\r\n, not 7. Consumed = 40 + 20 + 5 = 65. Remaining = 141 − 65 = 76 = surplus CRLF + 74-byte second body.

The test then required leftover == 0 || == 74. That 74 assumed the parser would swallow the illegal extra CRLF. It must not: that CRLF is not part of the first chunked-body. After the first body, leftover 76 is the spec-correct remainder for the old fixture.

This PR encodes empty elements as 0\r\n\r\n only. First body becomes 65 bytes, second 72, leftover after the first parse is exactly the second body. consumeChunkEncoding now stops on state == 0 (one chunked-body finished) instead of a magic leftover length that encoded the surplus CRLF.

testWithoutTrailer() already emitted 0\r\n\r\n via the generic size CRLF data CRLF path for empty data; it is unchanged.