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

Require state/nonce to only contain URL safe characters #126

Merged
merged 2 commits into from
Mar 21, 2024

Conversation

jogu
Copy link
Collaborator

@jogu jogu commented Mar 5, 2024

As discussed in #95 implementations often make mistakes handling characters that require escaping, character encoding conversion, etc. There is no advantage to using characters outside of the standard URL safe characters, and most people using base64url, UUIDs, JWTs, etc that are by default URL safe, so just make that restriction clear.

closes #95

As discussed in #95 implementations
often make mistakes handling characters that require escaping, character
encoding conversion, etc. There is no advantage to using characters outside
of the standard URL safe characters, and most people using base64url,
UUIDs, JWTs, etc that are by default URL safe, so just make that
restriction clear.

closes #95
Copy link
Collaborator

@Sakurann Sakurann left a comment

Choose a reason for hiding this comment

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

minor nit - thank you for the PR!

@jogu jogu requested review from bc-pi, paulbastian and awoie March 7, 2024 14:38
Copy link
Contributor

@awoie awoie left a comment

Choose a reason for hiding this comment

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

approving again

@jogu
Copy link
Collaborator Author

jogu commented Mar 21, 2024

Open for a few weeks, quite a few approvals and mentioned on a few working group calls without any objections been raised - merging.

@jogu jogu merged commit 0d5a0ae into main Mar 21, 2024
2 checks passed
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.

non-ASCII/non-url safe characters characters in nonce/state/etc
6 participants