Skip to content

webapi: add Request#formData and Response#formData#2969

Open
nikneym wants to merge 12 commits into
mainfrom
nikneym/req-res-form-data
Open

webapi: add Request#formData and Response#formData#2969
nikneym wants to merge 12 commits into
mainfrom
nikneym/req-res-form-data

Conversation

@nikneym

@nikneym nikneym commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

This PR adds Request#formData and Response#formData methods. also introduces simd.zig for reusable HTTP-related parsers, idea is to move away from one-off parsers created at each file to more generalized ones here.

nikneym added 12 commits July 17, 2026 14:40
Introduces another url decoder also, we may like to unite these at some point.
Idea is to have reusable parsing utilities here; not sure on the name, we can surely go for something else.
Adds `initFromUrlEncoded` and `initFromMultipart`; which are required for `Request#formData`.
Allows parsing `Content-Type` header value with ease.
@nikneym
nikneym force-pushed the nikneym/req-res-form-data branch from 81b4f15 to 3f09e00 Compare July 17, 2026 11:40
@nikneym
nikneym marked this pull request as ready for review July 17, 2026 11:42
@nikneym
nikneym requested a review from karlseguin July 17, 2026 11:42
@karlseguin

karlseguin commented Jul 19, 2026

Copy link
Copy Markdown
Collaborator

I'm not smart enough to review this. Maybe @krichprollsch can take a look. Claude did find some issues though:

Confirmed bugs (verified by running the built binary)

  1. Bare Zig errors leak to JS as generic Error, sometimes synchronously.
    Request.formData does self._body orelse return error.InvalidFormData and both methods do
    ... orelse return error.InvalidFormData when Content-Type is absent
    (Request.zig:311,314, Response.zig:471). I probed this live:
  • new Response().formData() → synchronously throws Error: InvalidFormData. Spec requires
    promise-returning methods to always reject, and with TypeError. The sibling methods
    (json rejects via rejectPromise) get this right; formData is the odd one out.
  • Request with urlencoded Content-Type and no body → throws Error: InvalidFormData. Per
    Fetch, a null body is an empty byte sequence: this must resolve with an empty FormData
    (Chrome does).
  1. The parser rejects bodies its own encoder produces. header_value_map marks HTAB
    (0x09) invalid, and parseHttpHeader skips only ' ' as leading OWS — but
    writeMultipartName escapes only CR/LF/". Verified live: fd.append('a\tb', 'v') encodes
    fine, then req.formData() rejects with TypeError. Chrome round-trips this. RFC 9110
    explicitly permits HTAB in field values and as OWS, so this is wrong even for the
    "general HTTP parser" ambition.

  2. parseDisposition hard-fails on any unquoted parameter. Any key=value without quotes →
    error.InvalidFormData for the entire body. That includes filename*=UTF-8''... (RFC
    5987), which real servers emit despite RFC 7578 discouraging it — and which, ironically,
    is the exact example string used in simd.zig's own parseHttpHeader test. Chrome skips
    unknown/malformed params; this parser torpedoes the whole parse. Response.formData()
    parses arbitrary server output, so lenience matters here.

And claude's comment on the code:

  • src/simd.zig is named for its implementation technique, not its domain — and half of
    it (parseDisposition, ContentTypeIterator, HttpHeader) contains no SIMD at all. It's a
    MIME/HTTP-parsing module. The codebase already has src/browser/Mime.zig as the
    content-type home (to be fair: Mime.parse lowercases input and couldn't extract a
    case-sensitive boundary as-is, so ContentTypeIterator isn't pure duplication — but the
    codebase now has two content-type parsers in two unrelated locations).

  • The generality is speculative. The PR description says the idea is to unify "one-off
    parsers created at each file" — but no existing call-site was migrated; simd.zig's only
    consumer is this feature. parseHttpHeader's error.Incomplete streaming contract is
    designed for push-style incremental parsing that nothing uses (multipart always has the
    full buffer). Earlier commits on the branch even deleted a dead parseHttpHeaders
    function. This collides with the project's no-speculative-generality lean.

  • The SIMD is optimizing a cold path. These parsers run when a page calls formData() on
    an in-memory body — small part headers, parsed once. For that, the file maintains three
    parallel strategies (vectors, SWAR, scalar tables), with inconsistent choices between
    two nearly identical functions (matchHeaderKey: 16-byte vectors, "SWAR is not preferred
    here"; matchHeaderValue: 32-byte vectors plus a SWAR path). Meanwhile
    FormData.indexOfSpecial is behaviorally identical to its own fallback line —
    std.mem.indexOfAnyPos(u8, slice, 0, "%+") — the entire function could be that one call.

  • Micro-obscurities: broadcast derives bit width via @ctz(@as(T, 0)) instead of
    @bitSizeOf; block_size = @sizeof(usize) is commented "Block size of the CPU" (it's
    register width); u16-bitcast compares for two-byte prefixes where mem.eql reads clearer.

  • header_key_map is simultaneously stricter than needed (rejects nothing a browser
    emits) and laxer than RFC token (accepts ", ,, ;-adjacent chars, and all bytes ≥ 0x80 in
    header names) — asymmetric with the value map being too strict about tabs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants