From 01552757a0473fe15d2b8b0175843a6dfd74d751 Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Sat, 31 Oct 2020 02:34:41 +0000 Subject: [PATCH] supporting the retry-after header for redirections as well --- lib/httpx.rb | 1 + lib/httpx/plugins/follow_redirects.rb | 22 +++++++++++++++++-- lib/httpx/plugins/rate_limiter.rb | 13 ++--------- lib/httpx/utils.rb | 18 +++++++++++++++ .../requests/plugins/follow_redirects.rb | 13 +++++++++++ test/support/requests/plugins/rate_limiter.rb | 4 ++-- test/support/session_with_mock_response.rb | 12 +++++----- 7 files changed, 63 insertions(+), 20 deletions(-) create mode 100644 lib/httpx/utils.rb diff --git a/lib/httpx.rb b/lib/httpx.rb index b5897063..0e28fce0 100644 --- a/lib/httpx.rb +++ b/lib/httpx.rb @@ -5,6 +5,7 @@ require "httpx/version" require "httpx/extensions" require "httpx/errors" +require "httpx/utils" require "httpx/altsvc" require "httpx/callbacks" require "httpx/loggable" diff --git a/lib/httpx/plugins/follow_redirects.rb b/lib/httpx/plugins/follow_redirects.rb index 1922984c..4189c2fd 100644 --- a/lib/httpx/plugins/follow_redirects.rb +++ b/lib/httpx/plugins/follow_redirects.rb @@ -59,8 +59,26 @@ module HTTPX return ErrorResponse.new(request, error, options) end - connection = find_connection(retry_request, connections, options) - connection.send(retry_request) + retry_after = response.headers["retry-after"] + + if retry_after + # Servers send the "Retry-After" header field to indicate how long the + # user agent ought to wait before making a follow-up request. + # When sent with any 3xx (Redirection) response, Retry-After indicates + # the minimum time that the user agent is asked to wait before issuing + # the redirected request. + # + retry_after = Utils.parse_retry_after(retry_after) + + log { "redirecting after #{retry_after} secs..." } + pool.after(retry_after) do + connection = find_connection(retry_request, connections, options) + connection.send(retry_request) + end + else + connection = find_connection(retry_request, connections, options) + connection.send(retry_request) + end nil end diff --git a/lib/httpx/plugins/rate_limiter.rb b/lib/httpx/plugins/rate_limiter.rb index f9680f92..296946b6 100644 --- a/lib/httpx/plugins/rate_limiter.rb +++ b/lib/httpx/plugins/rate_limiter.rb @@ -35,21 +35,12 @@ module HTTPX # the minimum time that the user agent is asked to wait before issuing # the redirected request. # - # The value of this field can be either an HTTP-date or a number of - # seconds to delay after the response is received. - def retry_after_rate_limit(_request, response) + def retry_after_rate_limit(_, response) retry_after = response.headers["retry-after"] return unless retry_after - begin - # first: bet on it being an integer - Integer(retry_after) - rescue ArgumentError - # Then it's a datetime - time = Time.httpdate(retry_after) - time - Time.now - end + Utils.parse_retry_after(retry_after) end end end diff --git a/lib/httpx/utils.rb b/lib/httpx/utils.rb new file mode 100644 index 00000000..25805f61 --- /dev/null +++ b/lib/httpx/utils.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module HTTPX + module Utils + module_function + + # The value of this field can be either an HTTP-date or a number of + # seconds to delay after the response is received. + def parse_retry_after(retry_after) + # first: bet on it being an integer + Integer(retry_after) + rescue ArgumentError + # Then it's a datetime + time = Time.httpdate(retry_after) + time - Time.now + end + end +end diff --git a/test/support/requests/plugins/follow_redirects.rb b/test/support/requests/plugins/follow_redirects.rb index 7ccd9e9a..3206c545 100644 --- a/test/support/requests/plugins/follow_redirects.rb +++ b/test/support/requests/plugins/follow_redirects.rb @@ -36,6 +36,19 @@ module Requests verify_status(response, 302) end + def test_plugin_follow_redirects_retry_after + session = HTTPX.plugin(SessionWithMockResponse[302, "retry-after" => "2"]).plugin(:follow_redirects) + + before_time = Process.clock_gettime(Process::CLOCK_MONOTONIC, :second) + response = session.get(max_redirect_uri(1)) + after_time = Process.clock_gettime(Process::CLOCK_MONOTONIC, :second) + + verify_status(response, 200) + + total_time = after_time - before_time + assert total_time >= 2, "request didn't take as expected to redirect (#{total_time} secs)" + end + def test_plugin_follow_insecure_no_insecure_downgrade return unless origin.start_with?("https") diff --git a/test/support/requests/plugins/rate_limiter.rb b/test/support/requests/plugins/rate_limiter.rb index b42b3e52..137232b9 100644 --- a/test/support/requests/plugins/rate_limiter.rb +++ b/test/support/requests/plugins/rate_limiter.rb @@ -41,7 +41,7 @@ module Requests verify_rated_responses(rate_limiter_session, 429) total_time = after_time - before_time - assert_in_delta 2, total_time, 1, "request didn't take as expected to retry (#{total_time} secs)" + assert total_time >= 2, "request didn't take as expected to retry (#{total_time} secs)" end def test_plugin_rate_limiter_retry_after_date @@ -57,7 +57,7 @@ module Requests verify_rated_responses(rate_limiter_session, 429) total_time = after_time - before_time - assert_in_delta 3, total_time, 1, "request didn't take as expected to retry (#{total_time} secs)" + assert total_time >= 2, "request didn't take as expected to retry (#{total_time} secs)" end private diff --git a/test/support/session_with_mock_response.rb b/test/support/session_with_mock_response.rb index 7ab4fa26..6af98568 100644 --- a/test/support/session_with_mock_response.rb +++ b/test/support/session_with_mock_response.rb @@ -7,6 +7,10 @@ module SessionWithMockResponse self end + module ResponseMethods + attr_writer :status + end + module InstanceMethods def initialize(*) super @@ -19,11 +23,9 @@ module SessionWithMockResponse response.close @mock_responses_counter -= 1 - mock_response = @options.response_class.new(request, - Thread.current[:httpx_mock_response_status], - "2.0", - Thread.current[:httpx_mock_response_headers]) - super(request, mock_response) + response.status = Thread.current[:httpx_mock_response_status] + response.merge_headers(Thread.current[:httpx_mock_response_headers]) + super(request, response) end end end