Inconsistent parsing of long strings to integers between JRuby and MRI #1608

Closed
iconara opened this Issue Apr 3, 2014 · 2 comments

Projects

None yet

2 participants

@iconara
Contributor
iconara commented Apr 3, 2014

Using the Integer function to parse a string into a number with long strings gives different results in MRI and JRuby:

$ rvm ruby-2.0.0-p353 do ruby -e 'p Integer("a4a7090024e111df8924001ff359171x", 16)'
-e:1:in `Integer': invalid value for Integer(): "a4a7090024e111df8924001ff359171x" (ArgumentError)
    from -e:1:in `<main>'
$ rvm jruby-1.7.11 do ruby -e 'p Integer("a4a7090024e111df8924001ff359171x", 16)'
13678792964825094081371290073587618161

Curiously it seems to have something to do with the length of the string:

$ rvm jruby-1.7.11 do ruby -e 'p Integer("111111111111111x", 16)'
76861433640456465
$ rvm jruby-1.7.11 do ruby -e 'p Integer("11111111111111x", 16)'
ArgumentError: invalid value for Integer: "11111111111111x"
  Integer at org/jruby/RubyKernel.java:513
   (root) at -e:1

(with 16 characters it works, with 15 it doesn't)

It also doesn't seem to have anything to do with the base:

$ rvm jruby-1.7.11 do ruby -e 'p Integer("11111111111111x")'
ArgumentError: invalid value for Integer: "11111111111111x"
  Integer at org/jruby/RubyKernel.java:499
   (root) at -e:1
$ rvm jruby-1.7.11 do ruby -e 'p Integer("111111111111111x")'
111111111111111
@iconara
Contributor
iconara commented Apr 3, 2014

Looks like it's ConvertBytes#byteListToInum that calls #bigParse when the string is longer than what would fit into a long, and #bigParse doesn't blow up when encountering a bad character, it just stops.

@iconara iconara referenced this issue in iconara/cql-rb Apr 3, 2014
Closed

UUID validation broken in JRuby #87

@headius
Member
headius commented Apr 7, 2014

Ok, confirmed locally, and it's not anything in the plumbing leading up to the actual conversion. I will look into your theory about bytesListToInum and bigParse.

@headius headius added a commit that referenced this issue Apr 7, 2014
@headius headius Regression spec for #1608. e05539b
@headius headius added a commit that closed this issue Apr 7, 2014
@headius headius Various fixes for Kernel.Integer().
* Localize conversion logic in TypeConverter.
* Add missing "badcheck" logic for String to Bignum. Fixes #1608.
* Fix last failure in ruby/test_integer.rb by defining
  Fixnum#succ.

Still needs tests or specs.
4e6e7e6
@headius headius closed this in 4e6e7e6 Apr 7, 2014
@headius headius added this to the JRuby 1.7.12 milestone Apr 7, 2014
@russCloak russCloak added a commit to russCloak/cql-rb that referenced this issue Aug 5, 2014
@russCloak russCloak Raise specific errors with invalid UUID cases
It would be more helpful for developers integrating functionality at a
higher level if errors raised for invalid UUIDs are namespaced. This
adds a new Cql::Uuid::InvalidUuidError that improves two cases:

 - The fix for jruby issue jruby/jruby#1608
   is no longer necessary because we are now raising an explicit error
   describing that the UUID is invalid - so the reliance on raising an
   ArgumentError in order to maintain consistency for Integer is no
   longer necessary.

 - The error messages are now more specific, and therefore more helpful
   for when a developer is doing an integration. On the same note, the
   errors being namespaced specifically under Cql::Uuid allows for a
   rescue for specific cases, instead of the higher-level ArgumentError
   that could occur in many other places, for many other reasons.
f3bd764
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment