test: add multipart/form-data rerouting regression coverage for #714 - #2421
test: add multipart/form-data rerouting regression coverage for #714#2421rodri-oliveira-dev wants to merge 11 commits into
Conversation
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. |

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