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 handler metadata headers #748

Merged
merged 3 commits into from
May 31, 2024
Merged

Restrict handler metadata headers #748

merged 3 commits into from
May 31, 2024

Conversation

emcfarlane
Copy link
Contributor

@emcfarlane emcfarlane commented May 28, 2024

This PR fixes the setting of protocol headers to avoid multiple value headers when providing metadata to a handler. The metadata headers are further restricted to avoid setting protocol headers like "Content-Type". This restriction allows the user to pass the response of a proxy call to a handler without having to filter the response headers themselves. This enforces the protocol headers are set by the handler and that they are unaffected from any user provided metadata.

Previously, returning the response of a client request to a handler would merge the headers together leading to protocol errors from invalid headers such as "Content-Type" having multiple values.

This PR fixes the setting of protocol headers to avoid multiple value
headers when providing metadata to a handler. The metadata headers are
further restricted to avoid setting protocol headers like "Content-Type".
This restriction allows the user to pass the response of a proxy call to a
handler without having to filter the response headers themselves. This
enforces the protocol headers are set by the handler and unaffected from
any user provided metadata.

Signed-off-by: Edward McFarlane <emcfarlane@buf.build>
connect_ext_test.go Outdated Show resolved Hide resolved
header.go Outdated Show resolved Hide resolved
handler.go Outdated Show resolved Hide resolved
Signed-off-by: Edward McFarlane <emcfarlane@buf.build>
Signed-off-by: Edward McFarlane <emcfarlane@buf.build>
@emcfarlane emcfarlane requested a review from jhump May 30, 2024 22:13
.golangci.yml Show resolved Hide resolved
header.go Show resolved Hide resolved
Copy link
Member

@jhump jhump left a comment

Choose a reason for hiding this comment

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

Couple of minor suggestions. Otherwise LGTM

@emcfarlane emcfarlane merged commit aba3ff5 into main May 31, 2024
12 checks passed
@emcfarlane emcfarlane deleted the ed/setProtocolHeaders branch May 31, 2024 20:51
@emcfarlane emcfarlane mentioned this pull request Jun 12, 2024
emcfarlane added a commit that referenced this pull request Jun 12, 2024
Upgrades the linter version to the latest fixing small issues. For
readability `nolint` global directives have been brought inline to avoid
misuse. This fixes the issue brought up in #748. Also bumps the
version of buf to the latest.

---------

Signed-off-by: Edward McFarlane <emcfarlane@buf.build>
@jhump jhump added the bug Something isn't working label Sep 19, 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.

2 participants