Skip to content
New issue

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

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

Already on GitHub? # to your account

Explicitly handle redirects when doing POST requests #1069

Merged
merged 2 commits into from
Dec 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions commodore/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,26 @@ def _lieutenant_request(
headers=headers,
params=params,
timeout=timeout,
# don't let requests handle redirects (usually if the API URL is given without
# https://), since requests will rewrite the method to GET for the redirect which
# makes no sense when we're trying to POST to a POST-only endpoint.
allow_redirects=False,
)

if r.status_code in [301, 302, 307, 308]:
# Explicitly redo the POST if we get a 301, 302, 307 or 308 status code for the
# first call. We don't validate that the redirect location has the same domain as
# the original request, since we already unconditionally follow redirects with the
# bearer token for GET requests.
# Note that this wouldn't be necessary if all Lieutenant APIs would redirect us with
# a 308 for POST requests.
r = requests.post(
r.headers["location"],
json.dumps(data),
headers=headers,
params=params,
timeout=timeout,
)
else:
raise NotImplementedError(f"QueryType {method} not implemented")
except ConnectionError as e:
Expand Down
44 changes: 44 additions & 0 deletions tests/test_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,50 @@ def test_lieutenant_post(request_data, response, expected):
_verify_call_status(post_url, token=request_data["token"])


@responses.activate
def test_lieutenant_post_redirect():
base_url = "http://syn.example.com/"
redir_url = "https://syn.example.com/"

post_url = url_normalize(f"{base_url}/clusters/c-cluster-1234/compileMeta")
real_post_url = url_normalize(f"{redir_url}/clusters/c-cluster-1234/compileMeta")
token = "token"
payload = {"some": "data", "other": "data"}

# successful post response from Lieutenant API has no body
responses.add(
responses.POST,
real_post_url,
content_type="application/json",
status=204,
body=None,
match=[matchers.json_params_matcher(payload)],
)
responses.add(
responses.POST,
post_url,
content_type="application/json",
headers={"location": real_post_url},
status=302,
body=None,
match=[matchers.json_params_matcher(payload)],
)

helpers.lieutenant_post(
base_url,
token,
"clusters/c-cluster-1234",
"compileMeta",
post_data=payload,
)

assert len(responses.calls) == 2
call_initial = responses.calls[0]
assert call_initial.request.url == post_url
call_redirected = responses.calls[1]
assert call_redirected.request.url == real_post_url


def test_unimplemented_query_method():
with pytest.raises(NotImplementedError, match="QueryType PATCH not implemented"):
helpers._lieutenant_request(
Expand Down
Loading