Merge pull request #4858 from shuujii/fix-that-String-to_f-accepts-consecutive-_-as-a-numeric-expression

Fix that `String#to_f` accepts consecutive `_` as a numeric expression

3 files changed, 57 insertions, 40 deletions

diff --git a/mrbgems/mruby-kernel-ext/test/kernel.rb b/mrbgems/mruby-kernel-ext/test/kernel.rb
index ad9177165..2bd901a4c 100644
--- a/mrbgems/mruby-kernel-ext/test/kernel.rb
+++ b/mrbgems/mruby-kernel-ext/test/kernel.rb
@@ -49,24 +49,42 @@ assert('Kernel#__method__') do
 end
 
 assert('Kernel#Integer') do
-  assert_equal(26, Integer("0x1a"))
-  assert_equal(930, Integer("0930", 10))
-  assert_equal(7, Integer("111", 2))
-  assert_equal(0, Integer("0"))
-  assert_equal(0, Integer("00000"))
+  assert_operator(26, :eql?, Integer("0x1a"))
+  assert_operator(930, :eql?, Integer("0930", 10))
+  assert_operator(7, :eql?, Integer("111", 2))
+  assert_operator(0, :eql?, Integer("0"))
+  assert_operator(0, :eql?, Integer("00000"))
+  assert_operator(123, :eql?, Integer('1_2_3'))
   assert_raise(TypeError) { Integer(nil) }
+  assert_raise(ArgumentError) { Integer('a') }
+  assert_raise(ArgumentError) { Integer('4a5') }
+  assert_raise(ArgumentError) { Integer('1_2__3') }
+  assert_raise(ArgumentError) { Integer('68_') }
+  assert_raise(ArgumentError) { Integer("15\0") }
+  assert_raise(ArgumentError) { Integer("15.0") }
   skip unless Object.const_defined?(:Float)
-  assert_equal(123, Integer(123.999))
+  assert_operator(123, :eql?, Integer(123.999))
 end
 
 assert('Kernel#Float') do
   skip unless Object.const_defined?(:Float)
-  assert_equal(1.0, Float(1))
-  assert_equal(123.456, Float(123.456))
-  assert_equal(123.456, Float("123.456"))
+  assert_operator(1.0, :eql?, Float(1))
+  assert_operator(123.456, :eql?, Float(123.456))
+  assert_operator(123.456, :eql?, Float("123.456"))
+  assert_operator(123.0, :eql?, Float('1_2_3'))
+  assert_operator(12.34, :eql?, Float('1_2.3_4'))
+  assert_operator(0.9, :eql?, Float('.9'))
   assert_raise(TypeError) { Float(nil) }
   assert_raise(ArgumentError) { Float("1.5a") }
   assert_raise(ArgumentError) { Float("1.5\0") }
+  assert_raise(ArgumentError) { Float('a') }
+  assert_raise(ArgumentError) { Float('4a5') }
+  assert_raise(ArgumentError) { Float('1_2__3') }
+  assert_raise(ArgumentError) { Float('68_') }
+  assert_raise(ArgumentError) { Float('68._7') }
+  assert_raise(ArgumentError) { Float('68.7_') }
+  assert_raise(ArgumentError) { Float('_68') }
+  assert_raise(ArgumentError) { Float('1_2.3__4') }
 end
 
 assert('Kernel#String') do
diff --git a/src/string.c b/src/string.c
index 61fbd4ded..f75679513 100644
--- a/src/string.c
+++ b/src/string.c
@@ -2522,15 +2522,10 @@ bad:
     while (p < end && n < e) prev = *n++ = *p++;
     while (*p) {
       if (*p == '_') {
-        /* remove underscores between digits */
-        if (badcheck) {
-          if (n == buf || !ISDIGIT(prev)) goto bad;
-          ++p;
-          if (!ISDIGIT(*p)) goto bad;
-        }
-        else {
-          while (*++p == '_');
-          continue;
+        /* remove an underscore between digits */
+        if (n == buf || !ISDIGIT(prev) || (++p, !ISDIGIT(*p))) {
+          if (badcheck) goto bad;
+          break;
         }
       }
       prev = *p++;
diff --git a/test/t/string.rb b/test/t/string.rb
index 65ad13103..6dbc91963 100644
--- a/test/t/string.rb
+++ b/test/t/string.rb
@@ -687,31 +687,35 @@ assert('String#sub!', '15.2.10.5.37') do
 end
 
 assert('String#to_f', '15.2.10.5.38') do
-  a = ''.to_f
-  b = '123456789'.to_f
-  c = '12345.6789'.to_f
-  d = '1e-2147483648'.to_f
-  e = '1e2147483648'.to_f
-
-  assert_float(0.0, a)
-  assert_float(123456789.0, b)
-  assert_float(12345.6789, c)
-  assert_float(0, d)
-  assert_float(Float::INFINITY, e)
+  assert_operator(0.0, :eql?, ''.to_f)
+  assert_operator(123456789.0, :eql?, '123456789'.to_f)
+  assert_operator(12345.6789, :eql?, '12345.6789'.to_f)
+  assert_operator(0.0, :eql?, '1e-2147483648'.to_f)
+  assert_operator(Float::INFINITY, :eql?, '1e2147483648'.to_f)
+  assert_operator(0.0, :eql?, 'a'.to_f)
+  assert_operator(4.0, :eql?, '4a5'.to_f)
+  assert_operator(12.0, :eql?, '1_2__3'.to_f)
+  assert_operator(123.0, :eql?, '1_2_3'.to_f)
+  assert_operator(68.0, :eql?, '68_'.to_f)
+  assert_operator(68.0, :eql?, '68._7'.to_f)
+  assert_operator(68.7, :eql?, '68.7_'.to_f)
+  assert_operator(0.0, :eql?, '_68'.to_f)
+  assert_operator(12.34, :eql?, '1_2.3_4'.to_f)
+  assert_operator(12.3, :eql?, '1_2.3__4'.to_f)
+  assert_operator(0.9, :eql?, '.9'.to_f)
 end if Object.const_defined?(:Float)
 
 assert('String#to_i', '15.2.10.5.39') do
-  a = ''.to_i
-  b = '32143'.to_i
-  c = 'a'.to_i(16)
-  d = '100'.to_i(2)
-  e = '1_000'.to_i
-
-  assert_equal 0, a
-  assert_equal 32143, b
-  assert_equal 10, c
-  assert_equal 4, d
-  assert_equal 1_000, e
+  assert_operator 0, :eql?, ''.to_i
+  assert_operator 32143, :eql?, '32143'.to_i
+  assert_operator 10, :eql?, 'a'.to_i(16)
+  assert_operator 4, :eql?, '100'.to_i(2)
+  assert_operator 1_000, :eql?, '1_000'.to_i
+  assert_operator 0, :eql?, 'a'.to_i
+  assert_operator 4, :eql?, '4a5'.to_i
+  assert_operator 12, :eql?, '1_2__3'.to_i
+  assert_operator 123, :eql?, '1_2_3'.to_i
+  assert_operator 68, :eql?, '68_'.to_i
 end
 
 assert('String#to_s', '15.2.10.5.40') do