request.rb in rest-client-maestro-1.7.4.maestro

- old
+ new

@@ -19,11 +19,12 @@
   # * :user and :password for basic auth, will be replaced by a user/password available in the :url
   # * :block_response call the provided block with the HTTPResponse as parameter
   # * :raw_response return a low-level RawResponse instead of a Response
   # * :max_redirects maximum number of redirections (default to 10)
   # * :verify_ssl enable ssl verification, possible values are constants from OpenSSL::SSL
-  # * :timeout and :open_timeout passing in -1 will disable the timeout by setting the corresponding net timeout values to nil
+  # * :timeout and :open_timeout are how long to wait for a response and to
+  #     open a connection, in seconds. Pass nil to disable the timeout.
   # * :ssl_client_cert, :ssl_client_key, :ssl_ca_file, :ssl_ca_path
   # * :ssl_version specifies the SSL version for the underlying Net::HTTP connection (defaults to 'SSLv3')
   class Request
 
     attr_reader :method, :url, :headers, :cookies,
@@ -46,12 +47,16 @@
       end
       @cookies = @headers.delete(:cookies) || args[:cookies] || {}
       @payload = Payload.generate(args[:payload])
       @user = args[:user]
       @password = args[:password]
-      @timeout = args[:timeout]
-      @open_timeout = args[:open_timeout]
+      if args.include?(:timeout)
+        @timeout = args[:timeout]
+      end
+      if args.include?(:open_timeout)
+        @open_timeout = args[:open_timeout]
+      end
       @block_response = args[:block_response]
       @raw_response = args[:raw_response] || false
       @verify_ssl = args[:verify_ssl] || false
       @ssl_client_cert = args[:ssl_client_cert] || nil
       @ssl_client_key = args[:ssl_client_key] || nil
@@ -90,17 +95,50 @@
       end
     end
 
     def make_headers user_headers
       unless @cookies.empty?
-        user_headers[:cookie] = @cookies.map { |(key, val)| "#{key.to_s}=#{CGI::unescape(val.to_s)}" }.sort.join('; ')
+
+        # Validate that the cookie names and values look sane. If you really
+        # want to pass scary characters, just set the Cookie header directly.
+        # RFC6265 is actually much more restrictive than we are.
+        @cookies.each do |key, val|
+          unless valid_cookie_key?(key)
+            raise ArgumentError.new("Invalid cookie name: #{key.inspect}")
+          end
+          unless valid_cookie_value?(val)
+            raise ArgumentError.new("Invalid cookie value: #{val.inspect}")
+          end
+        end
+
+        user_headers[:cookie] = @cookies.map { |key, val| "#{key}=#{val}" }.sort.join('; ')
       end
       headers = stringify_headers(default_headers).merge(stringify_headers(user_headers))
       headers.merge!(@payload.headers) if @payload
       headers
     end
 
+    # Do some sanity checks on cookie keys.
+    #
+    # Properly it should be a valid TOKEN per RFC 2616, but lots of servers are
+    # more liberal.
+    #
+    # Disallow the empty string as well as keys containing control characters,
+    # equals sign, semicolon, comma, or space.
+    #
+    def valid_cookie_key?(string)
+      return false if string.empty?
+
+      ! Regexp.new('[\x0-\x1f\x7f=;, ]').match(string)
+    end
+
+    # Validate cookie values. Rather than following RFC 6265, allow anything
+    # but control characters, comma, and semicolon.
+    def valid_cookie_value?(value)
+      ! Regexp.new('[\x0-\x1f\x7f,;]').match(value)
+    end
+
     def net_http_class
       if RestClient.proxy
         proxy_uri = URI.parse(RestClient.proxy)
         Net::HTTP::Proxy(proxy_uri.host, proxy_uri.port, proxy_uri.user, proxy_uri.password)
       else
@@ -165,16 +203,25 @@
       end
       net.cert = @ssl_client_cert if @ssl_client_cert
       net.key = @ssl_client_key if @ssl_client_key
       net.ca_file = @ssl_ca_file if @ssl_ca_file
       net.ca_path = @ssl_ca_path if @ssl_ca_path
-      net.read_timeout = @timeout if @timeout
-      net.open_timeout = @open_timeout if @open_timeout
 
-      # disable the timeout if the timeout value is -1
-      net.read_timeout = nil if @timeout == -1
-      net.open_timeout = nil if @open_timeout == -1
+      if defined? @timeout
+        if @timeout == -1
+          warn 'To disable read timeouts, please set timeout to nil instead of -1'
+          @timeout = nil
+        end
+        net.read_timeout = @timeout
+      end
+      if defined? @open_timeout
+        if @open_timeout == -1
+          warn 'To disable open timeouts, please set open_timeout to nil instead of -1'
+          @open_timeout = nil
+        end
+        net.open_timeout = @open_timeout
+      end
 
       RestClient.before_execution_procs.each do |before_proc|
         before_proc.call(req, args)
       end
 
@@ -203,11 +250,11 @@
       else
         raise e
       end
     rescue EOFError
       raise RestClient::ServerBrokeConnection
-    rescue Timeout::Error
+    rescue Timeout::Error, Errno::ETIMEDOUT
       raise RestClient::RequestTimeout
     end
 
     def setup_credentials(req)
       req.basic_auth(user, password) if user
@@ -217,9 +264,10 @@
       if @raw_response
         # Taken from Chef, which as in turn...
         # Stolen from http://www.ruby-forum.com/topic/166423
         # Kudos to _why!
         @tf = Tempfile.new("rest-client")
+        @tf.binmode
         size, total = 0, http_response.header['Content-Length'].to_i
         http_response.read_body do |chunk|
           @tf.write chunk
           size += chunk.size
           if RestClient.log