SecureRandom.random_number doesn't accept a range #4607

Closed
Ruined1 opened this Issue May 16, 2017 · 6 comments

Comments

Projects
None yet
3 participants
@Ruined1

Ruined1 commented May 16, 2017

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

This comment has been minimized.

Show comment
Hide comment
@enebo

enebo May 16, 2017

Member

@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.

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

This comment has been minimized.

Show comment
Hide comment
@enebo

enebo May 16, 2017

Member

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!?!?! :)

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

This comment has been minimized.

Show comment
Hide comment
@enebo

enebo May 16, 2017

Member

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?

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?

@Ruined1

This comment has been minimized.

Show comment
Hide comment
@Ruined1

Ruined1 May 16, 2017

@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...

Ruined1 commented May 16, 2017

@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

This comment has been minimized.

Show comment
Hide comment
@kares

kares Jun 6, 2017

Member

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.

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

@Ruined1

This comment has been minimized.

Show comment
Hide comment

Ruined1 commented Jun 6, 2017

@kares Thank you!

@kares kares added this to the JRuby 9.1.11.0 milestone Jun 8, 2017

@kares kares closed this in #4656 Jun 12, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment