Upgrade python/httplib2 to v0.17.0

Test: None
Change-Id: I4189cf870ba80629316fbe58c56b996fa0a082d5

10 files changed, 170 insertions, 48 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 07fb949..1dee4c4 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,3 +1,18 @@
+0.17.0
+
+  feature: Http().redirect_codes set, works after follow(_all)_redirects check
+  This allows one line workaround for old gcloud library that uses 308
+  response without redirect semantics.
+  https://github.com/httplib2/httplib2/issues/156
+
+0.16.0
+
+  IMPORTANT cache invalidation change, fix 307 keep method, add 308 Redirects
+  https://github.com/httplib2/httplib2/issues/151
+
+  proxy: username/password as str compatible with pysocks
+  https://github.com/httplib2/httplib2/issues/154
+
 0.15.0
 
   python2: regression in connect() error handling
diff --git a/METADATA b/METADATA
index 8b3c1c6..605407f 100644
--- a/METADATA
+++ b/METADATA
@@ -9,10 +9,10 @@ third_party {
     type: GIT
     value: "https://github.com/httplib2/httplib2.git"
   }
-  version: "v0.15.0"
+  version: "v0.17.0"
   last_upgrade_date {
-    year: 2019
-    month: 12
-    day: 23
+    year: 2020
+    month: 2
+    day: 1
   }
 }
diff --git a/python2/httplib2/__init__.py b/python2/httplib2/__init__.py
index c8302eb..f32accf 100644
--- a/python2/httplib2/__init__.py
+++ b/python2/httplib2/__init__.py
@@ -19,7 +19,7 @@ __contributors__ = [
     "Alex Yu",
 ]
 __license__ = "MIT"
-__version__ = '0.15.0'
+__version__ = '0.17.0'
 
 import base64
 import calendar
@@ -291,6 +291,12 @@ HOP_BY_HOP = [
     "upgrade",
 ]
 
+# https://tools.ietf.org/html/rfc7231#section-8.1.3
+SAFE_METHODS = ("GET", "HEAD")  # TODO add "OPTIONS", "TRACE"
+
+# To change, assign to `Http().redirect_codes`
+REDIRECT_CODES = frozenset((300, 301, 302, 303, 307, 308))
+
 
 def _get_end2end_headers(response):
     hopbyhop = list(HOP_BY_HOP)
@@ -1175,9 +1181,9 @@ class HTTPConnectionWithTimeout(httplib.HTTPConnection):
 
             host = self.host
             port = self.port
-        
+
         socket_err = None
-        
+
         for res in socket.getaddrinfo(host, port, 0, socket.SOCK_STREAM):
             af, socktype, proto, canonname, sa = res
             try:
@@ -1353,9 +1359,9 @@ class HTTPSConnectionWithTimeout(httplib.HTTPSConnection):
 
             host = self.host
             port = self.port
-            
+
         socket_err = None
-        
+
         address_info = socket.getaddrinfo(host, port, 0, socket.SOCK_STREAM)
         for family, socktype, proto, canonname, sockaddr in address_info:
             try:
@@ -1661,10 +1667,14 @@ class Http(object):
         # If set to False then no redirects are followed, even safe ones.
         self.follow_redirects = True
 
+        self.redirect_codes = REDIRECT_CODES
+
         # Which HTTP methods do we apply optimistic concurrency to, i.e.
         # which methods get an "if-match:" etag header added to them.
         self.optimistic_concurrency_methods = ["PUT", "PATCH"]
 
+        self.safe_methods = list(SAFE_METHODS)
+
         # If 'follow_redirects' is True, and this is set to True then
         # all redirecs are followed, including unsafe ones.
         self.follow_all_redirects = False
@@ -1858,10 +1868,10 @@ class Http(object):
 
         if (
             self.follow_all_redirects
-            or (method in ["GET", "HEAD"])
-            or response.status == 303
+            or method in self.safe_methods
+            or response.status in (303, 308)
         ):
-            if self.follow_redirects and response.status in [300, 301, 302, 303, 307]:
+            if self.follow_redirects and response.status in self.redirect_codes:
                 # Pick out the location header and basically start from the beginning
                 # remembering first to strip the ETag header and decrement our 'depth'
                 if redirections:
@@ -1881,7 +1891,7 @@ class Http(object):
                             response["location"] = urlparse.urljoin(
                                 absolute_uri, location
                             )
-                    if response.status == 301 and method in ["GET", "HEAD"]:
+                    if response.status == 308 or (response.status == 301 and method in self.safe_methods):
                         response["-x-permanent-redirect-url"] = response["location"]
                         if "content-location" not in response:
                             response["content-location"] = absolute_uri
@@ -1918,7 +1928,7 @@ class Http(object):
                         response,
                         content,
                     )
-            elif response.status in [200, 203] and method in ["GET", "HEAD"]:
+            elif response.status in [200, 203] and method in self.safe_methods:
                 # Don't cache 206's since we aren't going to handle byte range requests
                 if "content-location" not in response:
                     response["content-location"] = absolute_uri
@@ -2018,6 +2028,7 @@ class Http(object):
                 headers["accept-encoding"] = "gzip, deflate"
 
             info = email.Message.Message()
+            cachekey = None
             cached_value = None
             if self.cache:
                 cachekey = defrag_uri.encode("utf-8")
@@ -2038,8 +2049,6 @@ class Http(object):
                         self.cache.delete(cachekey)
                         cachekey = None
                         cached_value = None
-            else:
-                cachekey = None
 
             if (
                 method in self.optimistic_concurrency_methods
@@ -2051,13 +2060,15 @@ class Http(object):
                 # http://www.w3.org/1999/04/Editing/
                 headers["if-match"] = info["etag"]
 
-            if method not in ["GET", "HEAD"] and self.cache and cachekey:
-                # RFC 2616 Section 13.10
+            # https://tools.ietf.org/html/rfc7234
+            # A cache MUST invalidate the effective Request URI as well as [...] Location and Content-Location
+            # when a non-error status code is received in response to an unsafe request method.
+            if self.cache and cachekey and method not in self.safe_methods:
                 self.cache.delete(cachekey)
 
             # Check the vary header in the cache to see if this request
             # matches what varies in the cache.
-            if method in ["GET", "HEAD"] and "vary" in info:
+            if method in self.safe_methods and "vary" in info:
                 vary = info["vary"]
                 vary_headers = vary.lower().replace(" ", "").split(",")
                 for header in vary_headers:
@@ -2068,11 +2079,14 @@ class Http(object):
                         break
 
             if (
-                cached_value
-                and method in ["GET", "HEAD"]
-                and self.cache
+                self.cache
+                and cached_value
+                and (method in self.safe_methods or info["status"] == "308")
                 and "range" not in headers
             ):
+                redirect_method = method
+                if info["status"] not in ("307", "308"):
+                    redirect_method = "GET"
                 if "-x-permanent-redirect-url" in info:
                     # Should cached permanent redirects be counted in our redirection count? For now, yes.
                     if redirections <= 0:
@@ -2083,7 +2097,7 @@ class Http(object):
                         )
                     (response, new_content) = self.request(
                         info["-x-permanent-redirect-url"],
-                        method="GET",
+                        method=redirect_method,
                         headers=headers,
                         redirections=redirections - 1,
                     )
diff --git a/python2/httplib2/socks.py b/python2/httplib2/socks.py
index 5cef776..71eb4eb 100644
--- a/python2/httplib2/socks.py
+++ b/python2/httplib2/socks.py
@@ -238,7 +238,15 @@ class socksocket(socket.socket):
         headers -     Additional or modified headers for the proxy connect
         request.
         """
-        self.__proxy = (proxytype, addr, port, rdns, username, password, headers)
+        self.__proxy = (
+            proxytype,
+            addr,
+            port,
+            rdns,
+            username.encode() if username else None,
+            password.encode() if password else None,
+            headers,
+        )
 
     def __negotiatesocks5(self, destaddr, destport):
         """__negotiatesocks5(self,destaddr,destport)
diff --git a/python3/httplib2/__init__.py b/python3/httplib2/__init__.py
index d8c3d34..6467c79 100644
--- a/python3/httplib2/__init__.py
+++ b/python3/httplib2/__init__.py
@@ -15,7 +15,7 @@ __contributors__ = [
     "Alex Yu",
 ]
 __license__ = "MIT"
-__version__ = '0.15.0'
+__version__ = '0.17.0'
 
 import base64
 import calendar
@@ -161,6 +161,13 @@ HOP_BY_HOP = [
     "upgrade",
 ]
 
+# https://tools.ietf.org/html/rfc7231#section-8.1.3
+SAFE_METHODS = ("GET", "HEAD", "OPTIONS", "TRACE")
+
+# To change, assign to `Http().redirect_codes`
+REDIRECT_CODES = frozenset((300, 301, 302, 303, 307, 308))
+
+
 from httplib2 import certs
 CA_CERTS = certs.where()
 
@@ -315,7 +322,7 @@ def _parse_cache_control(headers):
 # Whether to use a strict mode to parse WWW-Authenticate headers
 # Might lead to bad results in case of ill-formed header value,
 # so disabled by default, falling back to relaxed parsing.
-# Set to true to turn on, usefull for testing servers.
+# Set to true to turn on, useful for testing servers.
 USE_WWW_AUTH_STRICT_PARSING = 0
 
 # In regex below:
