Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throw if a request transform overrides the request HttpContent #2722

Merged
merged 1 commit into from
Feb 6, 2025

Conversation

MihaZupan
Copy link
Member

From #2721

Can I also ask you what kind of features will break if I replace the content?

If you look at https://github.com/microsoft/reverse-proxy/blob/main/src/ReverseProxy/Forwarder/HttpForwarder.cs you can search for requestContent.

A few examples:

  • Error handling will be wrong (reported ForwarderError, proxy's response status code)
  • ContentTransferring telemetry will not be emitted
  • We use state on the custom HttpContent to reliably synchronize the request with how the body is read. As a result, you could see thread-safety issues as your custom content logic may still be running (and using things like the HttpContext) when YARP already exits.
  • We won't signal to the client that we're aborting request reads
  • Possible further issues in the future as we're making assumptions that the content wasn't changed

It's possible some scenarios happened to work well enough that someone is using this. We could consider logging an error instead of throwing, but it's something that users should not do either way.

@MihaZupan MihaZupan added this to the YARP 2.3 milestone Jan 22, 2025
@MihaZupan MihaZupan closed this Jan 30, 2025
@MihaZupan MihaZupan reopened this Jan 30, 2025
@MihaZupan MihaZupan merged commit 8cbd078 into dotnet:main Feb 6, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants