-
Notifications
You must be signed in to change notification settings - Fork 9
Filter out websocket extensions from upstream #53
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
Comments
Hi @Lalufu, thank you for reporting this.
This sounds like an issue with
Can you try manually modifying the request? Here is the documentation: https://wsh032.github.io/fastapi-proxy-lib/Usage/Advanced/#modify-request |
I would not call this an issue with The setup where I encountered this is a bit convoluted, admittedly.
There's a user agent In this setup. The compressed frame is received by The current solution I have for this is to disable support for The primary issue here is that there are multiple independent WS connections involved on both sides of the reverse proxy, and each side must be allowed to make independent decisions about extension negotiation. This means that no |
I believe the current implementation is correct:
Therefore, the connection between As for fastapi-proxy-lib/src/fastapi_proxy_lib/core/websocket.py Lines 263 to 310 in 64a7be4
I believe that if the issue is indeed with compressed
|
|
Reading the `httpx-ws` and `wsproto` code, it seems in indeed that while
`wsproto` supports deflate, `httpx-ws` does not make use of it, and expects
uncompressed frames to be received from the remote. This is a valid
design decision.
`httpx-ws` also does not seem to properly filter headers passed to it, allowing
the `permessage-deflate` header passed to it from `fastapi-proxy-lib` to
be passed through and influence the connection in a way that `httpx_ws`
does not expect.
I agree that `httpx_ws` should implement better filtering, not sending
`sec-websocket-*` headers to `httpx_ws` in the first place should also
be implemented in the interest of avoiding issues.
|
I'm glad we reached a consensus.😁 Would you like to open an issue/feature request with
I suggest waiting to see what
As for the current mitigation, I suggest you manually modify the request: https://wsh032.github.io/fastapi-proxy-lib/Usage/Advanced/#modify-request from collections.abc import Generator
from typing import Any
import httpx
from fastapi_proxy_lib.fastapi.app import reverse_http_app
from httpx import Request
class MyCustomAuth(httpx.Auth):
# ref: <https://www.python-httpx.org/advanced/authentication/#custom-authentication-schemes>
def __init__(self, token: str):
self.token = token
def auth_flow(self, request: httpx.Request) -> Generator[Request, Any, None]:
# filter out all the headers that start with "sec-websocket"
raw_headers = request.headers
filtered_headers = filter(
lambda header: not header[0].startswith(b"sec-websocket"),
raw_headers.raw,
)
request.headers = httpx.Headers(
list(filtered_headers), encoding=raw_headers.encoding
)
# Or:
#
# if "sec-websocket-extension" in request.headers:
# del request.headers["sec-websocket-extension"]
yield request
app = reverse_http_app(
client=httpx.AsyncClient(auth=MyCustomAuth("bearer_token")),
base_url="http://www.httpbin.org/",
) |
Describe the bug
In
fastapi-proxy-lib/src/fastapi_proxy_lib/core/websocket.py
Line 536 in 64a7be4
httpx_ws
call. This can lead to a desync in the negotiated options. For example, passing throughsec-websocket-extensions: permessage-deflate
from upstream to downstream may cause the downstream host to compress websocket messages, whichhttpx_ws
will not expect and fail.Probably all
sec-websocket*
headers should not be passed to downstream.Observed on fastapi-proxy-lib 0.2.0
The text was updated successfully, but these errors were encountered: