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

Added documentation describing the dklen is expected in bytes for the… #106624

Merged
merged 4 commits into from
May 22, 2024
Merged

Conversation

thiezn
Copy link
Contributor

@thiezn thiezn commented Jul 11, 2023

Update hashlib.pbkdf2_hmac docs that dklen is in bytes

The hashlib.pbkfdf2_hmac documentation states that you can pass on the length of the derived key. It is however ambiguous if it's meant to be passed on as bytes or bits.

The CPython implementation relies on the OpenSSL C library. Looking at the OpenSSL source code, the function is expected to be in bytes.

Although this might be obvious to most people, some other language libraries handle this in bits.

Ps. This is my first pull request to cPython. I hope I'm not missing anything?


📚 Documentation preview 📚: https://cpython-previews--106624.org.readthedocs.build/

@thiezn thiezn requested a review from tiran as a code owner July 11, 2023 13:08
@bedevere-bot bedevere-bot added awaiting review docs Documentation in the Doc dir skip news labels Jul 11, 2023
@cpython-cla-bot
Copy link

cpython-cla-bot bot commented Jul 11, 2023

All commit authors signed the Contributor License Agreement.
CLA signed

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

There is a similar case in the documentation for scrypt() few lines below. Please add "in bytes" there too.

@thiezn thiezn requested a review from gpshead as a code owner May 22, 2024 07:12
@thiezn
Copy link
Contributor Author

thiezn commented May 22, 2024

Updated dklen documentation for scrypt() to include 'in bytes'

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

LGTM.

@serhiy-storchaka serhiy-storchaka added needs backport to 3.12 bug and security fixes needs backport to 3.13 bugs and security fixes skip issue labels May 22, 2024
@serhiy-storchaka serhiy-storchaka merged commit 5adf78f into python:main May 22, 2024
30 checks passed
@miss-islington-app
Copy link

Thanks @thiezn for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 22, 2024
…thonGH-106624)

(cherry picked from commit 5adf78f)

Co-authored-by: Mathijs Mortimer <thiezn@users.noreply.github.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 22, 2024
…thonGH-106624)

(cherry picked from commit 5adf78f)

Co-authored-by: Mathijs Mortimer <thiezn@users.noreply.github.com>
@bedevere-app
Copy link

bedevere-app bot commented May 22, 2024

GH-119383 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label May 22, 2024
@bedevere-app
Copy link

bedevere-app bot commented May 22, 2024

GH-119384 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 bug and security fixes label May 22, 2024
serhiy-storchaka pushed a commit that referenced this pull request May 22, 2024
…ons (GH-106624) (GH-119384)

(cherry picked from commit 5adf78f)

Co-authored-by: Mathijs Mortimer <thiezn@users.noreply.github.com>
serhiy-storchaka pushed a commit that referenced this pull request May 22, 2024
…ons (GH-106624) (GH-119383)

(cherry picked from commit 5adf78f)

Co-authored-by: Mathijs Mortimer <thiezn@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip issue skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants