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

Restrict metadata headers in error propagation #711

Merged
merged 10 commits into from
Mar 19, 2024

Conversation

emcfarlane
Copy link
Contributor

@emcfarlane emcfarlane commented Mar 14, 2024

This PR addresses issues when propagating errors from a client back to a handler. On the client side connect errors will contain all response headers: transport (Content-Type, Content-Length, etc), protocol and application headers. These could break the transport when trying to re-encode the error or leak sensitive information between services. For any wire errors (errors decoded from a client response) we now disable meta propagation. For other errors we now also restrict the headers propagated.

This PR addresses issues when propagating errors from a client back to
a handler. On the client side connect errors will contain transport
based metadata headers like `Content-Type`, `Content-Length`, etc. These
will break when trying to re-encoding the error. Currently
`Content-Type` is always overridden. We now expand this behaviour to
include all headers utilised by connect, gRPC or gRPCWeb.
@emcfarlane emcfarlane changed the title Restrict metadata headers from errors Restrict metadata headers in error propagation Mar 15, 2024
@akshayjshah
Copy link
Member

Haven't looked at the logic in depth, but we should explain this behavior in the docs (IsWireError, ConnectError.Meta, or both).

error.go Outdated Show resolved Hide resolved
@emcfarlane emcfarlane merged commit 7b3b344 into main Mar 19, 2024
12 checks passed
@emcfarlane emcfarlane deleted the ed/mergeMetadataHeaders branch March 19, 2024 19:25
@jhump jhump added the bug Something isn't working label Mar 20, 2024
@jhump jhump mentioned this pull request Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants