fix: cap room.comments at 200 entries per collaboration session#1167
Open
rachel-d-07 wants to merge 1 commit into
Open
fix: cap room.comments at 200 entries per collaboration session#1167rachel-d-07 wants to merge 1 commit into
rachel-d-07 wants to merge 1 commit into
Conversation
Fixes imDarshanGK#1128 - room.comments was unbounded, allowing a single client to flood the list indefinitely and cause unbounded memory growth. Each new comment also broadcasts the full list to all connected clients, so payload size grew linearly with no limit. Added MAX_COMMENTS_PER_ROOM = 200 constant and a guard at the top of _handle_comment_added() that rejects further comments with an error response once the cap is reached.
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
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.
Fixes #1128 - room.comments was unbounded, allowing a single client to flood the list indefinitely and cause unbounded memory growth. Each new comment also broadcasts the full list to all connected clients, so payload size grew linearly with no limit.
Added MAX_COMMENTS_PER_ROOM = 200 constant and a guard at the top of _handle_comment_added() that rejects further comments with an error response once the cap is reached.
Description
In
backend/app/routers/collaboration.py, theCollaborationRoomdataclass held an unbounded
commentslist. Everycomment_addedmessage appended to it and then broadcast the full list to all
connected WebSocket clients — meaning payload size grew linearly with
no upper limit. A single malicious or misbehaving client could flood
the list indefinitely, consuming unbounded server memory and
increasingly large broadcast payloads.
Fix: added
MAX_COMMENTS_PER_ROOM = 200alongside the existingMAX_CODE_CHARSandMAX_COMMENT_CHARSconstants, and added a guardat the top of
_handle_comment_added()that rejects further commentswith a clear error response once the cap is reached. No other logic
is affected.
Related Issue
Fixes #1128
Type of change
Checklist
mainpytest -vand all tests passfeat/fix/docs/test: short descriptionScreenshots (if frontend change)
Test evidence
Local environment does not have all backend dependencies configured
for a full pytest run. Fix verified by code inspection and import check:
async with room.lockso no lock is acquiredon rejected requests — no performance cost for the rejection path
>=comparison so the cap is enforced at exactly 200, not 201the file (
{"type": "error", "detail": "..."})MAX_COMMENTS_PER_ROOMplaced alongsideMAX_CODE_CHARSandMAX_COMMENT_CHARSfor consistency and easy future tuning