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

fakefront: fix bad Domain set on cookies in some cases #497

Merged
merged 1 commit into from
Sep 11, 2023

Conversation

rohanpm
Copy link
Member

@rohanpm rohanpm commented Sep 11, 2023

When simulating cloudfront events we used the WSGI SERVER_NAME variable as the cloudfront distribution hostname.

Problem: that variable seems to be the requested bind address passed to gunicorn, but that can be '0.0.0.0' to request binding to all addresses. That variable ends up being included as 'Domain=0.0.0.0' in responses to the /_/cookie endpoint, which causes the cookie never to match since 0.0.0.0 is not a valid domain.

Fix it by picking the hostname out of the HTTP Host header, which should always match the client's idea of the currently accessed host.

It seems like this has always been broken, but it was hidden since fakefront doesn't need/validate signed URLs or cookies. It was uncovered after a recent pipeline-test change which causes tests to more strictly parse returned cookies.

When simulating cloudfront events we used the WSGI SERVER_NAME variable
as the cloudfront distribution hostname.

Problem: that variable seems to be the requested bind address passed to
gunicorn, but that can be '0.0.0.0' to request binding to all addresses.
That variable ends up being included as 'Domain=0.0.0.0' in responses to
the /_/cookie endpoint, which causes the cookie never to match since
0.0.0.0 is not a valid domain.

Fix it by picking the hostname out of the HTTP Host header, which should
always match the client's idea of the currently accessed host.

It seems like this has always been broken, but it was hidden since
fakefront doesn't need/validate signed URLs or cookies. It was uncovered
after a recent pipeline-test change which causes tests to more strictly
parse returned cookies.
@rohanpm rohanpm marked this pull request as ready for review September 11, 2023 01:48
@rohanpm rohanpm merged commit 95ad798 into release-engineering:master Sep 11, 2023
1 check passed
@rohanpm rohanpm deleted the fakefront-cookie-domain branch September 11, 2023 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants