diff --git a/hrtime.h b/hrtime.h index 80aff5deb3..7ed4e6b04c 100644 --- a/hrtime.h +++ b/hrtime.h @@ -206,6 +206,7 @@ double2hrtime(rb_hrtime_t *hrt, double d) const double TIMESPEC_SEC_MAX_PLUS_ONE = 2.0 * (TIMESPEC_SEC_MAX_as_double / 2.0 + 1.0); if (TIMESPEC_SEC_MAX_PLUS_ONE <= d) { + *hrt = RB_HRTIME_MAX; return NULL; } else if (d <= 0) { diff --git a/re.c b/re.c index e16e631a15..c1214f392a 100644 --- a/re.c +++ b/re.c @@ -3772,6 +3772,8 @@ str_to_option(VALUE str) * If optional keyword argument +timeout+ is given, * its float value overrides the timeout interval for the class, * Regexp.timeout. + * If +nil+ is passed as +timeout, it uses the timeout interval + * for the class, Regexp.timeout. * * With argument +regexp+ given, returns a new regexp. The source, * options, timeout are the same as +regexp+. +options+ and +n_flag+ @@ -3847,7 +3849,9 @@ rb_reg_initialize_m(int argc, VALUE *argv, VALUE self) { double limit = NIL_P(timeout) ? 0.0 : NUM2DBL(timeout); - if (limit < 0) limit = 0; + if (!NIL_P(timeout) && limit <= 0) { + rb_raise(rb_eArgError, "invalid timeout: %"PRIsVALUE, timeout); + } double2hrtime(®->timelimit, limit); } @@ -4478,7 +4482,9 @@ rb_reg_s_timeout_set(VALUE dummy, VALUE limit) rb_ractor_ensure_main_ractor("can not access Regexp.timeout from non-main Ractors"); - if (timeout < 0) timeout = 0; + if (!NIL_P(limit) && timeout <= 0) { + rb_raise(rb_eArgError, "invalid timeout: %"PRIsVALUE, limit); + } double2hrtime(&rb_reg_match_time_limit, timeout); return limit; diff --git a/test/ruby/test_regexp.rb b/test/ruby/test_regexp.rb index 7d7d7e5180..3d3cdbb46e 100644 --- a/test/ruby/test_regexp.rb +++ b/test/ruby/test_regexp.rb @@ -1579,7 +1579,7 @@ class TestRegexp < Test::Unit::TestCase end def test_s_timeout - assert_separately([], "#{<<-"begin;"}\n#{<<-"end;"}") + assert_separately([], "#{<<-"begin;"}\n#{<<-'end;'}") begin; timeout = EnvUtil.apply_timeout_scale(0.2) @@ -1597,15 +1597,42 @@ class TestRegexp < Test::Unit::TestCase end; end - def test_timeout - assert_separately([], "#{<<-"begin;"}\n#{<<-"end;"}") + def test_s_timeout_corner_cases + assert_separately([], "#{<<-"begin;"}\n#{<<-'end;'}") begin; - dummy_timeout = EnvUtil.apply_timeout_scale(10) - timeout = EnvUtil.apply_timeout_scale(0.2) + assert_nil(Regexp.timeout) - Regexp.timeout = dummy_timeout # This should be ignored + # This is just an implementation detail that users should not depend on: + # If Regexp.timeout is set to a value greater than the value that can be + # represented in the internal representation of timeout, it uses the + # maximum value that can be represented. + Regexp.timeout = Float::INFINITY + assert_equal(((1<<64)-1) / 1000000000.0, Regexp.timeout) - re = Regexp.new("^a*b?a*$", timeout: timeout) + Regexp.timeout = 1e300 + assert_equal(((1<<64)-1) / 1000000000.0, Regexp.timeout) + + assert_raise(ArgumentError) { Regexp.timeout = 0 } + assert_raise(ArgumentError) { Regexp.timeout = -1 } + + Regexp.timeout = nil + assert_nil(Regexp.timeout) + end; + end + + def per_instance_redos_test(global_timeout, per_instance_timeout, expected_timeout) + assert_separately([], "#{<<-"begin;"}\n#{<<-'end;'}") + global_timeout = #{ global_timeout.inspect } + per_instance_timeout = #{ per_instance_timeout.inspect } + expected_timeout = #{ expected_timeout.inspect } + begin; + global_timeout = EnvUtil.apply_timeout_scale(global_timeout) + per_instance_timeout = EnvUtil.apply_timeout_scale(per_instance_timeout) + + Regexp.timeout = global_timeout + + re = Regexp.new("^a*b?a*$", timeout: per_instance_timeout) + assert_equal(per_instance_timeout, re.timeout) t = Time.now assert_raise_with_message(Regexp::TimeoutError, "regexp match timeout") do @@ -1613,7 +1640,37 @@ class TestRegexp < Test::Unit::TestCase end t = Time.now - t - assert_in_delta(timeout, t, timeout / 2) + assert_in_delta(expected_timeout, t, expected_timeout / 2) + end; + end + + def test_timeout_shorter_than_global + per_instance_redos_test(10, 0.2, 0.2) + end + + def test_timeout_longer_than_global + per_instance_redos_test(0.01, 0.5, 0.5) + end + + def test_timeout_nil + per_instance_redos_test(0.5, nil, 0.5) + end + + def test_timeout_corner_cases + assert_separately([], "#{<<-"begin;"}\n#{<<-'end;'}") + begin; + assert_nil(//.timeout) + + # This is just an implementation detail that users should not depend on: + # If Regexp.timeout is set to a value greater than the value that can be + # represented in the internal representation of timeout, it uses the + # maximum value that can be represented. + assert_equal(((1<<64)-1) / 1000000000.0, Regexp.new("foo", timeout: Float::INFINITY).timeout) + + assert_equal(((1<<64)-1) / 1000000000.0, Regexp.new("foo", timeout: 1e300).timeout) + + assert_raise(ArgumentError) { Regexp.new("foo", timeout: 0) } + assert_raise(ArgumentError) { Regexp.new("foo", timeout: -1) } end; end end