From 0893ee492c189e4d2c5badfd326a2e45049c89a5 Mon Sep 17 00:00:00 2001 From: KOBAYASHI Shuji Date: Mon, 9 Dec 2019 21:47:59 +0900 Subject: Fix that `String#to_f` accepts consecutive `_` as a numeric expression Consecutive `_` is not allowed as a numeric expression: 1_2__3 #=> SyntaxError Float("1_2__3") #=> ArgumentError Integer("1_2__3") #=> ArgumentError "1_2__3".to_i #=> 12 But `String#to_f` accept it, so I fixed the issue. Before this patch: "1_2__3".to_f #=> 123 After this patch: "1_2__3".to_f #=> 12 --- mrbgems/mruby-kernel-ext/test/kernel.rb | 36 ++++++++++++++++++------- src/string.c | 13 +++------ test/t/string.rb | 48 ++++++++++++++++++--------------- 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 -- cgit v1.2.3