fix timeout and timeout_ms to handle floats and negative numbers (#361)

* fix timeout and timeout_ms to handle floats and negative numbers

The setter used `CURB_IMMED_SETTER` macro and it casts value to `long`
and loses precision if float was passed. C will round down as part of
the conversion and if it converts to 0 then the IMMED_GETTER will return
nil.

This results in a confusing output described in #355.

This commit deperecates `CURLOPT_TIMEOUT` option and the code will rely
exclusively on `timeout_ms` (and `CURLOPT_TIMEOUT_MS` accordingly).

There are minor user-facing changes:

* timeout and timeout_ms will not return nil anymore
* change of timeout will now also change timeout_ms respectively
* timeout will return numeric rather than fixnum

Otherwise everything should be working as expected.

* remove unnecessary code to handle timeout and timeout_ms

This is being handled in the setters now and the condition will never
evaluate to true so it's good to remove.

ext/curb_easy.c

@@ -1139,7 +1139,7 @@ static VALUE ruby_curl_easy_max_redirects_get(VALUE self) {
 
 /*
  * call-seq:
- *   easy.timeout = fixnum or nil                     => fixnum or nil
+ *   easy.timeout = float, fixnum or nil              => numeric
  *
  * Set the maximum time in seconds that you allow the libcurl transfer
  * operation to take. Normally, name lookups can take a considerable time
@@ -1148,20 +1148,39 @@ static VALUE ruby_curl_easy_max_redirects_get(VALUE self) {
  *
  * Set to nil (or zero) to disable timeout (it will then only timeout
  * on the system's internal timeouts).
+ *
+ * Uses timeout_ms internally instead of timeout because it allows for
+ * better precision and libcurl will use the last set value when both
+ * timeout and timeout_ms are set.
+ *
  */
-static VALUE ruby_curl_easy_timeout_set(VALUE self, VALUE timeout) {
-  CURB_IMMED_SETTER(ruby_curl_easy, timeout, 0);
+static VALUE ruby_curl_easy_timeout_set(VALUE self, VALUE timeout_s) {
+  ruby_curl_easy *rbce;
+  Data_Get_Struct(self, ruby_curl_easy, rbce);
+
+  if (Qnil == timeout_s || NUM2DBL(timeout_s) <= 0.0) {
+    rbce->timeout_ms = 0;
+  } else {
+    rbce->timeout_ms = (unsigned long)(NUM2DBL(timeout_s) * 1000);
+  }
+
+  return DBL2NUM(rbce->timeout_ms / 1000.0);
 }
 
 /*
  * call-seq:
- *   easy.timeout                                     => fixnum or nil
+ *   easy.timeout                                     => numeric
  *
  * Obtain the maximum time in seconds that you allow the libcurl transfer
  * operation to take.
+ *
+ * Uses timeout_ms internally instead of timeout.
+ *
  */
-static VALUE ruby_curl_easy_timeout_get(VALUE self, VALUE timeout) {
-  CURB_IMMED_GETTER(ruby_curl_easy, timeout, 0);
+static VALUE ruby_curl_easy_timeout_get(VALUE self) {
+  ruby_curl_easy *rbce;
+  Data_Get_Struct(self, ruby_curl_easy, rbce);
+  return DBL2NUM(rbce->timeout_ms / 1000.0);
 }
 
 /*
@@ -1177,7 +1196,16 @@ static VALUE ruby_curl_easy_timeout_get(VALUE self, VALUE timeout) {
  * on the system's internal timeouts).
  */
 static VALUE ruby_curl_easy_timeout_ms_set(VALUE self, VALUE timeout_ms) {
-  CURB_IMMED_SETTER(ruby_curl_easy, timeout_ms, 0);
+  ruby_curl_easy *rbce;
+  Data_Get_Struct(self, ruby_curl_easy, rbce);
+
+  if (Qnil == timeout_ms || NUM2DBL(timeout_ms) <= 0.0) {
+    rbce->timeout_ms = 0;
+  } else {
+    rbce->timeout_ms = NUM2ULONG(timeout_ms);
+  }
+
+  return ULONG2NUM(rbce->timeout_ms);
 }
 
 /*
@@ -1187,8 +1215,10 @@ static VALUE ruby_curl_easy_timeout_ms_set(VALUE self, VALUE timeout_ms) {
  * Obtain the maximum time in milliseconds that you allow the libcurl transfer
  * operation to take.
  */
-static VALUE ruby_curl_easy_timeout_ms_get(VALUE self, VALUE timeout_ms) {
-  CURB_IMMED_GETTER(ruby_curl_easy, timeout_ms, 0);
+static VALUE ruby_curl_easy_timeout_ms_get(VALUE self) {
+  ruby_curl_easy *rbce;
+  Data_Get_Struct(self, ruby_curl_easy, rbce);
+  return LONG2NUM(rbce->timeout_ms);
 }
 
 /*
@@ -2174,13 +2204,8 @@ VALUE ruby_curl_easy_setup(ruby_curl_easy *rbce) {
 
   curl_easy_setopt(curl, CURLOPT_UNRESTRICTED_AUTH, rbce->unrestricted_auth);
 
-  /* timeouts override each other we cannot blindly set timeout and timeout_ms */
-  if (rbce->timeout && rbce->timeout > 0) {
-    curl_easy_setopt(curl, CURLOPT_TIMEOUT, rbce->timeout);
-  }
-  if (rbce->timeout_ms && rbce->timeout_ms > 0) {
-    curl_easy_setopt(curl, CURLOPT_TIMEOUT_MS, rbce->timeout_ms);
-  }
+  curl_easy_setopt(curl, CURLOPT_TIMEOUT_MS, rbce->timeout_ms);
+
   curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT, rbce->connect_timeout);
   curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT_MS, rbce->connect_timeout_ms);
   curl_easy_setopt(curl, CURLOPT_DNS_CACHE_TIMEOUT, rbce->dns_cache_timeout);

tests/tc_curl_easy.rb

@@ -333,29 +333,69 @@ def test_max_redirects_01
     c.max_redirects = nil
     assert_nil c.max_redirects
   end
-
+
+  def test_timeout_with_floats
+    c = Curl::Easy.new($TEST_URL)
+
+    c.timeout = 1.5
+    assert_equal 1500, c.timeout_ms
+    assert_equal 1.5, c.timeout
+  end
+
+  def test_timeout_with_negative
+    c = Curl::Easy.new($TEST_URL)
+
+    c.timeout = -1.5
+    assert_equal 0, c.timeout
+    assert_equal 0, c.timeout_ms
+
+    c.timeout = -4.8
+    assert_equal 0, c.timeout
+    assert_equal 0, c.timeout_ms
+  end
+
   def test_timeout_01
     c = Curl::Easy.new($TEST_URL)
-    
-    assert_nil c.timeout
-    
+
+    assert_equal 0, c.timeout
+
     c.timeout = 3
     assert_equal 3, c.timeout
-    
-    c.timeout = nil
-    assert_nil c.timeout
+
+    c.timeout = 0
+    assert_equal 0, c.timeout
   end
 
   def test_timeout_ms_01
     c = Curl::Easy.new($TEST_URL)
 
-    assert_nil c.timeout_ms
+    assert_equal 0, c.timeout_ms
 
     c.timeout_ms = 100
     assert_equal 100, c.timeout_ms
 
     c.timeout_ms = nil
-    assert_nil c.timeout_ms
+    assert_equal 0, c.timeout_ms
+  end
+
+  def test_timeout_ms_with_floats
+    c = Curl::Easy.new($TEST_URL)
+
+    c.timeout_ms = 55.5
+    assert_equal 55, c.timeout_ms
+    assert_equal 0.055, c.timeout
+  end
+
+  def test_timeout_ms_with_negative
+    c = Curl::Easy.new($TEST_URL)
+
+    c.timeout_ms = -1.5
+    assert_equal 0, c.timeout
+    assert_equal 0, c.timeout_ms
+
+    c.timeout_ms = -4.8
+    assert_equal 0, c.timeout
+    assert_equal 0, c.timeout_ms
   end
 
   def test_connect_timeout_01