Repository navigation
Stream large multipart uploads through a MultipartReader endpoint parameter - #4674
Merged
jeremydmiller merged 1 commit intoSep 29, 2026
Merged
Conversation
…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.
This was referenced Sep 29, 2026
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>
This was referenced Sep 29, 2026
This was referenced Sep 29, 2026
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
This was referenced Oct 5, 2026
This was referenced Oct 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Wolverine.Http has no way to stream a large upload.
IFormFile,IFormFileCollection,IFormCollectionand[FromForm]all go throughRequest.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 aMultipartReader. You can do that today with a rawHttpRequestparameter, but then every endpoint reimplements the content-type check, boundary parsing and limits.This PR lets an endpoint take a
MultipartReader:What the parameter does:
Request.Form. The reader wraps the raw request body.Request.Form. It reads the registeredFormOptionsand applies the endpoint'sIFormOptionsMetadata([RequestFormLimits]) on top, followingFormFeature: boundary length, header count/length, andMultipartBodyLengthLimitas the per-sectionBodyLengthLimit.ProblemDetails.multipart/form-datainAccepts. It counts as a form endpoint forAutoAntiforgeryOnFormEndpoints(), since the CSRF exposure is the same.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 aMultipartReadergets the same instance.[DisableRequestSizeLimit]/[RequestSizeLimit]already reach the endpoint metadata, and ASP.NET Core'sEndpointRoutingMiddlewareapplies 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
IFormFileframes 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_uploadscovers:IFormFeatureis still null after reading. I checked that this probe fails against anIFormCollectionendpoint.[RequestFormLimits]reaches the readerThe full
Wolverine.Http.Testssuite passes locally: 1084 passed, 10 skipped.