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

Secure Source of Randomness #11301

Closed
wants to merge 3 commits into from
Closed

Secure Source of Randomness #11301

wants to merge 3 commits into from

Conversation

gjskha
Copy link

@gjskha gjskha commented Mar 16, 2024

Replaces the random.random() function with the more secure function secrets.SystemRandom().random()

@dolfinus
Copy link
Contributor

Why? This is a part of Github automatization related to translations, not a part of FastAPI source code

@Zaczero
Copy link

Zaczero commented Mar 19, 2024

Why? This is a part of Github automatization related to translations, not a part of FastAPI source code

Also I think in this case, faster random is a slightly better choice. There seems to be nothing cryptographic about this sleep_time.

@codespearhead
Copy link

I agree with the above comment.

That bot couldn't pick up on the context.

This PR should be closed in my opinion.

@tiangolo
Copy link
Owner

tiangolo commented Apr 2, 2024

Thanks for the interest @gjskha, but yeah, as others point out, this is not needed here. So I'll close this one. But thanks for the interest! ☕

@tiangolo tiangolo closed this Apr 2, 2024
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 this pull request may close these issues.

None yet

5 participants