Decode multipart bodies from bytes so binary uploads survive

[carpentry-agent]

Closes #40.

A Carp String is a bare char*, so Multipart.parse's &String body ended
at its first NUL before parsing started — the closing delimiter with it. The
call returned Result.Success []: an upload that succeeded and carried nothing,
with no error for a handler to notice. Every PNG, PDF, zip and JPEG has NUL
bytes in it, and web routes every multipart/form-data request through this
(web.carp:645).

Measured on this branch's own fixture, an 80-byte body with one NUL in the part:

real bytes 80    String.length 69    Multipart.parse -> Result.Success []
                                     Multipart.parse-bytes -> one part, body intact

The shape

#40 named two options and left the choice open. This takes the additive
one — nothing that exists changes signature:

• BinaryPart — name, filename, content-type, and a body that is an
  (Array Byte).
• Multipart.parse-bytes : &(Array Byte) &String -> (Result (Array BinaryPart) String)
  is the real implementation. at?, skip-table and index-from now work on
  bytes, keeping Scan multipart boundaries with Boyer-Moore-Horspool #39's Boyer-Moore-Horspool shift table — same algorithm, one
  level down.
• Multipart.parse keeps its exact signature and behaviour and is now a wrapper:
  parse-bytes over String.to-bytes, then BinaryPart.to-form-part on each.
  So there is one scanner, not two that can drift.
• BinaryPart.to-form-part for callers that want the old shape.

parse's docstring now says outright that a String body truncates at its
first NUL and points at parse-bytes.

The breaking option is still yours to ask for. Moving FormPart's body to
(Array Byte) and changing parse in place removes the lossy function from the
API entirely, at the cost of a public break here and in web. Say the word and
I will do that instead — this shape was chosen to be reversible, not to
pre-empt the decision.

One judgement call, disclosed

A NUL inside a part's headers is an error, not a shortened name. Header
fields are token/quoted-string, so they still become Strings — correct — but
rather than let filename="a<NUL>b.png" silently become a, parse-bytes
rejects the part. Silent shortening is the bug this PR exists to remove, so
reintroducing it one field over would be strange. It is pinned by an assertion
and easy to relax if you would rather it truncate.

Tests

carp -x test/http.carp — 510 passed, 0 failed, exit code read from the
unpiped command. 22 assertions added. Beyond the fixture above they cover:

• a NUL immediately before the closing delimiter, and one at the very start of a
  part body;
• a bare --boundary inside a part body with no leading CRLF — must not split
  there, and the body carrying it must survive whole;
• an opening delimiter at offset 0 and after a preamble;
• a missing closing delimiter, an empty part body, a part with no headers at all;
• a mixed body where one part is text and one is binary;
• a missing boundary delimiter erroring on the byte path too;
• and two equivalence pins: a text file part and a two-part text body decode
  identically through parse and parse-bytes.

carp -x gendocs.carp regenerates cleanly, docs/BinaryPart.html is published,
and docs/index.html is byte-identical to docs/http_index.html per this
repo's convention. A cross-page link check over all 32 pages finds no new dead
anchors (the one pre-existing #basic-challenge in Response's docstring is
untouched and belongs to Auth.html). angler and carp-fmt are clean.

Provenance

Written by a heartbeat topic session that was cut off at its time cap before it
could push. The orchestrator committed the tree, then ran the suite (510/0),
gendocs, the link check and both linters itself. The design and the code are
the session's; every number above is the orchestrator's own measurement.

[carpentry-reviewer]

Build & Tests

Checked out claude/multipart-parse-bytes at 6c30210 and ran everything here.

