httpclient.rb in httpclient-2.6.0

- old
+ new

@@ -306,11 +306,11 @@
     end
   end
 
   # HTTPClient::SSLConfig:: SSL configurator.
   attr_reader :ssl_config
-  # WebAgent::CookieManager:: Cookies configurator.
+  # HTTPClient::CookieManager:: Cookies configurator.
   attr_accessor :cookie_manager
   # An array of response HTTP message body String which is used for loop-back
   # test.  See test/* to see how to use it.  If you want to do loop-back test
   # of HTTP header, use test_loopback_http_response instead.
   attr_reader :test_loopback_response
@@ -413,11 +413,11 @@
     @test_loopback_response = []
     @session_manager = SessionManager.new(self)
     @session_manager.agent_name = agent_name || DEFAULT_AGENT_NAME
     @session_manager.from = from
     @session_manager.ssl_config = @ssl_config = SSLConfig.new(self)
-    @cookie_manager = WebAgent::CookieManager.new
+    @cookie_manager = CookieManager.new
     @follow_redirect_count = 10
     load_environment
     self.proxy = proxy if proxy
     keep_webmock_compat
   end
@@ -574,11 +574,10 @@
     reset_all
   end
 
   # Try to save Cookies to the file specified in set_cookie_store.  Unexpected
   # error will be raised if you don't call set_cookie_store first.
-  # (interface mismatch between WebAgent::CookieManager implementation)
   def save_cookie_store
     @cookie_manager.save_cookies
   end
 
   # Returns stored cookies.
@@ -925,10 +924,15 @@
   end
 
 private
 
   class RetryableResponse < StandardError # :nodoc:
+    attr_reader :res
+
+    def initialize(res = nil)
+      @res = res
+    end
   end
 
   class KeepAliveDisconnected < StandardError # :nodoc:
     attr_reader :sess
     attr_reader :cause
@@ -945,28 +949,41 @@
       # it has any one of the key
       key.all? { |e| args[0].key?(e) }
   end
 
   def do_request(method, uri, query, body, header, &block)
-    conn = Connection.new
     res = nil
     if HTTP::Message.file?(body)
       pos = body.pos rescue nil
     end
     retry_count = @session_manager.protocol_retry_count
     proxy = no_proxy?(uri) ? nil : @proxy
+    previous_request = previous_response = nil
     while retry_count > 0
       body.pos = pos if pos
       req = create_request(method, uri, query, body, header)
+      if previous_request
+        # to remember IO positions to read
+        req.http_body.positions = previous_request.http_body.positions
+      end
       begin
         protect_keep_alive_disconnected do
-          do_get_block(req, proxy, conn, &block)
+          # TODO: remove Connection.new
+          # We want to delete Connection usage in do_get_block but Newrelic gem depends on it.
+          # https://github.com/newrelic/rpm/blob/master/lib/new_relic/agent/instrumentation/httpclient.rb#L34-L36
+          conn = Connection.new
+          res = do_get_block(req, proxy, conn, &block)
+          # Webmock's do_get_block returns ConditionVariable
+          if !res.respond_to?(:previous)
+            res = conn.pop
+          end
         end
-        res = conn.pop
+        res.previous = previous_response
         break
-      rescue RetryableResponse
-        res = conn.pop
+      rescue RetryableResponse => e
+        previous_request = req
+        previous_response = res = e.res
         retry_count -= 1
       end
     end
     res
   end
@@ -1027,19 +1044,25 @@
     end
     if HTTP::Message.file?(body)
       pos = body.pos rescue nil
     end
     retry_number = 0
+    previous = nil
+    request_query = query
     while retry_number < @follow_redirect_count
       body.pos = pos if pos
-      res = do_request(method, uri, query, body, header, &filtered_block)
+      res = do_request(method, uri, request_query, body, header, &filtered_block)
+      res.previous = previous
       if res.redirect?
         if res.header['location'].empty?
           raise BadResponseError.new("Missing Location header for redirect", res)
         end
         method = :get if res.see_other? # See RFC2616 10.3.4
         uri = urify(@redirect_uri_callback.call(uri, res))
+        # To avoid duped query parameter. 'location' must include query part.
+        request_query = nil
+        previous = res
         retry_number += 1
       else
         return res
       end
     end
@@ -1056,14 +1079,17 @@
 
   def protect_keep_alive_disconnected
     begin
       yield
     rescue KeepAliveDisconnected => e
-      if e.sess
-        @session_manager.invalidate(e.sess.dest)
+      # Force to create new connection
+      Thread.current[:HTTPClient_AcquireNewConnection] = true
+      begin
+        yield
+      ensure
+        Thread.current[:HTTPClient_AcquireNewConnection] = false
       end
-      yield
     end
   end
 
   def create_request(method, uri, query, body, header)
     method = method.to_s.upcase
@@ -1099,12 +1125,15 @@
     end
     req = HTTP::Message.new_request(method, uri, query, body, boundary)
     header.each do |key, value|
       req.header.add(key.to_s, value)
     end
-    if @cookie_manager && cookie = @cookie_manager.find(uri)
-      req.header.add('Cookie', cookie)
+    if @cookie_manager
+      cookie_value = @cookie_manager.cookie_value(uri)
+      if cookie_value
+        req.header.add('Cookie', cookie_value)
+      end
     end
     req
   end
 
   def create_boundary
@@ -1150,12 +1179,13 @@
     @request_filter.each do |filter|
       filter.filter_request(req)
     end
     if str = @test_loopback_response.shift
       dump_dummy_request_response(req.http_body.dump, str) if @debug_dev
-      conn.push(HTTP::Message.new_response(str, req.header))
-      return
+      res = HTTP::Message.new_response(str, req.header)
+      conn.push(res)
+      return res
     end
     content = block ? nil : ''
     res = HTTP::Message.new_response(content, req.header)
     @debug_dev << "= Request\n\n" if @debug_dev
     sess = @session_manager.query(req, proxy)
@@ -1176,12 +1206,13 @@
     @session_manager.keep(sess) unless sess.closed?
     commands = @request_filter.collect { |filter|
       filter.filter_response(req, res)
     }
     if commands.find { |command| command == :retry }
-      raise RetryableResponse.new
+      raise RetryableResponse.new(res)
     end
+    res
   end
 
   def do_get_stream(req, proxy, conn)
     @request_filter.each do |filter|
       filter.filter_request(req)
@@ -1207,9 +1238,10 @@
     @session_manager.keep(sess) unless sess.closed?
     _ = @request_filter.collect { |filter|
       filter.filter_response(req, res)
     }
     # ignore commands (not retryable in async mode)
+    res
   end
 
   def do_get_header(req, res, sess)
     res.http_version, res.status, res.reason, headers = sess.get_header
     res.header.set_headers(headers)