test: add multipart/form-data rerouting regression coverage for #714 by rodri-oliveira-dev · Pull Request #2421 · ThreeMammals/Ocelot · GitHub
Skip to content

test: add multipart/form-data rerouting regression coverage for #714 - #2421

Open
rodri-oliveira-dev wants to merge 11 commits into
ThreeMammals:developfrom
rodri-oliveira-dev:test/issue-714-multipart-regression
Open

test: add multipart/form-data rerouting regression coverage for #714#2421
rodri-oliveira-dev wants to merge 11 commits into
ThreeMammals:developfrom
rodri-oliveira-dev:test/issue-714-multipart-regression

Conversation

@rodri-oliveira-dev

@rodri-oliveira-dev rodri-oliveira-dev commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Closes #714

Summary

Adds regression coverage and a real browser sample for #714 against the current develop branch.

The original issue reported that multipart/form-data requests 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 original Content-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

  • Acceptance coverage that reroutes POST /upload to /api/files using multipart/form-data
  • Verifies that the downstream route is reached successfully
  • Verifies the downstream path
  • Verifies the multipart Content-Type and boundary
  • Verifies a regular form field
  • Verifies the file field name, original filename, file content type, and exact file contents
  • Unit coverage for RequestMapper confirming that the multipart body and Content-Type boundary are preserved unchanged
  • Unit coverage for the multiplexer context clone confirming that a cloned multipart request body can still be read as form data without copying Request.Form

Real sample

Adds samples/MultipartFormData with two small apps:

  • ApiGateway: serves a valid HTML page containing a real enctype="multipart/form-data" form with a text field and file input, then forwards POST /upload through Ocelot
  • DownstreamService: handles POST /api/files, reads the request using ReadFormAsync, and returns the received form field, file metadata, file contents, and request content type

The sample README includes the run instructions and uses only the existing ASP.NET Core/Ocelot project setup.

Review follow-up

  • The one-test constants in the acceptance test were moved to local variables and passed into helpers where needed.
  • The requested real sample/browser form scenario was added with an actual HTML upload page.
  • I reviewed the Form = from.Form idea 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 add Form = from.Form.
  • POST aggregation/multiplexing support remains out of scope and should be treated as a future feature/design discussion.

Validation

Validated locally with .NET SDK 10.0.400:

  • dotnet restore Ocelot.slnx passed
  • dotnet restore Ocelot.Samples.slnx passed, with existing ServiceFabric/Steeltoe dependency warnings outside this PR
  • dotnet build Ocelot.slnx --no-restore passed with 0 warnings/errors
  • dotnet build Ocelot.Samples.slnx --no-restore passed, with the same existing ServiceFabric/Steeltoe warnings
  • dotnet format Ocelot.slnx --no-restore --verify-no-changes --include acceptance/Request/MultipartFormDataTests.cs unit/Multiplexing/MultiplexingMiddlewareTests.cs unit/Request/Mapper/RequestMapperTests.cs passed
  • dotnet format Ocelot.Samples.slnx --no-restore --verify-no-changes --include samples/MultipartFormData/ApiGateway/Program.cs samples/MultipartFormData/DownstreamService/Program.cs passed
  • Issue Multipart/form-data is not rerouted (error 404) #714 unit tests passed on net8.0 and net10.0
  • Issue Multipart/form-data is not rerouted (error 404) #714 acceptance test passed on net8.0 and net10.0
  • Full unit suite passed on net8.0 and net10.0 with --culture invariant (1901 passed per TFM)
  • Full acceptance suite on net10.0 was run; it had one unrelated ClientWebSocketTests.Http20ClientWhenDirectConnectionThenShouldConnect HTTP/2 WebSocket timeout. Rerunning the suite excluding only that test passed (358 passed, 3 skipped).
  • The new sample was run manually: GET http://localhost:5567/ returned 200, and a real multipart POST /upload through the gateway returned 200 with the expected text field, file metadata, exact file contents, and multipart boundary received downstream.

Local note: net9.0 builds successfully, but the .NET 9 runtime is not installed in this environment, so local test execution was limited to net8.0 and net10.0.

Production impact

No production files under src/ are changed.

The current implementation already supports the regular multipart/form-data rerouting 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

@codecov

codecov Bot commented Aug 31, 2026

Copy link
Copy Markdown

@coveralls

coveralls commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

Coverage Status

coverage: 97.231% (+0.1%) from 97.116% — rodri-oliveira-dev:test/issue-714-multipart-regression into ThreeMammals:develop

@rodri-oliveira-dev
rodri-oliveira-dev marked this pull request as ready for review August 31, 2026 17:54
@raman-m
raman-m self-requested a review September 2, 2026 10:29
@raman-m raman-m added Core Ocelot Core related or system upgrade (not a public feature) Autumn'26 Autumn 2026 release Requester Ocelot feature: Requester labels Sep 2, 2026
@raman-m raman-m added this to the Autumn'26 milestone Sep 2, 2026

@raman-m raman-m left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Production impact

No production files under src/ are changed.

The current implementation already supports the regular multipart/form-data rerouting 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.

Comment thread acceptance/Request/MultipartFormDataTests.cs Outdated
@raman-m

raman-m commented Sep 2, 2026

Copy link
Copy Markdown
Member

Scope

This PR intentionally covers the regular, non-aggregated routing scenario discussed in #714.

Aggregated routes using POST/multiplexing are intentionally out of scope.

As discussed in the issue, aggregation currently has additional constraints around POST requests and raises a broader design question about combining potentially heterogeneous downstream responses such as JSON, XML, HTML, headers/status-only responses, or other payload types.

That scenario should therefore be treated separately from the regular multipart rerouting behavior validated by this PR.

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.

@rodri-oliveira-dev

Copy link
Copy Markdown
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Autumn'26 Autumn 2026 release Core Ocelot Core related or system upgrade (not a public feature) Requester Ocelot feature: Requester

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Multipart/form-data is not rerouted (error 404)

3 participants