Add support for wide characters in passwords. #2628
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The crypt function cannot handle most wide (Unicode) characters, but does handle some by attempting to downgrade to an 8-bit string. In order to support wide characters in passwords while not breaking any passwords which worked before this change, a password will be encoded into UTF-8 before being sent to crypt only when crypt dies on the original string.
drgrice1
reviewed
Nov 22, 2024
There was a problem hiding this comment.
This should use the encode method of Mojo::Util (which is really just the Encode::encode method), rather than the Encode::utf8_encode method. The Encode::utf8_encode method should not be used anymore.
Also, the process that you have duplicated in three places should be made into a utility method in lib/WeBWorK/Utils.pm, and just called in all three places. The crypt method can just be replaced with this utility method making much cleaner code.
I have put in a pull request to your branch making these changes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is an attempt to address #1141 about problems with wide characters in passwords.
Login attempts where the password field contained most wide Unicode characters would trigger an error page and not just a failed login attempt. So would pages where an attempt was made to set a password with such a character. The error pages was triggered as
cryptwould die on most wide Unicode characters. However, there were apparently some which would work as a result of special handling in crypt for wide characters which could be downgraded to 8-bit characters.For example the character ג in passwords would trigger the error.
In order to support wide characters in passwords while not breaking any passwords which worked before this change, a password will be encoded into UTF-8 before being sent to crypt only when crypt dies on the original string.