fix: reuse cached form in consumeForm (multipart + antiforgery)#154
Merged
pimbrouwers merged 2 commits intofalcoframework:masterfrom Apr 24, 2026
Merged
Conversation
…-read bug When antiforgery validation runs before Request.getForm (e.g. via getFormOptions / mapForm / validateCsrfToken), IAntiforgery.ValidateRequestAsync reads and buffers the form body via ReadFormAsync. For multipart/form-data POSTs, consumeForm would then branch into StreamFormAsync, which re-reads the (now-drained) body and fails with 'Unexpected end of Stream'. Detect that the form has already been parsed by checking the request's IFormFeature and reuse the cached ctx.Request.Form in that case, instead of attempting to re-read the stream.
Member
|
Amazing work. Thank you. |
Member
|
It looks like something in the integration tests is causing the build action to stall. Have a look and let me know what you think. |
Contributor
Author
|
taking another look myself 🤦 |
Top-level module statements (`let bldr = CreateBuilder()` / `let wapp = bldr.Build()` / `wapp.Run()`) compile into one-shot static initialization. When a second WebApplicationFactory (e.g. the derived factory in AntiforgeryMultipartTests via WithWebHostBuilder) re-invokes the entry point to spin up its own test host, DeferredHostBuilder.Build() runs against the already-built builder and throws "InvalidOperationException: Build can only be called once" — reliably on CI's parallel collections, intermittently elsewhere. Move construction into an explicit [<EntryPoint>] main so each factory invocation gets a fresh builder and WebApplication. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Member
|
Honestly, don't sweat it. Thanks for looking so quickly. I truly appreciate your effort. It's been a while since I had any technical discussions with anyone about this project, and I really enjoyed doing that with you. I hope we can continue working together when time permits! |
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
2 participants
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.
When antiforgery validation runs before
Request.getForm(the flowgetFormSecureuses internally),IAntiforgery.ValidateRequestAsyncreads and buffers the form body viaReadFormAsync. Formultipart/form-data,consumeFormwould then take theStreamFormAsyncbranch and re-read the now-drained stream, failing withIOException: Unexpected end of Stream. Non-multipart was fine becauseReadFormAsyncreturns the cachedHttpRequest.Form.Fix: detect the already-parsed form via
IFormFeature.Formand reuse it before deciding between streaming and non-streaming read.Regression test:
AntiforgeryMultipartTests.POST multipart/form-data with valid CSRF token succeeds via getForm— asserted to fail onupstream/masterwith the IOException, passes with the fix. A second guard test covers the urlencoded path so it can't regress in the other direction.