test: add multipart/form-data rerouting regression coverage for #714 - #2421
rodri-oliveira-dev wants to merge 15 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2421 +/- ##
===========================================
+ Coverage 95.55% 95.69% +0.14%
===========================================
Files 264 264
Lines 6970 6970
Branches 761 761
===========================================
+ Hits 6660 6670 +10
+ Misses 201 193 -8
+ Partials 109 107 -2
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Production impact
No production files under
src/are changed.The current implementation already supports the regular
multipart/form-datarerouting scenario, so this PR only adds regression coverage to prevent that behavior from being unintentionally broken in the future.Refs #714
I'm going to change the requirements. I can't approve this without real improvements.
We need to test it using a sample or via a browser test with an actual HTML page.
Agreed. This could become a future feature proposal. I'll develop the concept further and probably create an official feature issue during the current release. In cooperation with you or without. |
|
Thanks, @raman-m. I added the requested real sample in It now includes an actual HTML page served by the gateway with I kept the automated regression coverage as well: the acceptance test validates the forwarded form field, file field name, filename, file content type, exact file contents, downstream path, and multipart boundary; the unit tests cover I also reviewed the Validation passed locally for restore/build, sample build, formatting checks, issue #714 unit/acceptance tests on net8/net10, full unit suite on net8/net10 with invariant culture, and full net10 acceptance suite when excluding only the unrelated HTTP/2 WebSocket direct-connection timeout. |
Closes #714
Summary
Adds regression coverage and a real browser sample for #714 against the current
developbranch.The original issue reported that
multipart/form-datarequests containing a file returned 404 instead of being rerouted. After validating the current request pipeline, the original issue is no longer reproducible for regular, non-aggregated routes: Ocelot streams the raw request body and preserves the originalContent-Type, including the multipart boundary.No production-code change under
src/is required for the regular rerouting scenario. This PR documents and protects that behavior.Coverage added
POST /uploadto/api/filesusingmultipart/form-dataContent-Typeand boundaryRequestMapperconfirming that the multipart body andContent-Typeboundary are preserved unchangedRequest.FormReal sample
Adds
samples/MultipartFormDatawith two small apps:ApiGateway: serves a valid HTML page containing a realenctype="multipart/form-data"form with a text field and file input, then forwardsPOST /uploadthrough OcelotDownstreamService: handlesPOST /api/files, reads the request usingReadFormAsync, and returns the received form field, file metadata, file contents, and request content typeThe sample README includes the run instructions and uses only the existing ASP.NET Core/Ocelot project setup.
Review follow-up
Form = from.Formidea from Add 'multipart/form-data' support for file uploads #1372. It was not technically necessary for this PR: the regular route works through the existing raw-body streaming path, and the multiplexer clone can read multipart form data from the cloned body plus copied content headers. Therefore this PR intentionally does not addForm = from.Form.Validation
Validated locally with .NET SDK
10.0.400:dotnet restore Ocelot.slnxpasseddotnet restore Ocelot.Samples.slnxpassed, with existing ServiceFabric/Steeltoe dependency warnings outside this PRdotnet build Ocelot.slnx --no-restorepassed with 0 warnings/errorsdotnet build Ocelot.Samples.slnx --no-restorepassed, with the same existing ServiceFabric/Steeltoe warningsdotnet format Ocelot.slnx --no-restore --verify-no-changes --include acceptance/Request/MultipartFormDataTests.cs unit/Multiplexing/MultiplexingMiddlewareTests.cs unit/Request/Mapper/RequestMapperTests.cspasseddotnet format Ocelot.Samples.slnx --no-restore --verify-no-changes --include samples/MultipartFormData/ApiGateway/Program.cs samples/MultipartFormData/DownstreamService/Program.cspassednet8.0andnet10.0net8.0andnet10.0net8.0andnet10.0with--culture invariant(1901passed per TFM)net10.0was run; it had one unrelatedClientWebSocketTests.Http20ClientWhenDirectConnectionThenShouldConnectHTTP/2 WebSocket timeout. Rerunning the suite excluding only that test passed (358passed,3skipped).GET http://localhost:5567/returned200, and a real multipartPOST /uploadthrough the gateway returned200with the expected text field, file metadata, exact file contents, and multipart boundary received downstream.Local note:
net9.0builds successfully, but the .NET 9 runtime is not installed in this environment, so local test execution was limited tonet8.0andnet10.0.Production impact
No production files under
src/are changed.The current implementation already supports the regular
multipart/form-datarerouting scenario, so this PR adds regression coverage and a real executable sample to prevent that behavior from being unintentionally broken in the future.Refs #714