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

V2.1.2 Verify that passwords 64 characters or longer are permitted but may be no longer than 128 characters. #886

Closed
Sjord opened this issue Dec 24, 2020 · 1 comment · Fixed by #891

Comments

@Sjord
Copy link
Contributor

Sjord commented Dec 24, 2020

V2.1.2 Verify that passwords 64 characters or longer are permitted but may be no longer than 128 characters.

This requirement could use improvement on the subject of English grammar.

Verify that passwords 64 characters or longer

This is missing an "of", so should be "passwords of 64 characters or longer".

but may be no longer than 128 characters.

This subsentence is missing a subject, and the previous subsentence doesn't have a clear subject, since it's imperative.

I suggest:

Verify that passwords of at least 64 characters are permitted, and that passwords of more than 128 characters are denied.

The upper limit was introduced in #756. @ThunderSon suggested:

V2.1.2 Verify that passwords allow for long passwords (e.g. 64 characters) and have an upper bound limit (e.g. 128 characters).

@tghosth suggests:

V2.1.2 Verify that passwords of up to 64 characters are allowed but no more than that to avoid resource overuse and other bugs.

Or:

V2.1.2 Verify that passwords of up to 64 characters are allowed but no more than that to avoid resource overuse and the bcrypt truncation bug.

The sentence we have now was introduced by @jmanico in 336c235.

I don't want to discuss the value of the upper bound, just improve this requirement's language.

@jmanico
Copy link
Member

jmanico commented Dec 24, 2020 via email

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 a pull request may close this issue.

2 participants