Bravado should not silently follow redirects

[mattdowdell]

In my API spec, I have an endpoint that returns a 301 redirect. This redirects to another URL on a different server that subsequently returns a 200. However, because bravado is using requests, this redirect never comes back to bravado which gets the 200 (which is not a specified response) and then throws an error.

Relevant section of logs below. Logs without a prefix for where it came from comes from setting http.client.HTTPConnection.debuglevel = 5. Comments are inserted for clarity and some addresses are redacted for privacy (as this is on a private network).

# here we send the initial request which responds with a 301
[10:11:10.432][bravado.client][DEBUG] getSecureClient({'_request_options': {'headers': {'UserID': '123456'}}, 'storeID': '3888d0f1-7896-4c8e-98c8-4934d5a37aff'})
send: b'GET /api/v2/backup/stores/3888d0f1-7896-4c8e-98c8-4934d5a37aff/secure-client HTTP/1.1\r\nHost: localhost:4000\r\nUser-Agent: python-requests/2.22.0\r\nAccept-Encoding: gzip, d                                        eflate\r\nAccept: */*\r\nConnection: keep-alive\r\nUserID: 123456\r\n\r\n'
reply: 'HTTP/1.1 301 Moved Permanently\r\n'
header: Location: http://<redirected-url>/<somefile>
header: Vary: Origin
header: Date: Mon, 17 Feb 2020 10:11:10 GMT
header: Content-Length: 0
[10:11:10.437][urllib3.connectionpool][DEBUG] http://localhost:4000 "GET /api/v2/backup/stores/3888d0f1-7896-4c8e-98c8-4934d5a37aff/secure-client HTTP/1.1" 301 0

# because bravado doesn't configure anything for redirects
# they are automatically followed by requests/urllib3
[10:11:10.440][urllib3.connectionpool][DEBUG] Starting new HTTP connection (1): web-proxy.sdc.hpecorp.net:8080
send: b'GET http://<redirected-url>/<somefile> HTTP/1.1\r\nHost: 16.26.136.45:8080\r\nUser-Agent: python-requests/2.22.0\r\nAccept-Encoding: gzip, deflate\r\nAccept: */*\r\nConnection: keep-alive\r\nUserID: 123456\r\n\r\n'
reply: 'HTTP/1.1 200 OK\r\n'
header: Date: Mon, 17 Feb 2020 10:12:29 GMT
header: X-Content-Type-Options: nosniff
header: Content-Security-Policy: sandbox; default-src 'none'; img-src 'self'; style-src 'self';
header: X-WebKit-CSP: sandbox; default-src 'none'; img-src 'self'; style-src 'self';
header: X-Content-Security-Policy: sandbox; default-src 'none'; img-src 'self'; style-src 'self';
header: Last-Modified: Mon, 17 Feb 2020 09:49:05 GMT
header: Expires: Mon, 17 Feb 2020 09:49:05 GMT
header: Accept-Ranges: bytes
header: Content-Type: application/octet-stream
header: Content-Length: 17681912
header: Server: Jetty(9.4.z-SNAPSHOT)
header: Connection: Keep-Alive
header: Age: 0
[10:11:10.474][urllib3.connectionpool][DEBUG] http://<proxy>:8080 "GET http://<redirected-url>/<somefile> HTTP/1.1" 200 17681912
Traceback (most recent call last):
  File "/home/matt/repos/baas-cli/.venv/lib/python3.7/site-packages/bravado/http_future.py", line 337, in unmarshal_response
    op=operation,
  File "/home/matt/repos/baas-cli/.venv/lib/python3.7/site-packages/bravado/http_future.py", line 370, in unmarshal_response_inner
    response_spec = get_response_spec(status_code=response.status_code, op=op)
  File "/home/matt/repos/baas-cli/.venv/lib/python3.7/site-packages/bravado_core/response.py", line 160, in get_response_spec
    "status_code or use a `default` response.".format(status_code, op),
bravado_core.exception.MatchingResponseNotFound: Response specification matching http status_code 200 not found for operation Operation(getSecureClient). Either add a response specifi                                        cation for the status_code or use a `default` response.

Initially I wondered if it was worth exposing out redirect configuration to the bravado client, but on reflection I don't think bravado should be doing anything in this area. If the API spec says it returns a 301, bravado should not be doing anything extra that might obscure that. I also wondered that if the responses claim they should return a 30X status then redirects should be implicitly disabled, but that creeps into the territory of being a bit too smart or non-obvious.

[macisamuele]

@mattdowell thanks for reporting this.

Ideally we should not follow redirects as APIs are defining the supported status codes.

I'm curious to understand if this behaviour is present only on the requests client or is present also in the fido client or bravado-asyncio client.

I'm currently running short on bandwidth so I cannot ensure the publication of a PR in a reasonable time (let's say a week). If you have time to provide a PR addressing this issue it will be great (we do welcome contributions)

[mattdowdell]

Thanks for the quick response @macisamuele.

The fido client calls into twisted at a glance which the docs suggests that it will not follow redirects by default. bravado-asyncio appears to use aiohttp which seems to follow a similar interface as requests and looks to follow redirects by default. All of this is based on navigating unfamiliar code and minimal searching, so take the results with a pinch of salt.

I can look into fixing this - it'd definitely be useful for some API testing my team are doing - if you're able to provide some basic pointers? Some initial searches suggest code around requests_client.py#L338 and client.py#L234 look relevant.

Is there any license agreement to sign as well?

[macisamuele]

As far as I can tell there are no special agreements to sign.

About the code pointers seems like you have already identified them. A small request would be to add a specific test into bravado.testing.integration_tests to ensure no further regressions on this.