Fix Request Redirection Handling (hvac#348)

* simplify chained comparison

* Ensure regression unit test case coverage for paths/redirects

* Revert redirection handling back to the requests module

* Handle double slashes in paths

* Fix syntax for python 2.7

* Log when we transform a requested url

* Explictly assert that we have the expect requests in mocker history

hvac/adapters.py

@@ -235,6 +235,12 @@ def request(self, method, url, headers=None, raise_exception=True, **kwargs):
         :return: The response of the request.
         :rtype: requests.Response
         """
+        if '//' in url:
+            # Vault CLI treats a double forward slash ('//') as a single forward slash for a given path.
+            # To avoid issues with the requests module's redirection logic, we perform the same translation here.
+            logger.warning('Replacing double-slashes ("//") in path with single slash ("/") to avoid Vault redirect response.')
+            url = url.replace('//', '/')
+
         url = self.urljoin(self.base_uri, url)
 
         if not headers:
@@ -253,14 +259,13 @@ def request(self, method, url, headers=None, raise_exception=True, **kwargs):
         _kwargs = self._kwargs.copy()
         _kwargs.update(kwargs)
 
-        response = self.session.request(method, url, headers=headers,
-                                        allow_redirects=False, **_kwargs)
-
-        # NOTE(ianunruh): workaround for https://github.com/hvac/hvac/issues/51
-        while response.is_redirect and self.allow_redirects:
-            url = self.urljoin(self.base_uri, response.headers['Location'])
-            response = self.session.request(method, url, headers=headers,
-                                            allow_redirects=False, **_kwargs)
+        response = self.session.request(
+            method=method,
+            url=url,
+            headers=headers,
+            allow_redirects=self.allow_redirects,
+            **_kwargs
+        )
 
         if raise_exception and 400 <= response.status_code < 600:
             text = errors = None

tests/unit_tests/test_adapters.py

@@ -1,7 +1,10 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+import logging
 from unittest import TestCase
 
 import requests_mock
-from parameterized import parameterized
+from parameterized import parameterized, param
 
 from hvac import adapters
 
@@ -10,22 +13,64 @@ class TestRequest(TestCase):
     """Unit tests providing coverage for requests-related methods in the hvac Client class."""
 
     @parameterized.expand([
-        ("standard Vault address", 'https://localhost:8200'),
-        ("Vault address with route", 'https://example.com/vault'),
+        param(
+            "standard Vault address",
+            url='https://localhost:8200',
+        ),
+        param(
+            "Vault address with route",
+            url='https://example.com/vault',
+        ),
+        param(
+            "regression test for hvac issue #51",
+            url='https://localhost:8200',
+            path='keyring/http://some.url/sub/entry',
+        ),
+        param(
+            "redirect with location header for issue #343",
+            url='https://localhost:8200',
+            path='secret/some-secret',
+            redirect_url='https://some-other-place.com/secret/some-secret',
+
+        ),
     ])
-    @requests_mock.Mocker()
-    def test_get(self, test_label, test_url, requests_mocker):
-        test_path = 'v1/sys/health'
+    def test_get(self, label, url, path='v1/sys/health', redirect_url=None):
+        path = path.replace('//', '/')
         expected_status_code = 200
-        mock_url = '{0}/{1}'.format(test_url, test_path)
-        requests_mocker.register_uri(
-            method='GET',
-            url=mock_url,
-        )
-        adapter = adapters.Request(base_uri=test_url)
-        response = adapter.get(
-            url='v1/sys/health',
-        )
+        mock_url = '{0}/{1}'.format(url, path)
+        expected_request_urls = [mock_url]
+        adapter = adapters.Request(base_uri=url)
+        response_headers = {}
+        response_status_code = 200
+        if redirect_url is not None:
+            response_headers['Location'] = redirect_url
+            response_status_code = 301
+        with requests_mock.mock() as requests_mocker:
+            logging.debug('Registering "mock_url": %s' % mock_url)
+            requests_mocker.register_uri(
+                method='GET',
+                url=mock_url,
+                headers=response_headers,
+                status_code=response_status_code,
+            )
+            if redirect_url is not None:
+                expected_request_urls.append(redirect_url)
+                logging.debug('Registering "redirect_url": %s' % redirect_url)
+                requests_mocker.register_uri(
+                    method='GET',
+                    url=redirect_url,
+                )
+
+            response = adapter.get(
+                url=path,
+            )
+
+        # Assert all our expected uri(s) were requested
+        for request_num, expected_request_url in enumerate(expected_request_urls):
+            self.assertEqual(
+                first=expected_request_url,
+                second=requests_mocker.request_history[request_num].url
+            )
         self.assertEqual(
             first=expected_status_code,
             second=response.status_code,