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

preauth - being able to receive base64-encoded headers #90

Merged
merged 1 commit into from
Dec 13, 2023

Conversation

pmauduit
Copy link
Member

@pmauduit pmauduit commented Dec 12, 2023

As Netty will always consider received headers as US-ASCII encoded strings, this can lead to issues when the values contain accented characters.

With this new feature, it is possible to consider the headers values as base64-encoded, which will be decoded before use.

Tests: IT added.

@pmauduit pmauduit requested a review from groldan December 12, 2023 18:10
@pmauduit pmauduit force-pushed the preauth-base64-decode-headers branch from bbe213c to f016c40 Compare December 12, 2023 20:57
@pmauduit
Copy link
Member Author

Updated / force-pushed to take into account @groldan 's feedback: instead of setting a new configuration property, we can assume that if the the header's value is prefixed with {base64} then the rest will be base64-encoded ; this also allows to reuse the existing code from georchestra-commons, which was already taking care of base64-decoding.

Also updated the doc / apache2 mod_mellon configuration example.

@pmauduit pmauduit marked this pull request as ready for review December 13, 2023 12:08
@pmauduit
Copy link
Member Author

@groldan ready for review

As Netty will always consider received headers as US-ASCII encoded,
this can lead to issues when the values contain accented characters.

With this new feature, it is possible to consider the values as
base64-encoded, which will be decoded before use.

Tests: IT added.
@groldan groldan force-pushed the preauth-base64-decode-headers branch from f016c40 to 8485235 Compare December 13, 2023 13:21
Copy link
Member

@groldan groldan left a comment

Choose a reason for hiding this comment

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

Made a small change to the test to make is more clear and remove the public modifiers. Looks good.

@pmauduit pmauduit merged commit c7c345b into main Dec 13, 2023
3 checks passed
@pmauduit pmauduit deleted the preauth-base64-decode-headers branch December 13, 2023 13:27
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