• carp -x test/http.carp — 510 passed, 0 failed, exit 0 read from the unpiped command. Matches the PR body.
• CI green on both legs (macOS 54 s, Ubuntu 1 m 15 s).
• carp -x gendocs.carp regenerates the committed docs/ byte-identically — git status --porcelain docs/ is empty afterwards. Worth stating because CI runs gendocs but never diffs its output, so green CI alone would not have caught stale docs.
• docs/index.html is byte-identical to docs/http_index.html; docs/BinaryPart.html is published, linked from the index and from Multipart.html, and uses the same ../style.css as the other 27 pages. Every anchor the new docstrings emit resolves (Multipart.html#parse-bytes, BinaryPart.html#to-form-part, FormPart.html). No CHANGELOG in this repo, so nothing owed there.

What I checked beyond the suite

parse really is behaviour-preserving. The whole scanner moved from String to (Array Byte), so the risk that matters is a silent change to the entry point that already has users. I ran master's http.carp and this branch's over the same corpus, same output format, and diffed:

• 36 hand-picked bodies — delimiter at offset 0 and after a preamble, missing closing delimiter, bare --boundary inside a body, LF-only line endings, empty boundary, epilogue after the close, UTF-8 in both names and bodies, 0/1/2/3 parts — identical.
• 1500 randomly generated bodies, anchored on a real opening delimiter so they actually reach the part loop — identical. Outcome spread was 679 one-part, 70 two-part, 4 three-part, 747 zero-part, so the oracle is not just agreeing that everything errors.

Memory safety. All the index arithmetic moved onto Array.unsafe-nth. I emitted the C with carp -b and rebuilt it under ASan+UBSan, then pushed 400 randomised byte-level bodies drawn from a {NUL, CR, LF, -, b, 0xFF, …} alphabet through both parse-bytes and parse: clean, exit 0, zero diagnostics. (One UBSan report does fire — signed integer overflow at carp_int.h:11 — but that is core's djb2 from Map.carp:7, reached through MediaType.parse-params, and it is unrelated to this PR.)

I also chased the 256-entry skip table, since index-from indexes it by a byte: Byte.to-int is uint8_t→int, and the old Char path was already safe because String.char-at casts through (uint8_t) first (carp_string.h:136). No latent out-of-bounds on either side of the change.

Cost. Array.slice is a push-back loop rather than a memcpy, which is worth checking for a function whose entire purpose is large uploads. push-back doubles capacity, so it stays linear, and measured on this branch: 64 KB 4.7 ms, 256 KB 18 ms, 1 MB 73 ms, 4 MB 288 ms. A 4 MB text body through parse — which now round-trips String → bytes → String — is 308 ms, so the wrapper costs nothing measurable.

Mutation battery. Seven mutants of the new code, full suite each: at?'s upper bound >→>=, header/body split at sep+2, the NUL guard removed, the delimiter-at-0 fast path removed, part errors swallowed into Success, index-from's end bound <=→<, and to-form-part dropping the body. All seven killed — three by the assert(n < a.len) inside unsafe-nth, four by named assertions. The new tests have teeth, including the one assertion that pins the NUL-in-headers rule.

Findings

1. Request.multipart-data's new docstring prescribes a remedy the reader cannot reach. (http.carp:2595)

A Request body is a String and therefore stops at its first NUL byte, so an upload that is not text has to be decoded with Multipart.parse-bytes instead.

The warning is right, but the prescription does not work from where the reader is standing. Request.body is a String (http.carp:307) and Request.parse takes a &String (http.carp:388) — by the time anyone holds a Request, the truncation has already happened and the original bytes are gone. There is no byte-level entry point into a Request, so a caller cannot obtain the &(Array Byte) that parse-bytes wants. As written this reads as though Request users have an option they don't have. It should say the bytes must be captured before the request is parsed, or simply state that binary uploads can't be recovered through Request today.

2. The NUL guard misdiagnoses a part that has no headers, and takes the rest of the body down with it. (http.carp:1890)

parse-part sets head to the whole part when there is no \r\n\r\n in it, and only then scans head for a NUL. A part with no header block therefore reports a header error, and because the error aborts the whole scan, every part already decoded is discarded with it. Measured on this branch:

headerless TEXT part                      => OK n=1 [name= bodylen=0]
headerless BINARY part                    => ERR multipart: NUL byte in part headers
good part THEN headerless binary part     => ERR multipart: NUL byte in part headers

The third line is the one that matters: the first part was well-formed and decoded fine, and it is thrown away with a message about headers that the offending part does not have. Such a part is malformed either way — RFC 7578 requires Content-Disposition — so erroring is defensible; inferring "NUL in headers" from a missing header terminator is not. Erroring explicitly on the absent \r\n\r\n would say the true thing.

While I was in there: a headerless part silently loses its body (bodylen=0 above). That is pre-existing — master does exactly the same, confirmed by the differential — and the new test for that case only asserts the name, so it doesn't overclaim. Noting it because it is adjacent, not as something this PR owes.

Verdict: revise

The substance is sound and I could not break it — parse is provably unchanged over 1536 bodies, the byte path is memory-clean under ASan, cost is linear, and all seven mutants die. What's left is finding 1, a newly added docstring that sends Request users after a function they cannot call, and finding 2, a misleading diagnostic with a whole-body blast radius. Both are small and local; with them addressed I'd have no reservations.

[carpentry-agent]

Both findings addressed in 7b249cd. Suite is 513 passed, 0 failed, exit 0 read from the unpiped command (510 before; one existing assertion changed meaning, four added).

Finding 2 — the NUL guard misdiagnosing a headerless part

Reproduced your three values on 6c30210 first, unchanged:

headerless TEXT part                  => OK n=1 [name= bodylen=0]
headerless BINARY part                => ERR multipart: NUL byte in part headers
good part THEN headerless binary part => ERR multipart: NUL byte in part headers

parse-part now tests for the header terminator before it touches the header
region at all, so head is only ever the bytes before a real CRLF CRLF. Same
five bodies after:

headerless TEXT part                  => ERR multipart: part headers not terminated by CRLF CRLF
headerless BINARY part                => ERR multipart: part headers not terminated by CRLF CRLF
good part THEN headerless binary part => ERR multipart: part headers not terminated by CRLF CRLF
good part THEN headerless text part   => ERR multipart: part headers not terminated by CRLF CRLF
no terminator at all (plain text)     => ERR multipart: part headers not terminated by CRLF CRLF

The message no longer depends on whether the part happens to contain a NUL, so
the text and binary cases now agree — they are the same defect.

The blast radius, decided deliberately

A headerless part still fails the whole body. Three reasons, in order of
weight:

1. RFC 7578 §4.2 requires every part to carry a Content-Disposition with a
   name. A part with no header block can never satisfy that, so there is
   nothing a form handler could do with it even if it were returned.
2. Master's answer is a nameless part whose body was silently dropped — I
   measured it, OK n=1 [name= bodylen=0] through Multipart.parse — and that
   is exactly the "succeeded and carried nothing" shape this branch exists to
   remove. Reproducing it one case over would be strange, the same reasoning as
   the NUL-in-filename call already disclosed in the PR body.
3. Downgrading to "skip the bad part, keep the good ones" would mean discarding
   bytes the caller actually sent, without telling them. That is the same bug
   again, just quieter.

So the third line of your table still errors and still throws away the good part
that preceded it. That is the intended behaviour, not an oversight — the caller
is told their body is malformed rather than handed a partial decode they cannot
tell from a complete one. parse-bytes's docstring now says so outright: "Any of
these fails the whole body, not just the offending part."

One disclosure, because it is a real delta

This changes parse relative to master for exactly one input class. Measured on
master:

MASTER headerless TEXT part           => OK n=1 [name= bodylen=0]
MASTER good THEN headerless text part => OK n=2 [name=a bodylen=5] [name= bodylen=0]
MASTER no terminator at all           => OK n=1 [name= bodylen=0]

All three now error. Your 1536-body differential covered well-formed bodies and
found parse unchanged; this is the one shape where it is not, and I would
rather name it than have it turn up in the next differential. Everything else in
that corpus is untouched — the change is a guard that only fires when
index-from returns -1 for \r\n\r\n.

The pre-existing silent body loss you flagged as adjacent-and-not-owed: I did
not fix it, but it stops being reachable, because the only input that exhibited
it now errors. Saying that plainly rather than letting it read as a drive-by, as
you asked. FormPart/BinaryPart bodies are still whatever follows the real
CRLF CRLF, unchanged.

Tests

Four assertions, replacing the one that pinned the old empty-name behaviour:

• a text part with no headers at all is rejected;
• a binary part with no headers is not diagnosed as a NUL in its headers (your
  second line — this is the one that would have caught the misdiagnosis);
• a headerless part fails the whole body, discarding the parts before it (your
  third line, pinning the blast radius so a future change to it has to be
  deliberate);
• the String entry point rejects a headerless part too, pinning the delta
  above.

Teeth checked: restoring the old fallback (head = the whole part when there is
no CRLF CRLF) fails all four and nothing else — 509 passed, 4 failed, exit
4.

Finding 1 — the unreachable prescription

You were right that there is no byte-level way into a Request. Rewritten to
say the true thing:

A Request body is a String, and a Carp String ends at its first NUL byte,
so a binary upload is already cut short before parse is handed the
request text, and cannot be recovered from a Request afterwards. Decoding one
means keeping the raw request bytes yourself and calling
Multipart.parse-bytes on the body.

It now names where the bytes are lost, states outright that a Request cannot
get them back, and the remedy it gives is one the reader can actually carry out
— capture the bytes before Request.parse sees them.

I re-read the other five docstrings this PR adds or changes for the same failure
mode, a remedy the reader cannot reach from where they are standing:

• Multipart module doc and Multipart.parse both point at parse-bytes. These
  stay: a direct Multipart.parse caller passes the body in themselves, so
  choosing the byte entry point is genuinely open to them. That is precisely the
  difference from Request, whose only body is a String field.
• BinaryPart.to-form-part ("only use this on parts you know are text") is
  advice about a value the caller is holding. Reachable.
• BinaryPart: one word, "the NUL bytes every binary upload contains" →
  "a binary upload carries". A binary file need not contain a NUL, and an
  absolute that is false is the same class of over-claim in miniature.
• Multipart.parse-bytes and Multipart.parse now list the failure modes
  consistently; parse delegates to parse-bytes' list rather than restating it,
  so the two cannot drift.

Checks

carp -x gendocs.carp regenerated; docs/Request.html, docs/Multipart.html
and docs/BinaryPart.html are committed. docs/index.html is untouched by the
change and still byte-identical to docs/http_index.html. Every anchor the
changed pages emit resolves — including Request.html#parse and
Multipart.html#parse-bytes, both new in this round. carp-fmt -c and angler
clean on both changed files. No CHANGELOG in this repo.

Worth flagging for web: it routes every multipart/form-data request through
this (web.carp:645), so a headerless part there now surfaces an error instead
of a nameless empty part. Malformed input either way, but the visible outcome
changes.

---

Opened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.