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
Limit maximal size of incoming fragment. #252
Conversation
@@ -158,6 +158,9 @@ pub struct Settings { | |||
/// The maximum length of outgoing frames. Messages longer than this will be fragmented. | |||
/// Default: 65,535 | |||
pub fragment_size: usize, | |||
/// The maximum length of acceptable incoming frames. Messages longer than this will be rejected. | |||
/// Default: unlimited | |||
pub max_fragment_size: usize, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the implementation you are using u64
and here usize
? Why not use u64
everywhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
u64
is required by the Cursor
API. I prefer usize
in the settings for two reasons:
- Consistency with
fragment_size
- It is restricted by the available memory anyway, so I think
usize
describes that better. In the worst case we are going to be upcasting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay :)
@housleyjk Any chance to get a review on this one? |
fwiw we've pushed |
Sorry, been quite busy with other stuff recently. Thanks for the PR. |
Since frame size is allowed to be up to
2^64
one can forge a frame that declares extremely large payload size.In case we haven't received enough data yet:
https://github.com/housleyjk/ws-rs/compare/master...tomusdrw:td-maxframe?expand=1#diff-8c1748e0d20c7a39978ef7656d9f39f4R326
we reset the cursor position and wait for more data in
in_buffer
.By default the
in_buffer
is allowed to grow indefinitely, so the server will go OOM at some point trying to read such a big frame.PR introduces additional setting
max_fragment_size
which defines maximal (sane) allowed frame payload size.