Repository navigation
[SYSADMIN ACTION][Bugfix:System] Filter websocket cookies - #13393
Merged
bmcutler merged 3 commits intoOct 7, 2026
Merged
Conversation
Forward only submitty_websocket_token to the socket server. Ratchet rejects handshakes over 4096 bytes with a 413, and the browser sends every cookie for the host, so unrelated cookies could block websocket connections. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
RichardJSun
force-pushed
the
websocket-proxy-cookie-filter
branch
from
September 30, 2026 16:29
4933a85 to
05ff77a
Compare
RichardJSun
marked this pull request as ready for review
October 1, 2026 02:28
RichardJSun
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #13393 +/- ##
============================================
- Coverage 22.88% 22.87% -0.01%
- Complexity 10299 10306 +7
============================================
Files 307 307
Lines 39868 39878 +10
Branches 672 672
============================================
Hits 9123 9123
- Misses 30176 30186 +10
Partials 569 569
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Member
|
@RichardJSun |
Member
Author
|
Professor, I uploaded screenshots from submitty.cs.rpi.edu to #13391 and added them to the description above. They show that the websocket failure comes from cookie size, because removing the 2.78 KB This PR handles cookies Submitty doesn't control, such as |
RichardJSun
marked this pull request as draft
October 2, 2026 03:08
…figs install_system.sh copies the nginx config only when none exists, so existing servers never get the cookie filter. A system migration adds the line to the /ws block of the installed config, and the install's nginx restart applies it. Also say in the config comment that the 4096-byte limit comes from Ratchet, not from this config. Assisted-By: Claude Opus 5.5
bmcutler
requested changes
Oct 7, 2026
bmcutler
left a comment
bmcutler
left a comment
Member
There was a problem hiding this comment.
Please revert/remove the migration. We leave editing of these configuration files to the system administrator because they often have other customizations. This is why the PR is labeled SYSADMIN ACTION.
Thank you for updating the comment.
Sysadmins edit their installed nginx config by hand, because it often has local customizations. The version notes describe the change. Assisted-By: Claude Opus 5.5
Member
Author
|
Removed the migration. The steps are now in the v26.10.01 version notes (Submitty/submitty.github.io#797). |
bmcutler
added a commit
to Submitty/submitty.github.io
that referenced
this pull request
Oct 7, 2026
Update NGINX config file to limit size of cookies See also Submitty/Submitty#13393 Co-authored-by: Barb Cutler <Barb Cutler>
bmcutler
approved these changes
Oct 7, 2026
bmcutler
marked this pull request as ready for review
October 7, 2026 04:38
bmcutler
added a commit
that referenced
this pull request
Oct 7, 2026
### Why is this Change Important & Necessary? Fixes #13391 Partially addresses #10376 The websocket server rejects handshakes over 4 KB. Three cookies that only JavaScript reads (`cm_folder_data`, `open_files` and `<gradeable_id>_history`) grow with use. They can push the handshake past the limit, and then every websocket feature breaks for that user. ### What is the New Behavior? The three values move to web storage, so the browser no longer sends them to the server. `cm_folder_data` and `open_files` go to `localStorage`. Checkpoint history goes to `sessionStorage`, because it holds student scores and logout does not clear `localStorage`. Course Materials deletes the old cookie on load, so affected users recover on their next visit. Folder state in the old cookie is not carried over. #### Screenshots and video On submitty.cs.rpi.edu, the office hours queue fails with a 2.78 KB `cm_folder_data` cookie and works after I delete only that cookie: <img width="1452" height="475" alt="Office hours queue with the websocket warning and a 2.78 KB cm_folder_data cookie" src="https://github.com/user-attachments/assets/6d26fcbb-9729-43bd-84a7-d179ab8104e3" /> <img width="1470" height="486" alt="Office hours queue without the warning after deleting cm_folder_data" src="https://github.com/user-attachments/assets/c5a1d5ae-f9ab-45c9-98d2-5558d95f097f" /> On a local dev server with the large cookie from step 1, `main` fails with 413: https://github.com/user-attachments/assets/95cc0ba0-a483-4c9e-bef7-e3cb5c79923c <img width="800" alt="main: websocket banner and HTTP 413 with the oversized cookie" src="https://github.com/user-attachments/assets/fb098576-7978-484c-bf03-a981f099183c" /> On this branch, Course Materials deletes the cookie and the websocket connects: https://github.com/user-attachments/assets/bde8224f-8a21-4718-b722-af02012e43fa <img width="800" alt="This branch: cookie deleted and HTTP 101" src="https://github.com/user-attachments/assets/fe449561-2de1-42c1-a797-a112764affe4" /> TA grading state on this branch: <img width="800" alt="open_files in localStorage and the file reopened for another student" src="https://github.com/user-attachments/assets/0b495375-312d-469e-b1dc-9e7383002c65" /> <img width="800" alt="grading_lab_history in sessionStorage" src="https://github.com/user-attachments/assets/95b66cec-5370-42f6-9fee-7f1cae5b7167" /> ### What steps should a reviewer take to reproduce or test the bug or new feature? 1. On `main`, open Course Materials and run this in the console to make a large cookie: `Cookies.set('cm_folder_data', JSON.stringify(Object.fromEntries([...Array(24).keys()].map(i => ['/var/local/submitty/courses/f26/sample/uploads/course_materials/folder' + i, { id: 'f' + i, open: true }]))))` 2. Open the office hours queue. The websocket fails with 413. 3. Switch to this branch and reload Course Materials. The cookie is gone. 4. Open the office hours queue again. It connects. 5. Open and close some folders, then reload. They keep their state in Local Storage. 6. In TA grading with Auto open checked, open some files and go to the next student. The files reopen, and `open_files` is in Local Storage. 7. Change some checkpoint scores. `<gradeable_id>_history` is in Session Storage. ### Automated Testing & Documentation No new tests. The existing Cypress tests do not use these cookies. ### Other information #13393 handles the cookies that other sites set. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Barb Cutler <bmcutler@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why is this Change Important & Necessary?
Related to #13391
Ratchet, the library the websocket server uses, rejects handshakes over 4096 bytes with a 413. This PR doesn't add that limit. The browser sends every cookie for the host with the handshake, including cookies that other RPI sites set on
.rpi.edu. Submitty cannot remove those cookies. The socket server reads onlysubmitty_websocket_token.What is the New Behavior?
nginx forwards only
submitty_websocket_tokento the socket server, so other cookies cannot make the handshake too large. The PHP websocket client is unaffected, because the server identifies it by headers, not cookies.Existing servers keep their own nginx config, so a sysadmin adds the line by hand. See https://submitty.org/sysadmin/installation/version_notes/v26.10.01 (Submitty/submitty.github.io#797).
Screenshots and video
On submitty.cs.rpi.edu, the cookie list includes
_snow_id.1d43from another site on.rpi.edu:On a local dev server with the 3.4 KB cookie from step 2, the
mainnginx config fails with 413, and a live queue announcement does not arrive:13393-before.mp4
With this config, the websocket connects and the announcement arrives live:
13393-after.mp4
What steps should a reviewer take to reproduce or test the bug or new feature?
.setup/nginx/submitty.confto thelocation /wsblock of/etc/nginx/sites-available/submitty.conf, then runsudo systemctl reload nginx.Cookies.set('big_test_cookie', 'x'.repeat(3500))in the browser console.mainit fails with 413.Automated Testing & Documentation
No new tests.
Other information
SYSADMIN ACTION: see https://submitty.org/sysadmin/installation/version_notes/v26.10.01.
This doesn't help once the whole
Cookieheader passes 8 KB. nginx and Apache both reject such requests with a 400 by default, so every page fails at that point, not just the websocket.