Merge pull request #454 from mattdowdell/nofollow-redirects

Disable following of redirects by default

bravado/fido_client.py

@@ -226,6 +226,7 @@ def prepare_request_for_twisted(request_params):
                 'url': string,
                 'timeout': float,  # optional
                 'connect_timeout': float,  # optional
+                'allow_redirects': bool,  # optional
             }
         """
 
@@ -244,7 +245,7 @@ def prepare_request_for_twisted(request_params):
             params=request_params.get('params'),
             files=request_params.get('files'),
             url=request_params.get('url'),
-            method=request_params.get('method')
+            method=request_params.get('method'),
         )
 
         # content-length was computed by 'requests' based on the current body

bravado/requests_client.py

@@ -266,6 +266,7 @@ def result(self, timeout=None):
         response = self.session.send(
             prepared_request,
             timeout=self.build_timeout(timeout),
+            allow_redirects=self.misc_options['allow_redirects'],
             **settings
         )
         return response
@@ -360,6 +361,7 @@ def separate_params(
         misc_options = {
             'ssl_verify': self.ssl_verify,
             'ssl_cert': self.ssl_cert,
+            'allow_redirects': sanitized_params.pop('allow_redirects', False),
         }
 
         if 'connect_timeout' in sanitized_params:

bravado/testing/integration_test.py

@@ -16,6 +16,7 @@
 from bravado.client import SwaggerClient
 from bravado.exception import BravadoConnectionError
 from bravado.exception import BravadoTimeoutError
+from bravado.exception import HTTPMovedPermanently
 from bravado.http_client import HttpClient
 from bravado.http_future import FutureAdapter
 from bravado.swagger_model import Loader
@@ -159,6 +160,17 @@ def _class_fqn(clz):
                 },
             },
         },
+        '/redirect': {
+            'get': {
+                'operationId': 'redirect_test',
+                'produces': ['application/json'],
+                'responses': {
+                    '301': {
+                        'description': 'HTTP/301',
+                    },
+                },
+            },
+        },
     },
 }
 
@@ -234,6 +246,14 @@ def sleep_api():
     return sec_to_sleep
 
 
+@bottle.get('/redirect')
+def redirect_test():
+    return bottle.HTTPResponse(
+        status=301,
+        headers={'Location': '/json'},
+    )
+
+
 def run_bottle_server(port):
     """Wrapper function for bottle.run so that the child Python interpreter finds the bottle routes on Windows."""
     bottle.run(quiet=True, host='localhost', port=port)
@@ -473,6 +493,43 @@ def test_msgpack_support(self, swagger_http_server):
         assert response.headers['Content-Type'] == APP_MSGPACK
         assert unpackb(response.raw_bytes, encoding='utf-8') == API_RESPONSE
 
+    def test_following_redirects(self, swagger_http_server):
+        try:
+            # if running the default tests, the fido dependencies aren't loaded
+            # so rather than load dependencies unnecessarily, skip the test if that's not the case
+            # the coverage test incorrectly marks the exception handling as uncovered
+            # hence the pragma usage
+            from bravado.fido_client import FidoClient
+        except ImportError:  # pragma: no cover
+            pytest.skip('Fido dependencies have not been loaded, skipping test')  # pragma: no cover
+        else:
+            # the FidoClient doesn't have a way to turn off redirects being followed
+            # so limit this test to the RequestsClient instead
+            if isinstance(self.http_client, FidoClient):
+                pytest.skip('Following redirects is not supported in the FidoClient')
+
+        response = self.http_client.request({
+            'method': 'GET',
+            'url': '{server_address}/redirect'.format(server_address=swagger_http_server),
+            'params': {},
+            'allow_redirects': True,
+        }).result(timeout=1)
+
+        assert isinstance(response, IncomingResponse) and response.status_code == 200
+
+    def test_redirects_are_not_followed(self, swagger_http_server):
+        with pytest.raises(HTTPMovedPermanently) as excinfo:
+            self.http_client.request({
+                'method': 'GET',
+                'url': '{server_address}/redirect'.format(server_address=swagger_http_server),
+                'params': {},
+            }).result(timeout=1)
+
+        exc = excinfo.value
+
+        assert isinstance(exc.response, IncomingResponse) and exc.response.status_code == 301
+        assert isinstance(exc.response, IncomingResponse) and exc.response.headers['Location'] == '/json'
+
     def test_timeout_errors_are_thrown_as_BravadoTimeoutError(self, swagger_http_server):
         if not self.http_future_adapter_type.timeout_errors:
             pytest.skip('{} does NOT defines timeout_errors'.format(self.http_future_adapter_type))

tests/requests_client/RequestsClient/separate_params_test.py

@@ -14,5 +14,6 @@ def test_separate_params():
         'connect_timeout': 1,
         'ssl_cert': None,
         'ssl_verify': True,
+        'allow_redirects': False,
         'timeout': 2,
     }

tests/requests_client/RequestsFutureAdapter/header_type_casting_test.py

@@ -5,7 +5,15 @@
 def test_result_header_values_are_bytes(session_mock, request_mock):
     session_mock.merge_environment_settings.return_value = {}
     request_mock.headers = {b'X-Foo': b'hi'}
-    RequestsFutureAdapter(session_mock, request_mock, misc_options={'ssl_verify': True, 'ssl_cert': None}).result()
+    RequestsFutureAdapter(
+        session_mock,
+        request_mock,
+        misc_options={
+            'ssl_verify': True,
+            'ssl_cert': None,
+            'allow_redirects': False
+        },
+    ).result()
 
     # prepare_request should be called with a request containing correctly
     # casted headers (bytestrings should be preserved)