The description for the output of Crypt::randomBytesBase64() says it will be up to 4 * $count, but that is not true.

Using the Base64 encoding the output of $count bytes is 4/3 * $count bytes.

Instead of describing the maximum output length, which is not specific for the Drupal implementation, the description for the output should be similar to the one used for Crypt::hashBase64(), for example.

A base-64 encoded string, with + replaced with -, / with _ and any = padding characters removed.

Comments

kiamlaluno created an issue. See original summary.

avpaderno’s picture

Status: Active » Needs review
StatusFileSize
new738 bytes
ilgnerfagundes’s picture

Assigned: Unassigned » ilgnerfagundes
Status: Needs review » Reviewed & tested by the community

Looks good, thanks for your work.

Kind regards, Ilgner

avpaderno’s picture

Assigned: ilgnerfagundes » Unassigned
alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Yeah that documentation is wrong. The new docs are more helpful. Backported to 8.8.x as this is a docs only fix.

Committed d2d8fc9 and pushed to 9.0.x.
Committed 192a49e and pushed to 8.9.x.
Committed 341d1c5ac8 and pushed to 8.8.x. Thanks!

  • alexpott committed 192a49e on 8.9.x
    Issue #3118581 by kiamlaluno: The documentation for Crypt::...

  • alexpott committed d2d8fc9 on 9.0.x
    Issue #3118581 by kiamlaluno: The documentation for Crypt::...

  • alexpott committed 341d1c5 on 8.8.x
    Issue #3118581 by kiamlaluno: The documentation for Crypt::...
ilgnerfagundes’s picture

@alexpott can you give me credit for testing?

alexpott’s picture

Hi @ilgnerfagundes thank you for reviewing the patch. Assigning credit is one of the hardest parts of being a committer. When you do post a review, be sure to describe what you reviewed and how. This helps other reviewers understand why you considered the issue RTBC (and is considered for issue credit). For a simple change like this it's hard to credit an RTBC - but you could have detailed why the new documentation is sufficient and removing details about the length of the return value is okay. That's what I what I had to check when I reviewed the patch prior to committing it.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

quietone credited tetranz.

quietone’s picture