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

Update max message size #3453

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MiXaiLL76
Copy link

Tornado is used to proxy web applications in Jupyter (https://github.com/jupyterhub/jupyter-server-proxy/blob/main/jupyter_server_proxy/websocket.py#L52).

Many applications use socket communication, and messages (for example, files) can be larger than 10MB.

Can we set the _default_max_message_size variable from ENV? For example, as I propose in this PR.

I also ask, is it possible to immediately change the default value to 1024 (1GB), as it has already been done in Jupyter or code-server?

Now the default value is 10MB (in my proposal, I carefully increased it to 100MB)

I came to this decision and PR thanks to this issue (coder/code-server#7139)

@bdarnell
Copy link
Member

bdarnell commented Jan 6, 2025

The max_message_size is a safety parameter - if it's too high it's easy for your server to run out of memory, whether through deliberate DoS attacks or just regular traffic. It's appropriate for the default to be small and for applications that need it to increase as appropriate. For example, Jupyter is typically used in a single-user environment so it is reasonable for them to increase the limit.

We don't have any precedent for using environment variables like this in Tornado. If an application would like to use env vars for its configuration, you're welcome to do so in your application settings:

app = Application(..., websocket_max_message_size=os.environ.get("CODE_SERVER_WEBSOCKET_MAX_MESSAGE_SIZE", 100*1024*1024)

or by overriding the max_message_size property in your WebSocketHandler.

Tornado's websocket interface was designed back when it didn't seem reasonable to work with large amounts of data directly in the browser (we assumed that bulk data would either be split into multiple websocket messages or go over a separate HTTP request), so it's entirely possible that it's too clumsy to do the kinds of things people want to do today. I'm open to discussion about how to make it more clear when the limit has been hit and whether some sort of streaming interface (analogous to @stream_request_body) is appropriate (even though there doesn't appear to be a sub-message streaming interface on the browser side)

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