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.

Sign up for GitHub

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

Jump to bottom

SecureRandom.random_number doesn't accept a range #4607

Closed
DeeJayhX opened this issue May 16, 2017 · 6 comments · Fixed by #4656
Closed

SecureRandom.random_number doesn't accept a range #4607

DeeJayhX opened this issue May 16, 2017 · 6 comments · Fixed by #4656
Assignees
@kares
Milestone
JRuby 9.1.11.0

Comments

@DeeJayhX
Copy link

The problem is here:
https://github.com/jruby/jruby/blob/master/lib/ruby/stdlib/securerandom.rb#L147

At some point, the code to handle ranges was removed.

Environment

jruby 9.1.8.0 (2.3.1) 2017-03-06 90fc7ab Java HotSpot(TM) 64-Bit Server VM 25.121-b13 on 1.8.0_121-b13 +jit [mswin32-x86_64]

Windows 10 Professional x64

Expected Behavior

In standard Ruby I can do puts SecureRandom.random_number(50..55) and get a random result between 50 and 55.

Actual Behavior

If I do this in JRuby, I get an argument error for using a Range in place of a Fixnum.
@enebo
Copy link
Member

enebo commented May 16, 2017

@Ruined1 it does indeed work but our securerandom.rb is identical in the sense that we both have a 0 < n check at the top. You would think it also would blow up? So something else is replacing this definition somehow (I did verify this by putting a puts in securerandom.rb version and nothing prints out). The only random_number I can see in the C source is for Random itself....I am at a loss at the moment.

@enebo
Copy link
Member

enebo commented May 16, 2017

Oh haha...ok so it must be in C now and I must have missed it. That def random_number is in MRI source but it is commented out! with =begin. Why!?!?! :)

@enebo
Copy link
Member

enebo commented May 16, 2017

Ok I was hoping to find something trivial to change here (since I am releasing 9.1.9.0 this morning) but I do not see where random_number is actually defined. There is no references to it in MRI C source and the only ruby definition is commented out. I half wonder if something like openssl gem defines it?

@DeeJayhX
Copy link
Author

@enebo I went through the same exact steps you did lol. Right away I was like "OH I got this! Then pull-request and good to go!", but no such luck lol. I hope this is solved soon, I cannot use JRuby without this...

@kares
Copy link
Member

kares commented Jun 6, 2017

turns out its both - .rb parts are used but native ext is used. securerandom.rb is not up to date with MRI's
will look into that, and it seems like the current impl of random_number is moved into C will try to match it.

@kares kares self-assigned this Jun 6, 2017
@DeeJayhX
Copy link
Author

DeeJayhX commented Jun 6, 2017

@kares Thank you!

@kares kares mentioned this issue Jun 8, 2017
Merged
@kares kares added this to the JRuby 9.1.11.0 milestone Jun 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kares kares

Labels
None yet
Projects
None yet
Milestone
JRuby 9.1.11.0
Development

Successfully merging a pull request may close this issue.

SecureRandom update to match Ruby 2.3 kares/jruby
3 participants
@enebo @kares @DeeJayhX