@@ -1004,10 +1011,10 @@ class ProxyInfo(object):
           proxy_headers: Additional or modified headers for the proxy connect
           request.
         """
-        if isinstance(proxy_user, str):
-            proxy_user = proxy_user.encode()
-        if isinstance(proxy_pass, str):
-            proxy_pass = proxy_pass.encode()
+        if isinstance(proxy_user, bytes):
+            proxy_user = proxy_user.decode()
+        if isinstance(proxy_pass, bytes):
+            proxy_pass = proxy_pass.decode()
         self.proxy_type, self.proxy_host, self.proxy_port, self.proxy_rdns, self.proxy_user, self.proxy_pass, self.proxy_headers = (
             proxy_type,
             proxy_host,
@@ -1467,10 +1474,14 @@ class Http(object):
         # If set to False then no redirects are followed, even safe ones.
         self.follow_redirects = True
 
+        self.redirect_codes = REDIRECT_CODES
+
         # Which HTTP methods do we apply optimistic concurrency to, i.e.
         # which methods get an "if-match:" etag header added to them.
         self.optimistic_concurrency_methods = ["PUT", "PATCH"]
 
+        self.safe_methods = list(SAFE_METHODS)
+
         # If 'follow_redirects' is True, and this is set to True then
         # all redirecs are followed, including unsafe ones.
         self.follow_all_redirects = False
@@ -1663,10 +1674,10 @@ class Http(object):
 
         if (
             self.follow_all_redirects
-            or (method in ["GET", "HEAD"])
-            or response.status == 303
+            or method in self.safe_methods
+            or response.status in (303, 308)
         ):
-            if self.follow_redirects and response.status in [300, 301, 302, 303, 307]:
+            if self.follow_redirects and response.status in self.redirect_codes:
                 # Pick out the location header and basically start from the beginning
                 # remembering first to strip the ETag header and decrement our 'depth'
                 if redirections:
@@ -1686,7 +1697,7 @@ class Http(object):
                             response["location"] = urllib.parse.urljoin(
                                 absolute_uri, location
                             )
-                    if response.status == 301 and method in ["GET", "HEAD"]:
+                    if response.status == 308 or (response.status == 301 and (method in self.safe_methods)):
                         response["-x-permanent-redirect-url"] = response["location"]
                         if "content-location" not in response:
                             response["content-location"] = absolute_uri
@@ -1723,7 +1734,7 @@ class Http(object):
                         response,
                         content,
                     )
-            elif response.status in [200, 203] and method in ["GET", "HEAD"]:
+            elif response.status in [200, 203] and method in self.safe_methods:
                 # Don't cache 206's since we aren't going to handle byte range requests
                 if "content-location" not in response:
                     response["content-location"] = absolute_uri
@@ -1822,6 +1833,7 @@ a string that contains the response entity body.
                 headers["accept-encoding"] = "gzip, deflate"
 
             info = email.message.Message()
+            cachekey = None
             cached_value = None
             if self.cache:
                 cachekey = defrag_uri
@@ -1839,8 +1851,6 @@ a string that contains the response entity body.
                         self.cache.delete(cachekey)
                         cachekey = None
                         cached_value = None
-            else:
-                cachekey = None
 
             if (
                 method in self.optimistic_concurrency_methods
@@ -1852,13 +1862,15 @@ a string that contains the response entity body.
                 # http://www.w3.org/1999/04/Editing/
                 headers["if-match"] = info["etag"]
 
-            if method not in ["GET", "HEAD"] and self.cache and cachekey:
-                # RFC 2616 Section 13.10
+            # https://tools.ietf.org/html/rfc7234
+            # A cache MUST invalidate the effective Request URI as well as [...] Location and Content-Location
+            # when a non-error status code is received in response to an unsafe request method.
+            if self.cache and cachekey and method not in self.safe_methods:
                 self.cache.delete(cachekey)
 
             # Check the vary header in the cache to see if this request
             # matches what varies in the cache.
-            if method in ["GET", "HEAD"] and "vary" in info:
+            if method in self.safe_methods and "vary" in info:
                 vary = info["vary"]
                 vary_headers = vary.lower().replace(" ", "").split(",")
                 for header in vary_headers:
@@ -1869,11 +1881,14 @@ a string that contains the response entity body.
                         break
 
             if (
-                cached_value
-                and method in ["GET", "HEAD"]
-                and self.cache
+                self.cache
+                and cached_value
+                and (method in self.safe_methods or info["status"] == "308")
                 and "range" not in headers
             ):
+                redirect_method = method
+                if info["status"] not in ("307", "308"):
+                    redirect_method = "GET"
                 if "-x-permanent-redirect-url" in info:
                     # Should cached permanent redirects be counted in our redirection count? For now, yes.
                     if redirections <= 0:
@@ -1884,7 +1899,7 @@ a string that contains the response entity body.
                         )
                     (response, new_content) = self.request(
                         info["-x-permanent-redirect-url"],
-                        method="GET",
+                        method=redirect_method,
                         headers=headers,
                         redirections=redirections - 1,
                     )
diff --git a/python3/httplib2/socks.py b/python3/httplib2/socks.py
index 2926b4e..cc68e63 100644
--- a/python3/httplib2/socks.py
+++ b/python3/httplib2/socks.py
@@ -238,7 +238,15 @@ class socksocket(socket.socket):
         headers -     Additional or modified headers for the proxy connect
         request.
         """
-        self.__proxy = (proxytype, addr, port, rdns, username, password, headers)
+        self.__proxy = (
+            proxytype,
+            addr,
+            port,
+            rdns,
+            username.encode() if username else None,
+            password.encode() if password else None,
+            headers,
+        )
 
     def __negotiatesocks5(self, destaddr, destport):
         """__negotiatesocks5(self,destaddr,destport)
diff --git a/setup.py b/setup.py
index 33c8827..a3be8d4 100755
--- a/setup.py
+++ b/setup.py
@@ -4,7 +4,7 @@ import setuptools.command.test
 import sys
 
 pkgdir = {"": "python%s" % sys.version_info[0]}
-VERSION = '0.15.0'
+VERSION = '0.17.0'
 
 
 # `python setup.py test` uses existing Python environment, no virtualenv, no pip.
diff --git a/tests/__init__.py b/tests/__init__.py
index 496652b..a15db9e 100644
--- a/tests/__init__.py
+++ b/tests/__init__.py
@@ -406,10 +406,12 @@ def server_request(request_handler, **kwargs):
             request = HttpRequest.from_buffered(buf)
             if request is None:
                 break
+            # print("--- debug request\n" + request.raw.decode("ascii", "replace"))
             i += 1
             request.client_sock = sock
             request.number = i
             response = request_handler(request=request)
+            # print("--- debug response\n" + response.decode("ascii", "replace"))
             sock.sendall(response)
             request.client_sock = None
             if not tick(request):
diff --git a/tests/test_http.py b/tests/test_http.py
index 97b52dc..df99016 100644
--- a/tests/test_http.py
+++ b/tests/test_http.py
@@ -622,6 +622,57 @@ def test_get_307():
         assert content == b"final content\n"
 
 
+def test_post_307():
+    # 307: follow with same method
+    http = httplib2.Http(cache=tests.get_cache_path(), timeout=1)
+    http.follow_all_redirects = True
+    r307 = tests.http_response_bytes(status=307, headers={"location": "/final"})
+    r200 = tests.http_response_bytes(status=200, body=b"final content\n")
+
+    with tests.server_list_http([r307, r200, r307, r200]) as uri:
+        response, content = http.request(uri, "POST")
+        assert response.previous.status == 307
+        assert not response.previous.fromcache
+        assert response.status == 200
+        assert not response.fromcache
+        assert content == b"final content\n"
+
+        response, content = http.request(uri, "POST")
+        assert response.previous.status == 307
+        assert not response.previous.fromcache
+        assert response.status == 200
+        assert not response.fromcache
+        assert content == b"final content\n"
+
+
+def test_change_308():
+    # 308: follow with same method, cache redirect
+    http = httplib2.Http(cache=tests.get_cache_path(), timeout=1)
+    routes = {
+        "/final": tests.make_http_reflect(),
+        "": tests.http_response_bytes(
+            status="308 Permanent Redirect",
+            add_date=True,
+            headers={"cache-control": "max-age=300", "location": "/final"},
+        ),
+    }
+
+    with tests.server_route(routes, request_count=3) as uri:
+        response, content = http.request(uri, "CHANGE", body=b"hello308")
+        assert response.previous.status == 308
+        assert not response.previous.fromcache
+        assert response.status == 200
+        assert not response.fromcache
+        assert content.startswith(b"CHANGE /final HTTP")
+
+        response, content = http.request(uri, "CHANGE")
+        assert response.previous.status == 308
+        assert response.previous.fromcache
+        assert response.status == 200
+        assert not response.fromcache
+        assert content.startswith(b"CHANGE /final HTTP")
+
+
 def test_get_410():
     # Test that we pass 410's through
     http = httplib2.Http()
@@ -643,3 +694,12 @@ content"""
         assert response.status == 200
         assert content == b"content"
         assert response["link"], "link1, link2"
+
+
+def test_custom_redirect_codes():
+    http = httplib2.Http()
+    http.redirect_codes = set([300])
+    with tests.server_const_http(status=301, request_count=1) as uri:
+        response, content = http.request(uri, "GET")
+        assert response.status == 301
+        assert response.previous is None
diff --git a/tests/test_proxy.py b/tests/test_proxy.py
index 375367f..4ec8aea 100644
--- a/tests/test_proxy.py
+++ b/tests/test_proxy.py
@@ -32,8 +32,8 @@ def test_from_url_ident():
     pi = httplib2.proxy_info_from_url("http://zoidberg:fish@someproxy:99")
     assert pi.proxy_host == "someproxy"
     assert pi.proxy_port == 99
-    assert pi.proxy_user == b"zoidberg"
-    assert pi.proxy_pass == b"fish"
+    assert pi.proxy_user == "zoidberg"
+    assert pi.proxy_pass == "fish"
 
 
 def test_from_env():