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

authz_fix: keep the previous behaviour of adding encoding header #3805

Merged
merged 2 commits into from
Jul 10, 2018

Conversation

gsagula
Copy link
Member

@gsagula gsagula commented Jul 9, 2018

Signed-off-by: Gabriel gsagula@gmail.com

Description: This PR brings back the previous behaviour of adding/modifying the encoding headers. Before #3759, headers sent from the authorization server to the downstream client needed to be added via setReferenceKey() method which behaves differently than the current implementation which uses addCopy(). To mimic the previous behaviour, the header needs to be removed before adding the new one.

Risk Level: low
Testing: unit and manual testing

Signed-off-by: Gabriel <gsagula@gmail.com>
@gsagula
Copy link
Member Author

gsagula commented Jul 9, 2018

@mccv @mattklein123 I found that the previous bug fix changed the original behaviour of the filter. I open this PR to bring it back. Please feel free to take a look whenever you have a chance and let me know if it doesn't make sense. Thanks!

@htuch
Copy link
Member

htuch commented Jul 9, 2018

@gsagula can you add a test for this behavior?

@gsagula
Copy link
Member Author

gsagula commented Jul 9, 2018

@htuch For sure. I'll be on the road for the most of the day, but I can try to do that a bit later this evening or tomorrow. Thanks!

Signed-off-by: Gabriel <gsagula@gmail.com>
@gsagula
Copy link
Member Author

gsagula commented Jul 10, 2018

@htuch I've added the test. Please let me know if there is anything else that I should change. Thanks!

Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

Thanks!

@htuch htuch merged commit 80be024 into envoyproxy:master Jul 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants