Fix RUBY-2321 Timeout of 0 should mean no timeout

docs/tutorials/ruby-driver-create-client.txt

@@ -325,7 +325,9 @@ Ruby Options
 
    * - ``:connect_timeout``
      - The number of seconds to wait to establish a socket connection
-       before raising an exception.
+       before raising an exception. The value of ``0`` means no timeout.
+       Client creation will fail with an error if an invalid timeout value
+       is passed (such as a negative value or a non-numeric value).
      - ``Float``
      - 10 seconds
 
@@ -481,7 +483,9 @@ Ruby Options
 
    * - ``:socket_timeout``
      - The number of seconds to wait for an operation to execute on a
-       socket before raising an exception.
+       socket before raising an exception. The value of ``0`` means no timeout.
+       Client creation will fail with an error if an invalid timeout value
+       is passed (such as a negative value or a non-numeric value).
      - ``Float``
      - 5 seconds
 
@@ -701,6 +705,10 @@ URI options are explained in detail in the :manual:`Connection URI reference
    * - connectTimeoutMS=Integer
      - ``:connect_timeout => Float``
 
+       Unlike the corresponding Ruby option which fails client creation on
+       invalid values (e.g. negative and non-numeric values), invalid values
+       provided via this URI option are ignored with a warning.
+
    * - directConnection=Boolean
      - ``:direct_connection => Boolean``
 
@@ -762,6 +770,10 @@ URI options are explained in detail in the :manual:`Connection URI reference
    * - socketTimeoutMS=Integer
      - ``:socket_timeout => Float``
 
+       Unlike the corresponding Ruby option which fails client creation on
+       invalid values (e.g. negative and non-numeric values), invalid values
+       provided via this URI option are ignored with a warning.
+
    * - ssl=Boolean
      - ``:ssl => true|false``
 
@@ -824,6 +836,13 @@ Timeout Options
 When executing an operation, the number of seconds to wait for the driver
 to find an appropriate server to send an operation to. Defaults to 30.
 
+A value of 0 means no timeout.
+
+When an invalid value (e.g. a negative value or a non-numeric value) is passed
+via the URI option, the invalid input is ignored with a warning. When an
+invalid value is passed directly to Client via a Ruby option, Client
+construction fails with an error.
+
 In replica set deployments, this timeout should be set to exceed the typical
 :manual:`replica set election times </core/replica-set-elections/>`
 in order for the driver to transparently handle primary changes. This timeout
@@ -843,6 +862,13 @@ provide quicker failure when the server is not running.
 The number of seconds to wait for a socket read or write to complete on
 regular (non-monitoring) connections. Default is no timeout.
 
+A value of 0 means no timeout.
+
+When an invalid value (e.g. a negative value or a non-numeric value) is passed
+via the URI option, the invalid input is ignored with a warning. When an
+invalid value is passed directly to Client via a Ruby option, Client
+construction fails with an error.
+
 This timeout should take into account both network latency and operation
 duration. For example, setting this timeout to 5 seconds will abort queries
 taking more than 5 seconds to execute on the server with ``Mongo::Error::SocketTimeoutError``.

lib/mongo/client.rb

@@ -1137,6 +1137,17 @@ def validate_options!(addresses = nil)
         raise ArgumentError, "Conflicting client options: direct_connection=false and connect=#{options[:connect]}"
       end
 
+      %i(connect_timeout socket_timeout).each do |key|
+        if value = options[key]
+          unless Numeric === value
+            raise ArgumentError, "#{key} must be a non-negative number: #{value}"
+          end
+          if value < 0
+            raise ArgumentError, "#{key} must be a non-negative number: #{value}"
+          end
+        end
+      end
+
       if value = options[:bg_error_backtrace]
         case value
         when Integer

lib/mongo/socket.rb

@@ -249,8 +249,13 @@ def read_from_socket(length)
         return ''.force_encoding('BINARY')
       end
 
-      if _timeout = self.timeout
-        deadline = Time.now + _timeout
+      _timeout = self.timeout
+      if _timeout
+        if _timeout > 0
+          deadline = Time.now + _timeout
+        elsif _timeout < 0
+          raise Errno::ETIMEDOUT, "Negative timeout #{_timeout} given to socket"
+        end
       end
 
       # We want to have a fixed and reasonably small size buffer for reads

spec/mongo/client_construction_spec.rb

@@ -319,6 +319,56 @@
         end
       end
 
+      context 'timeout options' do
+        let(:client) do
+          new_local_client(SpecConfig.instance.addresses,
+            SpecConfig.instance.authorized_test_options.merge(options))
+        end
+
+        context 'when network timeouts are zero' do
+          let(:options) do
+            { socket_timeout: 0, connect_timeout: 0 }
+          end
+
+          it 'sets options to zeros' do
+            client.options[:socket_timeout].should == 0
+            client.options[:connect_timeout].should == 0
+          end
+
+          it 'connects and performs operations successfully' do
+            lambda do
+              client.database.command(ping: 1)
+            end.should_not raise_error
+          end
+        end
+
+        %i(socket_timeout connect_timeout).each do |option|
+          context "when #{option} is negative" do
+            let(:options) do
+              { option => -1 }
+            end
+
+            it 'fails client creation' do
+              lambda do
+                client
+              end.should raise_error(ArgumentError, /#{option} must be a non-negative number/)
+            end
+          end
+
+          context "when #{option} is of the wrong type" do
+            let(:options) do
+              { option => '42' }
+            end
+
+            it 'fails client creation' do
+              lambda do
+                client
+              end.should raise_error(ArgumentError, /#{option} must be a non-negative number/)
+            end
+          end
+        end
+      end
+
       context 'retry_writes option' do
         let(:client) do
           new_local_client_nmio(SpecConfig.instance.addresses, options)