Skip to content

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

Open
Lalufu opened this issue Feb 12, 2025 · 6 comments
Open

Filter out websocket extensions from upstream #53

Lalufu opened this issue Feb 12, 2025 · 6 comments
Labels
bug Something isn't working

Comments

@Lalufu
Copy link

Lalufu commented Feb 12, 2025

Describe the bug

In

headers=client_request_headers,
, headers are copied from the upstream websocket connection to the httpx_ws call. This can lead to a desync in the negotiated options. For example, passing through sec-websocket-extensions: permessage-deflate from upstream to downstream may cause the downstream host to compress websocket messages, which httpx_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

@Lalufu Lalufu added the bug Something isn't working label Feb 12, 2025
@WSH032
Copy link
Owner

WSH032 commented Feb 12, 2025

Hi @Lalufu, thank you for reporting this.


which httpx_ws will not expect and fail

This sounds like an issue with httpx-ws. Can you provide a minimal reproducible example? This way, I (or you) can report the issue to httpx-ws.


Probably all sec-websocket* headers should not be passed to downstream.

Can you try manually modifying the request? Here is the documentation: https://wsh032.github.io/fastapi-proxy-lib/Usage/Advanced/#modify-request

@Lalufu
Copy link
Author

Lalufu commented Feb 13, 2025

I would not call this an issue with httpx or httpx_ws, fastapi-proxy-lib is actively taking upstream headers, and adding them to the downstream request, and some of these headers affect the way the websocket connection works, and must be under the total control of the client (here httpx).

The setup where I encountered this is a bit convoluted, admittedly.

UA => P1 => P2 => O

There's a user agent UA, here a mozilla firefox browser. The user agent supports the permessage-deflate extension.
There are two intermediate reverse proxies P1 and P2, both using fastapi-proxy-lib and ReverseWebSocketProxy, running under uvicorn. Uvicorn supports the permessage-deflate extension.
There is an origin server O which provides the websocket service that UA ultimately wants to connect to. This origin does not support the permessage-deflate extension.

In this setup. UA sends a request with sec-websocket-extensions: permessage-deflate to P1. ReverseWebSocketProxy copies this header into the request to P2. The httpx client on P1 is unaware of this and does not expect compressed responses.
P2 copies this header into the request to O The httpx client on P2 is not aware of this and does not expect compressed responses.
O does not support this extension, and sends an uncompressed websocket frame to P2. This gets received successfully, and gets passed upstream to P1. In doing so, the websocket stack on P2 compresses the frame, because incoming connection from P1 indicated that permessage-deflate was supported.

The compressed frame is received by P1, and fails verification there. The httpx client on P1 is not aware that P2 sends compressed frames (because it never asked for it), and tries to interpret the received frame as an uncompressed websocket frame, which fails.

The current solution I have for this is to disable support for permessage-deflate in uvicorn, which makes it ignore the flag and not compress responses.

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 Sec-Websocket* headers should be passed into the client (here httpx) as the client needs to control these itself.

@WSH032
Copy link
Owner

WSH032 commented Feb 13, 2025

I believe the current implementation is correct:

  1. P1 sends permessage-deflate to P2.uvicorn during the handshake.
  2. fastapi-proxy-lib starts establishing the connection between P2.httpx and O.
  3. P2.httpx forwards permessage-deflate to O during the handshake.
  4. O rejects permessage-deflate and establishes a connection with P2.httpx.
  5. fastapi-proxy-lib completes the connection between P2.httpx and O, and forwards messages between P2.uvicorn and P2.httpx.
  6. P2.uvicorn accepts permessage-deflate and establishes a connection with P1.

Therefore, the connection between P1 and P2.uvicorn uses permessage-deflate, while the connection between P2.httpx and O does not use permessage-deflate.

As for fastapi-proxy-lib between P2.uvicorn and P2.httpx, it does not handle frames at all:

async def _wait_client_then_send_to_server(
*, client_ws: starlette_ws.WebSocket, server_ws: httpx_ws.AsyncWebSocketSession
) -> starlette_ws.WebSocketDisconnect:
"""Receive data from client, then send to target server.
Args:
client_ws: The websocket which receive data of client.
server_ws: The websocket which send data to target server.
Returns:
If the client_ws sends a shutdown message normally, will return starlette_ws.WebSocketDisconnect.
Raises:
error for receiving: refer to `_starlette_ws_receive_bytes_or_str`
error for sending: refer to `_httpx_ws_send_bytes_or_str`
"""
while True:
try:
receive = await _starlette_ws_receive_bytes_or_str(client_ws)
except starlette_ws.WebSocketDisconnect as e:
return e
else:
await _httpx_ws_send_bytes_or_str(server_ws, receive)
async def _wait_server_then_send_to_client(
*, client_ws: starlette_ws.WebSocket, server_ws: httpx_ws.AsyncWebSocketSession
) -> httpx_ws.WebSocketDisconnect:
"""Receive data from target server, then send to client.
Args:
client_ws: The websocket which send data to client.
server_ws: The websocket which receive data of target server.
Returns:
If the server_ws sends a shutdown message normally, will return httpx_ws.WebSocketDisconnect.
Raises:
error for receiving: refer to `_httpx_ws_receive_bytes_or_str`
error for sending: refer to `_starlette_ws_send_bytes_or_str`
"""
while True:
try:
receive = await _httpx_ws_receive_bytes_or_str(server_ws)
except httpx_ws.WebSocketDisconnect as e:
return e
else:
await _starlette_ws_send_bytes_or_str(client_ws, receive)


I believe that if the issue is indeed with compressed frames, then it must be a problem with httpx-ws(P1.httpx). This is because fastapi-proxy-lib does not handle frames at all; it only forwards text and binary messages.

httpx-ws does not seem to enable the wsproto.extensions.PerMessageDeflate extension based on the Sec-WebSocket-Extensions: permessage-deflate header:

@WSH032
Copy link
Owner

WSH032 commented Feb 13, 2025

I would not call this an issue with httpx or httpx_ws, fastapi-proxy-lib is actively taking upstream headers, and adding them to the downstream request, and some of these headers affect the way the websocket connection works, and must be under the total control of the client (here httpx).

ReverseWebSocketProxy copies this header into the request to P2. The httpx client on P1 is unaware of this and does not expect compressed responses.

httpx/httpx-ws indeed fully controls these Sec-WebSocket-* headers. Regardless, the headers forwarded by fastapi-proxy-lib have already informed httpx-ws of this (in other words, fastapi-proxy-lib is just a normal user of httpx-ws). If httpx-ws indeed does not support these extensions, it should either remove these headers or throw an exception (for example, fastapi-proxy-lib directly throws an exception for HTTP2 requests because it does not support them).

@Lalufu
Copy link
Author

Lalufu commented Feb 13, 2025 via email

@WSH032
Copy link
Owner

WSH032 commented Feb 13, 2025

I'm glad we reached a consensus.😁

Would you like to open an issue/feature request with httpx-ws? I am currently busy maintaining another repository of mine.


not sending sec-websocket-* headers to httpx_ws in the first place should also be implemented in the interest of avoiding issues.

I suggest waiting to see what httpx-ws decides before making any changes.

  • If httpx-ws decides to filter these headers, then fastapi-proxy-lib doesn't need to make any changes.
  • If httpx-ws decides to throw an exception, then I will filter these headers in fastapi-proxy-lib.

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/",
)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants