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

Fix x_forwarded_proto for websockets #2043

Merged
merged 1 commit into from
Jul 12, 2023

Conversation

aminalaee
Copy link
Member

@aminalaee aminalaee commented Jul 12, 2023

Summary

Replaces #2011
Closes #1935

Checklist

  • I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.

@aminalaee aminalaee changed the title Fix for websockets Fix x_forwarded_proto for websockets Jul 12, 2023
@aminalaee aminalaee force-pushed the fix-forwarded-proto-websocket branch from fb2c3c5 to 33e37c8 Compare July 12, 2023 13:00
@aminalaee aminalaee requested a review from a team July 12, 2023 13:08
@Kludex Kludex merged commit 806c227 into encode:master Jul 12, 2023
12 checks passed
@karpetrosyan
Copy link
Member

You forgot to include it in the changelog.

@Kludex
Copy link
Sponsor Member

Kludex commented Jul 12, 2023

You forgot to include it in the changelog.

We don't do that on per PR yet on uvicorn.

@aminalaee aminalaee deleted the fix-forwarded-proto-websocket branch July 12, 2023 16:34
aadnehovda added a commit to aadnehovda/uvicorn that referenced this pull request Feb 29, 2024
Minor fix for encode#2043

Traefik already sets the X-Forwarded-Proto headers to ws or wss for websockets. https://github.com/traefik/traefik/blob/c1ef7429771104e79f2e87b236b21495cb5765f0/pkg/middlewares/forwardedheaders/forwarded_header.go#L150

This change should make sure we don't overwrite those values.
Kludex added a commit that referenced this pull request Mar 1, 2024
… it (#2258)

* Fix X-Forwarded-Proto when the proxy already sets it to "ws" or "wss"

Minor fix for #2043

Traefik already sets the X-Forwarded-Proto headers to ws or wss for websockets. https://github.com/traefik/traefik/blob/c1ef7429771104e79f2e87b236b21495cb5765f0/pkg/middlewares/forwardedheaders/forwarded_header.go#L150

This change should make sure we don't overwrite those values.

* Fix the logic

* Update test_proxy_headers.py

Test whether passing "wss" in X-Forwarded-Proto works

* Simplify the logic

(probably more ways to write this... lmk which you prefer)

* Update tests and min implementation

* Remove new line

---------

Co-authored-by: Marcelo Trylesinski <marcelotryle@gmail.com>
cr313 added a commit to cr313/py-project-uvicorn that referenced this pull request Apr 19, 2024
… it (#2258)

* Fix X-Forwarded-Proto when the proxy already sets it to "ws" or "wss"

Minor fix for encode/uvicorn#2043

Traefik already sets the X-Forwarded-Proto headers to ws or wss for websockets. https://github.com/traefik/traefik/blob/c1ef7429771104e79f2e87b236b21495cb5765f0/pkg/middlewares/forwardedheaders/forwarded_header.go#L150

This change should make sure we don't overwrite those values.

* Fix the logic

* Update test_proxy_headers.py

Test whether passing "wss" in X-Forwarded-Proto works

* Simplify the logic

(probably more ways to write this... lmk which you prefer)

* Update tests and min implementation

* Remove new line

---------

Co-authored-by: Marcelo Trylesinski <marcelotryle@gmail.com>
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.

middleware/proxy_headers.py: Inconsistent x-forwarded-proto for websockets
3 participants