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

Properly set host header to ascii string in ProxyFixMiddleware. #178

Merged
merged 1 commit into from
Jan 4, 2024

Conversation

apollo13
Copy link
Contributor

@apollo13 apollo13 commented Jan 1, 2024

While playing with supporting trusted_cidrs in ProxyFixMiddleware I noticed that mypy/pyright cannot narrow the type properly when:

if scope["type"] in {"http", "websocket"}:

is used.

After rewriting that to:

if scope["type"] == "http" or scope["type"] == "websocket":

I was able to remove the type: ignore statements and discovered another bug, namely that the host header is set as a string and not as a bytes object.

I think the type narrowing is quite useful here, so maybe we want to keep it? An alternative would be to write it like this:

if scope["type"] == "lifespan":
   pass
else:
   ... existing code

though that does not end well if another scope type is added in the future.

No strong feeling if you want to keep the in {} check instead and ignore the typing, but the host header value should be bytes either way :D

@apollo13
Copy link
Contributor Author

apollo13 commented Jan 1, 2024

Oh and type narrowing for a set with literals will probably not get supported ever: microsoft/pyright#6871 (comment)

@pgjones pgjones merged commit 3fbd5f2 into pgjones:main Jan 4, 2024
13 checks passed
@apollo13 apollo13 deleted the proxy_fix_fixes branch March 12, 2024 20:47
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