Merge branch 'master' of ssh://github.com/urllib3/urllib3 into delete-makefiledelete-makefile

14 files changed, 109 insertions, 40 deletions

diff --git a/.travis.yml b/.travis.yml
index c803f488..f2835b58 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -105,6 +105,7 @@ matrix:
         - ./_travis/deploy.sh
 
   allow_failures:
+    - python: 3.8-dev
     - python: pypy3.5-6.0
     - python: pypy2.7-6.0
 
diff --git a/CHANGES.rst b/CHANGES.rst
index a7724471..55a9c0c9 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -1,6 +1,15 @@
 Changes
 =======
 
+dev (master)
+------------
+
+* Propagate Retry-After header settings to subsequent retries. (Pull #1607)
+
+* Fix edge case where Retry-After header was still respected even when
+  explicitly opted out of. (Pull #1607)
+
+
 1.25.3 (2019-05-23)
 -------------------
 
diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt
index 3da291e5..6f0362c3 100644
--- a/CONTRIBUTORS.txt
+++ b/CONTRIBUTORS.txt
@@ -275,5 +275,8 @@ In chronological order:
 * Katsuhiko YOSHIDA <https://github.com/kyoshidajp>
   * Remove Authorization header regardless of case when redirecting to cross-site
 
+* James Meickle <https://permadeath.com/>
+  * Improve handling of Retry-After header
+
 * [Your name or handle] <[email or website]>
   * [Brief summary of your changes]
diff --git a/docs/user-guide.rst b/docs/user-guide.rst
index 36081e09..bc77d547 100644
--- a/docs/user-guide.rst
+++ b/docs/user-guide.rst
@@ -302,8 +302,8 @@ to the standard-library :mod:`ssl` module. You may experience
 Using timeouts
 --------------
 
-Timeouts allow you to control how long requests are allowed to run before
-being aborted. In simple cases, you can specify a timeout as a ``float``
+Timeouts allow you to control how long (in seconds) requests are allowed to run
+before being aborted. In simple cases, you can specify a timeout as a ``float``
 to :meth:`~poolmanager.PoolManager.request`::
 
     >>> http.request(
diff --git a/setup.cfg b/setup.cfg
index dbd367b8..16d64d20 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -1,4 +1,5 @@
 [flake8]
+ignore = E501, E203, W503, W504
 exclude=./docs/conf.py,./src/urllib3/packages/*
 max-line-length=99
 
diff --git a/src/urllib3/connection.py b/src/urllib3/connection.py
index f0396f25..27bc1867 100644
--- a/src/urllib3/connection.py
+++ b/src/urllib3/connection.py
@@ -412,9 +412,9 @@ class VerifiedHTTPSConnection(HTTPSConnection):
             if not cert.get("subjectAltName", ()):
                 warnings.warn(
                     (
-                        "Certificate for {0} has no `subjectAltName`, falling back to check for a "  # noqa
-                        "`commonName` for now. This feature is being removed by major browsers and "  # noqa
-                        "deprecated by RFC 2818. (See https://github.com/shazow/urllib3/issues/497 "  # noqa
+                        "Certificate for {0} has no `subjectAltName`, falling back to check for a "
+                        "`commonName` for now. This feature is being removed by major browsers and "
+                        "deprecated by RFC 2818. (See https://github.com/shazow/urllib3/issues/497 "
                         "for details.)".format(hostname)
                     ),
                     SubjectAltNameWarning,
diff --git a/src/urllib3/contrib/pyopenssl.py b/src/urllib3/contrib/pyopenssl.py
index 58ea971e..3051ef3a 100644
--- a/src/urllib3/contrib/pyopenssl.py
+++ b/src/urllib3/contrib/pyopenssl.py
@@ -187,7 +187,7 @@ def _dnsname_to_stdlib(name):
         try:
             for prefix in [u"*.", u"."]:
                 if name.startswith(prefix):
-                    name = name[len(prefix) :]  # noqa
+                    name = name[len(prefix) :]
                     return prefix.encode("ascii") + idna.encode(name)
             return idna.encode(name)
         except idna.core.IDNAError:
@@ -349,7 +349,7 @@ class WrappedSocket(object):
         total_sent = 0
         while total_sent < len(data):
             sent = self._send_until_done(
-                data[total_sent : total_sent + SSL_WRITE_BLOCKSIZE]  # noqa
+                data[total_sent : total_sent + SSL_WRITE_BLOCKSIZE]
             )
             total_sent += sent
 
diff --git a/src/urllib3/contrib/securetransport.py b/src/urllib3/contrib/securetransport.py
index 7b975c0d..24e6b5c4 100644
--- a/src/urllib3/contrib/securetransport.py
+++ b/src/urllib3/contrib/securetransport.py
@@ -616,9 +616,7 @@ class WrappedSocket(object):
     def sendall(self, data):
         total_sent = 0
         while total_sent < len(data):
-            sent = self.send(
-                data[total_sent : total_sent + SSL_WRITE_BLOCKSIZE]  # noqa
-            )
+            sent = self.send(data[total_sent : total_sent + SSL_WRITE_BLOCKSIZE])
             total_sent += sent
 
     def shutdown(self):
diff --git a/src/urllib3/util/retry.py b/src/urllib3/util/retry.py
index c26f48c9..5a049fe6 100644
--- a/src/urllib3/util/retry.py
+++ b/src/urllib3/util/retry.py
@@ -210,6 +210,7 @@ class Retry(object):
             raise_on_status=self.raise_on_status,
             history=self.history,
             remove_headers_on_redirect=self.remove_headers_on_redirect,
+            respect_retry_after_header=self.respect_retry_after_header,
         )
         params.update(kw)
         return type(self)(**params)
@@ -294,7 +295,7 @@ class Retry(object):
         this method will return immediately.
         """
 
-        if response:
+        if self.respect_retry_after_header and response:
             slept = self.sleep_for_retry(response)
             if slept:
                 return
diff --git a/src/urllib3/util/timeout.py b/src/urllib3/util/timeout.py
index 6623bbad..c1dc1e97 100644
--- a/src/urllib3/util/timeout.py
+++ b/src/urllib3/util/timeout.py
@@ -46,19 +46,20 @@ class Timeout(object):
     :type total: integer, float, or None
 
     :param connect:
-        The maximum amount of time to wait for a connection attempt to a server
-        to succeed. Omitting the parameter will default the connect timeout to
-        the system default, probably `the global default timeout in socket.py
+        The maximum amount of time (in seconds) to wait for a connection
+        attempt to a server to succeed. Omitting the parameter will default the
+        connect timeout to the system default, probably `the global default
+        timeout in socket.py
         <http://hg.python.org/cpython/file/603b4d593758/Lib/socket.py#l535>`_.
         None will set an infinite timeout for connection attempts.
 
     :type connect: integer, float, or None
 
     :param read:
-        The maximum amount of time to wait between consecutive
-        read operations for a response from the server. Omitting
-        the parameter will default the read timeout to the system
-        default, probably `the global default timeout in socket.py
+        The maximum amount of time (in seconds) to wait between consecutive
+        read operations for a response from the server. Omitting the parameter
+        will default the read timeout to the system default, probably `the
+        global default timeout in socket.py
         <http://hg.python.org/cpython/file/603b4d593758/Lib/socket.py#l535>`_.
         None will set an infinite timeout.
 
@@ -195,7 +196,7 @@ class Timeout(object):
     def get_connect_duration(self):
         """ Gets the time elapsed since the call to :meth:`start_connect`.
 
-        :return: Elapsed time.
+        :return: Elapsed time in seconds.
         :rtype: float
         :raises urllib3.exceptions.TimeoutStateError: if you attempt
             to get duration for a timer that hasn't been started.
diff --git a/src/urllib3/util/url.py b/src/urllib3/util/url.py
index 2ba45360..f225cd8b 100644
--- a/src/urllib3/util/url.py
+++ b/src/urllib3/util/url.py
@@ -149,7 +149,7 @@ def split_first(s, delims):
     if min_idx is None or min_idx < 0:
         return s, "", None
 
-    return s[:min_idx], s[min_idx + 1 :], min_delim  # noqa
+    return s[:min_idx], s[min_idx + 1 :], min_delim
 
 
 def _encode_invalid_chars(component, allowed_chars, encoding="utf-8"):
@@ -173,7 +173,7 @@ def _encode_invalid_chars(component, allowed_chars, encoding="utf-8"):
 
     for i in range(0, len(uri_bytes)):
         # Will return a single character bytestring on both Python 2 & 3
-        byte = uri_bytes[i : i + 1]  # noqa
+        byte = uri_bytes[i : i + 1]
         byte_ord = ord(byte)
         if (is_percent_encoded and byte == b"%") or (
             byte_ord < 128 and byte.decode() in allowed_chars
diff --git a/test/test_response.py b/test/test_response.py
index 18b3e373..23a63be1 100644
--- a/test/test_response.py
+++ b/test/test_response.py
@@ -440,7 +440,7 @@ class TestResponse(object):
 
             def __init__(self, payload, payload_part_size):
                 self.payloads = [
-                    payload[i * payload_part_size : (i + 1) * payload_part_size]  # noqa
+                    payload[i * payload_part_size : (i + 1) * payload_part_size]
                     for i in range(NUMBER_OF_READS + 1)
                 ]
 
@@ -633,7 +633,7 @@ class TestResponse(object):
             data = compress.compress(b"foobar")
             data += compress.flush()
             for i in range(0, len(data), 2):
-                yield data[i : i + 2]  # noqa
+                yield data[i : i + 2]
 
         fp = MockChunkedEncodingResponse(list(stream()))
         r = httplib.HTTPResponse(MockSock)
@@ -801,7 +801,7 @@ class TestResponse(object):
             data = compress.compress(b"foo\nbar")
             data += compress.flush()
             for i in range(0, len(data), 2):
-                yield data[i : i + 2]  # noqa
+                yield data[i : i + 2]
 
         fp = MockChunkedEncodingResponse(list(stream()))
         r = httplib.HTTPResponse(MockSock)
@@ -862,7 +862,7 @@ class MockChunkedEncodingResponse(object):
                 return b""
             else:
                 chunk_part = self.cur_chunk[: i + 2]
-                self.cur_chunk = self.cur_chunk[i + 2 :]  # noqa
+                self.cur_chunk = self.cur_chunk[i + 2 :]
                 return chunk_part
         elif amt <= -1:
             chunk_part = self.cur_chunk
diff --git a/test/test_retry.py b/test/test_retry.py
index 7b228d75..c36476f8 100644
--- a/test/test_retry.py
+++ b/test/test_retry.py
@@ -1,10 +1,15 @@
+import datetime
+import mock
 import pytest
+import time
 
 from urllib3.response import HTTPResponse
+from urllib3.packages import six
 from urllib3.packages.six.moves import xrange
 from urllib3.util.retry import Retry, RequestHistory
 from urllib3.exceptions import (
     ConnectTimeoutError,
+    InvalidHeader,
     MaxRetryError,
     ReadTimeoutError,
     ResponseError,
@@ -271,3 +276,68 @@ class TestRetry(object):
         retry = Retry(remove_headers_on_redirect=["X-API-Secret"])
 
         assert list(retry.remove_headers_on_redirect) == ["x-api-secret"]
+
+    @pytest.mark.parametrize("value", ["-1", "+1", "1.0", six.u("\xb2")])  # \xb2 = ^2
+    def test_parse_retry_after_invalid(self, value):
+        retry = Retry()
+        with pytest.raises(InvalidHeader):
+            retry.parse_retry_after(value)
+
+    @pytest.mark.parametrize(
+        "value, expected", [("0", 0), ("1000", 1000), ("\t42 ", 42)]
+    )
+    def test_parse_retry_after(self, value, expected):
+        retry = Retry()
+        assert retry.parse_retry_after(value) == expected
+
+    @pytest.mark.parametrize("respect_retry_after_header", [True, False])
+    def test_respect_retry_after_header_propagated(self, respect_retry_after_header):
+
+        retry = Retry(respect_retry_after_header=respect_retry_after_header)
+        new_retry = retry.new()
+        assert new_retry.respect_retry_after_header == respect_retry_after_header
+
+    @pytest.mark.parametrize(
+        "retry_after_header,respect_retry_after_header,sleep_duration",
+        [
+            ("3600", True, 3600),
+            ("3600", False, None),
+            # Will sleep due to header is 1 hour in future
+            ("Mon, 3 Jun 2019 12:00:00 UTC", True, 3600),
+            # Won't sleep due to not respecting header
+            ("Mon, 3 Jun 2019 12:00:00 UTC", False, None),
+            # Won't sleep due to current time reached
+            ("Mon, 3 Jun 2019 11:00:00 UTC", True, None),
+            # Won't sleep due to current time reached + not respecting header
+            ("Mon, 3 Jun 2019 11:00:00 UTC", False, None),
+        ],
+    )
+    def test_respect_retry_after_header_sleep(
+        self, retry_after_header, respect_retry_after_header, sleep_duration
+    ):
+        retry = Retry(respect_retry_after_header=respect_retry_after_header)
+
+        # Date header syntax can specify an absolute date; compare this to the
+        # time in the parametrized inputs above.
+        current_time = mock.MagicMock(
+            return_value=time.mktime(
+                datetime.datetime(year=2019, month=6, day=3, hour=11).timetuple()
+            )
+        )
+
+        with mock.patch("time.sleep") as sleep_mock, mock.patch(
+            "time.time", current_time
+        ):
+            # for the default behavior, it must be in RETRY_AFTER_STATUS_CODES
+            response = HTTPResponse(
+                status=503, headers={"Retry-After": retry_after_header}
+            )
+
+            retry.sleep(response)
+
+            # The expected behavior is that we'll only sleep if respecting
+            # this header (since we won't have any backoff sleep attempts)
+            if respect_retry_after_header and sleep_duration is not None:
+                sleep_mock.assert_called_with(sleep_duration)
+            else:
+                sleep_mock.assert_not_called()
diff --git a/test/test_util.py b/test/test_util.py
index b2906f22..37c09ab2 100644
--- a/test/test_util.py
+++ b/test/test_util.py
@@ -13,7 +13,6 @@ import pytest
 from urllib3 import add_stderr_logger, disable_warnings
 from urllib3.util.request import make_headers, rewind_body, _FAILEDTELL
 from urllib3.util.response import assert_header_parsing
-from urllib3.util.retry import Retry
 from urllib3.util.timeout import Timeout
 from urllib3.util.url import get_host, parse_url, split_first, Url
 from urllib3.util.ssl_ import (
@@ -27,7 +26,6 @@ from urllib3.exceptions import (
     TimeoutStateError,
     InsecureRequestWarning,
     SNIMissingWarning,
-    InvalidHeader,
     UnrewindableBodyError,
 )
 from urllib3.util.connection import allowed_gai_family, _has_ipv6
@@ -782,19 +780,6 @@ class TestUtil(object):
         with patch("urllib3.util.connection.HAS_IPV6", False):
             assert allowed_gai_family() == socket.AF_INET
 
-    @pytest.mark.parametrize("value", ["-1", "+1", "1.0", six.u("\xb2")])  # \xb2 = ^2
-    def test_parse_retry_after_invalid(self, value):
-        retry = Retry()
-        with pytest.raises(InvalidHeader):
-            retry.parse_retry_after(value)
-
-    @pytest.mark.parametrize(
-        "value, expected", [("0", 0), ("1000", 1000), ("\t42 ", 42)]
-    )
-    def test_parse_retry_after(self, value, expected):
-        retry = Retry()
-        assert retry.parse_retry_after(value) == expected
-
     @pytest.mark.parametrize("headers", [b"foo", None, object])
     def test_assert_header_parsing_throws_typeerror_with_non_headers(self, headers):
         with pytest.raises(TypeError):