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
feat: use SecureRandom instead of Random to reduce the chance of auto-id collisions #156
Conversation
Codecov Report
@@ Coverage Diff @@
## master #156 +/- ##
=========================================
Coverage 71.62% 71.62%
Complexity 969 969
=========================================
Files 62 62
Lines 5222 5222
Branches 579 579
=========================================
Hits 3740 3740
Misses 1302 1302
Partials 180 180
Continue to review full report at Codecov.
|
Do we know if users have ran into any issues with this sort of switch due to SecureRandom blocking to return values until there is enough entropy?
[1] https://docs.oracle.com/javase/8/docs/api/java/security/SecureRandom.html |
@BenWhitehead I do not know if this would be an issue. I'll look into it though. I could see it potentially being an issue with the Android SDK (where the change to SecureRandom has already occurred), where blocking the main thread is strongly discouraged; however, for a server-side SDK I wouldn't anticipate it being an issue. |
The main location I'd worry about it in the server side SDK would be in some of our "lean" environments, i.e. Cloud Functions, App Engine etc |
I've sent a question to several folks on my team to see if they've ran into anything around entropy in those environments and will report back if I hear anything. |
@BenWhitehead: @wilhuff pointed out that SecureRandom will not block in the way that it is used in this PR: https://tersesystems.com/blog/2015/12/17/the-right-way-to-use-securerandom/. At least, it won't block on Mac or Linux. In Windows, it's unclear if it will block or not. But do those "lean environments" you referred to include Windows machines? |
I believe all the lean environments are linux/container based so |
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #155 ☕️