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

change java.util.Random to java.security.SecureRandom #1831

Merged
merged 4 commits into from
Dec 9, 2021

Conversation

mr-africa
Copy link
Contributor

📜 Description

change java.util.Random to java.security.SecureRandom for possible security reasons

💡 Motivation and Context

#1830

💚 How did you test it?

📝 Checklist

  • I reviewed the submitted code
  • I added tests to verify the changes
  • I updated the docs if needed
  • No breaking changes

🔮 Next steps

Copy link
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

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

It's available on Android since API 1 so from that perspective it would be fine.

@maciejwalkowiak @romtsn @marandaneto anything we should be aware before merging?

@maciejwalkowiak
Copy link
Contributor

Nothing to be scared of as far as I am aware in regards to running Sentry SDK on server 👍

@marandaneto
Copy link
Contributor

@mr-africa you dont need to add a new test, since there's already tests in place for that.

please run make format, add an entry to the https://github.com/getsentry/sentry-java/blob/main/CHANGELOG.md and commit :) thanks for doing this

CHANGELOG.md Outdated Show resolved Hide resolved
@marandaneto
Copy link
Contributor

@mr-africa test should work but you've to fix the import for the Random class in tests, See TracesSamplerTest

@codecov-commenter
Copy link

Codecov Report

Merging #1831 (44a69db) into main (9c2110f) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##               main    #1831   +/-   ##
=========================================
  Coverage     75.72%   75.72%           
  Complexity     2194     2194           
=========================================
  Files           218      218           
  Lines          7806     7806           
  Branches        828      828           
=========================================
  Hits           5911     5911           
  Misses         1493     1493           
  Partials        402      402           
Impacted Files Coverage Δ
sentry/src/main/java/io/sentry/SentryClient.java 85.98% <100.00%> (ø)
sentry/src/main/java/io/sentry/TracesSampler.java 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9c2110f...44a69db. Read the comment docs.

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.

5 participants