-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
#21022 Switch to using base64 encoding in wp_fast_hash()
so its return value remains below 60 characters in length
#8377
Conversation
…ue remains below 60 characters in length.
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN:
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change request inline to improve accuracy of the test for the issue identified.
…tivation_key` values.
Ok I've added new tests. This change isn't actaully sufficient, because the resulting |
@johnbillion This issue is part of WordPress 6.8 release and I wanted to check with you if we are on track for the same? |
@kjnanda Yes 21022 is on track for 6.8 and already has commits. This PR covers some follow-up adjustments. Thanks! |
…d by `wp_fast_hash()` has a length of no greater than 60 characters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me.
While manually testing, I changed the constant to include padding, ie SODIUM_BASE64_VARIANT_URLSAFE
, to make sure it was under the required length. It was.
I also confirmed that a hash length of 31 characters failed the new tests leaving 30 as the maximum possible length.
Trac ticket: https://core.trac.wordpress.org/ticket/21022