Fix per-instance Regexp timeout (#6621)

Fix per-instance Regexp timeout

This makes it follow what was decided in [Bug #19055]:

* `Regexp.new(str, timeout: nil)` should respect the global timeout
* `Regexp.new(str, timeout: huge_val)` should use the maximum value that
  can be represented in the internal representation
* `Regexp.new(str, timeout: 0 or negative value)` should raise an error

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) {

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(&reg->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;

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