Repository navigation
NGRequest: parse the body lazily (raw body + on-demand form values), not eagerly at the adaptor #66
Description
Summary
Make NGRequest lazy about interpreting the request body: stop draining the content stream and parsing form values eagerly at construction (in the adaptor), and instead expose the raw body, parsing form values on demand — and only when the content type actually says the body is a form.
This consolidates several existing FIXMEs in NGStandardRequest that are all facets of the same design smell, and removes a real footgun the dev /eval endpoint just had to work around.
The problem, concretely
Today the adaptor reads form values off the body first, then hands an already-drained stream to NGStandardRequest, which copies whatever's left into its content buffer:
NGAdaptorJetty.requestToNGRequest—parametersFromRequest(...)runs, thenRequest.asInputStream(...)(which is now spent for a form-encoded POST). Same in the servlet adaptor.NGStandardRequestconstructor —contentStream.transferTo(_contentByteStream), with the FIXME already on it:// FIXME: We're consuming the entire content stream at construction time for now. Eventually, whether to do this should be the consumer's decision // Hugi 2026-05-12
Consequence: for an application/x-www-form-urlencoded POST, contentString() returns empty — the raw body is gone, shredded into the form map (split on =/&). A handler that wanted the raw body has already lost, before it ran. The /ng/dev/eval endpoint hit exactly this: real Java in a curl --data body (which defaults to form-encoding) is unrecoverable, so eval now has to detect the case and tell the caller to send text/plain instead.
This is not WO's behavior — WO keeps _contentData separate from parsed form values, so its contentString() survives form parsing. But WO does it by storing the body twice (raw copy + parsed values) on every request, which is a memory tax we don't want to copy. The goal here is neither "drain eagerly" (current) nor "keep two copies" (WO) but "decide lazily."
Proposed direction
Move the decision from the adaptor to NGRequest:
- The adaptor hands
NGRequestthe raw body (bytes/stream) and the headers — and does not pre-parse form values. formValues()/formValueForKey(...)parse the body on first call, and only if the content type isapplication/x-www-form-urlencoded(query-string params are always available; they don't touch the body).contentBytes()/contentString()return the raw body untouched.- Parse once, cache the result.
A handler that wants the raw body reads it; a handler that wants form values triggers the parse. The two never fight, and neither pays for the other. This keeps ng's "a body is consume-once" philosophy — we just stop making the irreversible choice at the door.
FIXMEs this would retire / address
All already in NGStandardRequest, all the same underlying complaint ("the adaptor shouldn't be pre-populating / pre-deciding this; the request should model it"):
- L76 — content stream consumed at construction; "should be the consumer's decision" ← the core of this issue
- L136 — "The Map should be populated by the request object, not the adaptor" (form values)
- L260 — "Cookie header deserialization should happen in NGRequest instead of in the adaptor" (cookies — same shape; worth folding in)
- L154 / L171 — "NGRequest wants to be immutable" — the
_setFormValues/_setCookieValuessetters exist only because the adaptor pushes these in; lazy parsing lets them go, moving NGRequest toward the immutability it says it wants.
Notes / scope
- Multipart (
multipart/form-data, file uploads) is a separate content type and is already handled specially in the adaptor; keep that path as-is for now (it has its own FIXME about final design) or fold it into the same lazy model — decide during implementation, don't let it block the urlencoded case. - Thread-safety: lazy parse needs to be safe for concurrent first-callers (the file already flags
formValueForKey/formValuesForKeyas needing thread-safety, L305/L322) — do the parse under a lock / compute-once. remoteAddress(added recently for the /eval loopback check, with its own FIXME) is the same category — an adaptor-populated setter on a request that wants to be immutable. Worth sweeping into the same redesign.- Both adaptors (
ng-adaptor-jetty,ng-adaptor-jetty-servlet) construct requests; both change.
Context
Surfaced while building the /ng/dev/eval dev endpoint (part of the tool/agent dev loop driven by the parslips-dev-loop skill). The eval endpoint is correct as shipped — it rejects a form-encoded body with a clear "send text/plain" error — but that workaround exists only because of the eager-parse behavior above; this issue removes the need for it and cleans up the request model generally. Filed at the maintainer's request while the analysis was fresh; NGRequest is acknowledged to be in flux, so this is direction, not a rush.
Activity
hugithordarson commentedon Aug 22, 2026
Addendum (2026-08-22): multipart belongs to this design, and the whole thing should be tackled as one piece.
Multipart is the third body kind this issue's model has to account for (raw, form-encoded, multipart) — and today it has the same disease in a worse form: the adaptor parses parts itself, bridges text parts into formValues via generated-UUID indirection, and populates the request's live mutable
_uploadedFiles()map post-construction (the "really, really temporary" FIXME in NGAdaptorJetty). On top of the eager-parse problem,UploadedFileis nested inNGStandardRequest, so theNGRequestinterface imports a type from its own implementation.Direction agreed in design discussion: multipart requests become a separate class —
NGMultipartRequest implements NGRequest, chosen by the adaptor on Content-Type. ItsformValues()derives from text parts, and a typeduploadedFiles()accessor (no underscore, keyed directly by part name — the UUID indirection dies) exists only on this class; baseNGRequestloses_uploadedFiles()entirely.UploadedFilepromotes to a top-level type.How it composes with the lazy model proposed here:
NGFileUploadpattern-matches on the request type, so the classic silent failure (form missingenctype="multipart/form-data"→ browser submits filenames as text → nothing happens) becomes a clear diagnostic instead.UploadedFilealready holdsInputStream+ length) are where upload size caps land — connects to the unbounded-body-buffering item on the security list.Sequencing: this issue + the multipart split + the NGMessage/request-cycle model completion are one work package (pre-migration "shape" work) — doing them separately would mean reshaping the same constructor and parsing seams twice.
Read the request's body on first access, not at construction (refs #66)
A parser for form-encoded content, following the WHATWG URL standard …
Requests parse their own form values on first use, leaving the body r…
hugithordarson commentedon Oct 25, 2026
Progress (2026-10-25). The urlencoded half of this has landed:
NGFormValues, a parser for form-encoded content following the WHATWG URL standard. It's lenient where Jetty threw: a stray%or invalid UTF-8 used to make the adaptor fail with a 500 (/?a=100%), and is now kept literally or becomes U+FFFD.application/x-www-form-urlencoded(honouring its charset). The raw body stays readable in either order. Neither adaptor pre-parses any more, and the/ng/dev/evaltext/plain workaround is gone. Limits match Jetty's defaults: 1000 fields, 200,000 bytes of form body.This also matters for running ng in front of WO in a Jetty handler chain. WO's Jetty adaptor reads the raw body stream, so ng must not read a body for a request it passes on. Before this, the adaptor's eager parameter parsing read every form body before dispatch. Now nothing in ng reads the body until a handler asks for it.
Remaining:
NGMultipartRequest, as in the addendum above; it must be lazy too, or uploads aimed at WO behind ng still lose their body.IllegalArgumentException, which is a 500, as it was with Jetty. A proper 400/413 needs a way to map exceptions to statuses.Multipart requests are a request class of their own, reading their pa…
Multipart requests can hand over their parts as they arrive, for stre…
Multipart handling moves from the adaptors into ng: the adaptor provi…
Requests parse their own cookies from their Cookie headers; the adapt…
hugithordarson commentedon Oct 25, 2026
Everything this issue set out to do has landed. The adaptor now hands ng the method, URI, query string, headers and raw body (plus a multipart parser, see below), and the request interprets all of it itself, on demand.
/ng/dev/evaltext/plain workaround is gone.NGMultipartRequest, reading their parts on first use._uploadedFiles()and the UUID indirection are gone;NGFileUploadexplains a form missingenctype="multipart/form-data".incomingParts()hands a handler the parts as they arrive, for streaming large uploads.NGFileUploadgained aninputStreambinding.NGMultipartParser; Jetty's isNGIncomingParts, built on Jetty's own parser). ng chooses the request class (NGRequests.of), collects parts (memory up to a threshold, then temporary files), enforces limits fromng.multipart.*properties, and deletes temporary files when the adaptor calls_close(). This removed Jetty's hidden 10 MB per-file cap. The servlet adaptor shares the parser, so it streams too.Since nothing in ng reads the body before a handler asks for it, ng can sit in front of WO in a Jetty handler chain without consuming the body of a request it passes on.
ng-testapp demonstrates each way of receiving an upload (f04968e).
Follow-ups filed separately: