Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable following of redirects by default #454

Merged
merged 11 commits into from
Mar 27, 2020
3 changes: 2 additions & 1 deletion bravado/fido_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ def prepare_request_for_twisted(request_params):
'url': string,
'timeout': float, # optional
'connect_timeout': float, # optional
'allow_redirects': bool, # optional
}
"""

Expand All @@ -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
Expand Down
2 changes: 2 additions & 0 deletions bravado/requests_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
57 changes: 57 additions & 0 deletions bravado/testing/integration_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -159,6 +160,17 @@ def _class_fqn(clz):
},
},
},
'/redirect': {
'get': {
'operationId': 'redirect_test',
'produces': ['application/json'],
'responses': {
'301': {
'description': 'HTTP/301',
},
},
},
},
},
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,6 @@ def test_separate_params():
'connect_timeout': 1,
'ssl_cert': None,
'ssl_verify': True,
'allow_redirects': False,
'timeout': 2,
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down