Skip to content

Stream large multipart uploads through a MultipartReader endpoint parameter - #4674

Merged
jeremydmiller merged 1 commit into
JasperFx:mainfrom
erdtsieck:streaming-multipart-uploads
Sep 29, 2026
Merged

jeremydmiller merged 1 commit into
JasperFx:mainfrom
erdtsieck:streaming-multipart-uploads

Conversation

@erdtsieck

Copy link
Copy Markdown
Contributor

Wolverine.Http has no way to stream a large upload. IFormFile, IFormFileCollection, IFormCollection and [FromForm] all go through Request.Form, which buffers the whole body before the endpoint runs (in memory up to 64 KB per file, on disk beyond that). ASP.NET Core's answer is streaming with a MultipartReader. You can do that today with a raw HttpRequest parameter, but then every endpoint reimplements the content-type check, boundary parsing and limits.

This PR lets an endpoint take a MultipartReader:

[DisableRequestSizeLimit]
[RequestFormLimits(MultipartBodyLengthLimit = long.MaxValue)]
[WolverinePost("/upload/stream")]
public static async Task<string> Upload(MultipartReader reader, CancellationToken cancellationToken)
{
    while (await reader.ReadNextSectionAsync(cancellationToken) is { } section) { ... }
}

What the parameter does:

  • Never touches Request.Form. The reader wraps the raw request body.
  • Same limits as Request.Form. It reads the registered FormOptions and applies the endpoint's IFormOptionsMetadata ([RequestFormLimits]) on top, following FormFeature: boundary length, header count/length, and MultipartBodyLengthLimit as the per-section BodyLengthLimit.
  • Error responses. A non-multipart request gets a 415, like the JSON body path. A missing or oversized boundary gets a 400 with ProblemDetails.
  • Metadata. The endpoint advertises multipart/form-data in Accepts. It counts as a form endpoint for AutoAntiforgeryOnFormEndpoints(), since the CSRF exposure is the same.
  • Startup failures. Combining the reader with IFormFile, [FromForm] or a JSON body fails at startup, because the body can only be read once. So does putting it on a GET or HEAD (the existing Guard against HTTP endpoints that advertise a request body they cannot receive (the #3630 failure class) #3648 guard). Middleware that also takes a MultipartReader gets the same instance.
  • Size limits need no new code. [DisableRequestSizeLimit] / [RequestSizeLimit] already reach the endpoint metadata, and ASP.NET Core's EndpointRoutingMiddleware applies them. The tests' log shows it trying ("This server does not support the IHttpMaxRequestBodySizeFeature" under TestServer).

Not covered: F# codegen for the new frame. The existing IFormFile frames don't have it either. OpenAPI shows no schema for the parts, because the endpoint decides what it reads.

Docs: a new Streaming Large Uploads section in guide/http/files.md, covering the limits and the antiforgery caveat. Send the token in a header: without one, ASP.NET Core's antiforgery middleware reads the form, which buffers the body you meant to stream.

Tests

streaming_multipart_uploads covers:

  • sections read in order, a 100 KB file included
  • the form is never buffered: IFormFeature is still null after reading. I checked that this probe fails against an IFormCollection endpoint.
  • [RequestFormLimits] reaches the reader
  • the 415 and 400 responses
  • the Accepts, size-limit and antiforgery metadata. The antiforgery test fails without the one-term change.
  • the startup guards

The full Wolverine.Http.Tests suite passes locally: 1084 passed, 10 skipped.

…ameter

IFormFile, IFormCollection and [FromForm] all go through Request.Form, which
buffers the whole body before the endpoint runs. A MultipartReader parameter
now binds a reader over the unbuffered body, with the limits Request.Form would
apply (FormOptions overridden by [RequestFormLimits]). 415 for a non-multipart
request, 400 ProblemDetails for a missing or oversized boundary, and a startup
failure when another binding would read the body too.
@jeremydmiller
jeremydmiller merged commit e654c69 into JasperFx:main Sep 29, 2026
83 of 84 checks passed
jeremydmiller added a commit that referenced this pull request Sep 29, 2026
…ADME (#4683)

The naming rule was written down in both README.md and CLAUDE.md and enforced by
nothing, so it held only as well as whoever was reading. It is now in
.editorconfig, where the IDE and `dotnet format` will flag a new violation as it
is typed.

Two things worth knowing about how it is set up:

.editorconfig was in .gitignore, in the block with .vscode/ and omnisharp.json
that comes from the stock Visual Studio template. That looks like inherited
boilerplate rather than a decision -- and an ignored .editorconfig cannot enforce
a convention for anybody else -- so the ignore is gone.

The rules deliberately do not run at build time. EnforceCodeStyleInBuild is off
and this repo sets TreatWarningsAsErrors, so promoting them would turn every
pre-existing violation into a build error: roughly 290 in Wolverine core alone,
mostly PascalCased protected members. The full solution still builds at 0
warnings, 0 errors. Those are not worth mass-fixing either, because renaming a
protected member breaks anyone subclassing it.

`private static readonly` is left explicitly unenforced. It is genuinely split in
this codebase, about 25 PascalCase against 15 underscore-prefixed, so there is no
convention there to encode and picking one would flag a pile of fields over a
choice nobody has made. Constants are Pascal cased, which the codebase is
unanimous about (19 private consts, no exceptions).

Verified the rules actually fire rather than assuming it. IDE1006 needs its own
dotnet_diagnostic severity; without that line every naming rule in the file is
silently inert, which a probe with a deliberate violation caught.

The README also needed a refresh:

- it told contributors to open `wolverine.sln`, which does not exist -- the repo
  moved to .slnx. That was the first instruction a new contributor followed;
- it described `main` as 6.0.0-alpha pre-release packages built against
  JasperFx 2.0-alpha, and the 5.0 branch as shipping "until 6.0 is generally
  available". 6.x has been GA for a while and is on 6.41.0;
- the naming section now says the part that actually catches people out -- that
  casing follows accessibility, not member kind -- with an example showing one
  class using both;
- added the "don't edit CHANGELOG.md" rule. It lived only in CLAUDE.md, which is
  AI-facing, so a human contributor had no way to find it. PR #4674 added an
  entry in good faith for exactly that reason.


Claude-Session: https://claude.ai/code/session_01VDUrBeB4tTnKj4AExCS1nj

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
BlackChepo pushed a commit to BlackChepo/wolverine that referenced this pull request Oct 5, 2026
Follow-up to JasperFx#4674. ReadMultipartBody overrode GenerateCode only, so an F#
endpoint taking a MultipartReader fell through to Frame's default-throwing seam:
"F# code generation is not supported for the frame 'ReadMultipartBody'".

The contributor disclosed this and was right about the precedent -- FromFileStrategy's
frames have no emitter either, so it is not a regression from that PR. But the
frame is otherwise a faithful copy of the C# JSON shape, and JsonHandling DOES
have an emitter, so the asymmetry is worth closing.

The one non-obvious part is the qualification. ReadJsonAsync is an inherited
INSTANCE method, so the F# emitter writes `this.ReadJsonAsync` (JasperFx#393).
ReadMultipartAsync is static, so it is qualified by TYPE instead --
`Wolverine.Http.HttpHandler.ReadMultipartAsync(...)` -- exactly as
WriteStringFrame already does for the static WriteString. The C# emit can call
either one bare only because the generated handler derives from HttpHandler;
F# resolves no such thing and a bare call is an FS0039.

Everything else follows JsonHandling: the return is a struct tuple, so F# needs
`let! struct (a, b) =`, and since F# has no early `return` the abort guard
renders the rest of the chain in its `else` via FSharpEmitHelpers.WriteAbortGuard.

Proven by compilation, not by string matching: a MultipartReader endpoint joins
Wolverine.Http.FSharpContracts and its chain joins the codegen sample, so the
existing compile gate regenerates Generated.fs and runs dotnet build over it.
Reverting only the emitter fails 5 of the 6 tests with the NotSupportedException
above, which also proves the new endpoint actually reaches this frame.

Does not change the fsharp-coverage total's story -- see JasperFx#4681, where the
ratchet that would have caught this frame arriving unemitted lives.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VDUrBeB4tTnKj4AExCS1nj
BlackChepo pushed a commit to BlackChepo/wolverine that referenced this pull request Oct 5, 2026
…eline

The fsharp-coverage command has always given an exact answer in seconds, and
nothing ran it. So the remaining count drifted upward on its own: JasperFxGH-2969 was
closed with 97 of 160 frames still inheriting the default-throwing
Frame.GenerateFSharpCode seam, FromFileStrategy's frames arrived without an
emitter, JasperFx#4674 added ReadMultipartBody in the same state, and re-running the
command while writing this showed 98. None of that was anyone's mistake --
there was no gate, no test, and no review checklist item.

This is the gate. It does not close the gap; it stops the number growing.

Two pieces:

1. TallyFSharpCoverage is carved out of RunFSharpCoverage, which computed the
   three buckets and then threw them away into AnsiConsole. Same extraction, and
   the same reason, as FindCandidateHandlerTypes in this file. The existing
   fsharp_coverage_includes_http_frames test could only assert that the command
   returned true, which is true regardless of the count -- proof the seam was
   unusable as a gate.

2. The tally takes an explicit assembly set. This is not gold-plating. The CLI
   scans AppDomain.CurrentDomain.GetAssemblies(), which is right for "whatever
   this host references" and useless for a gate, because the answer depends on
   what the JIT has loaded when it asks. The first draft forced the load with
   `_ = typeof(HttpChain)`, the JIT elided it as having no effect, and the
   baseline came back 25 instead of 54 -- a green gate over less than half the
   surface it claimed to cover.

The baseline is a SET of frame names, not a count. A count can be satisfied by
deleting one frame while adding another, and tells a contributor nothing about
which frame changed. The failure message names the frames in both directions:
what arrived unemitted, and what now emits and should be struck from the list.

That paid for itself immediately -- the_baseline_only_lists_frames_that_still_exist
caught three names written from the issue's summary table (AsParametersBindingFrame,
AssignPropertyFrame, FormBindingFrame) that are actually in the GLOBAL namespace,
not Wolverine.Http.CodeGen.

Negative control: removing one name from the baseline fails the gate naming
exactly that frame, which is the scenario it exists for.

Scope is Wolverine + Wolverine.Http. Per JasperFx#4681's second suggestion, declaring
support per surface is likely more honest than one global total; Marten and
EF Core would each want their own gate in their own test project.

Note DiagnosticsTests (src/Samples) has 5 failures in finding_message_types --
identical on clean origin/main, so pre-existing and unrelated.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VDUrBeB4tTnKj4AExCS1nj
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