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

Limit maximal size of incoming fragment. #252

Merged
merged 1 commit into from Feb 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/connection.rs
Expand Up @@ -714,7 +714,8 @@ where
}

fn read_frames(&mut self) -> Result<()> {
while let Some(mut frame) = Frame::parse(&mut self.in_buffer)? {
let max_size = self.settings.max_fragment_size as u64;
while let Some(mut frame) = Frame::parse(&mut self.in_buffer, max_size)? {
match self.state {
// Ignore data received after receiving close frame
RespondingClose | FinishedClose => continue,
Expand Down
12 changes: 11 additions & 1 deletion src/frame.rs
Expand Up @@ -244,7 +244,7 @@ impl Frame {
}

/// Parse the input stream into a frame.
pub fn parse(cursor: &mut Cursor<Vec<u8>>) -> Result<Option<Frame>> {
pub fn parse(cursor: &mut Cursor<Vec<u8>>, max_payload_length: u64) -> Result<Option<Frame>> {
let size = cursor.get_ref().len() as u64 - cursor.position();
let initial = cursor.position();
trace!("Position in buffer {}", initial);
Expand Down Expand Up @@ -299,6 +299,16 @@ impl Frame {
}
trace!("Payload length: {}", length);

if length > max_payload_length {
return Err(Error::new(
Kind::Protocol,
format!(
"Rejected frame with payload length exceeding defined max: {}.",
max_payload_length
),
));
}

let mask = if masked {
let mut mask_bytes = [0u8; 4];
if cursor.read(&mut mask_bytes)? != 4 {
Expand Down
4 changes: 4 additions & 0 deletions src/lib.rs
Expand Up @@ -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,
Copy link

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?

Copy link
Contributor Author

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:

  1. Consistency with fragment_size
  2. 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.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay :)

/// The size of the incoming buffer. A larger buffer uses more memory but will allow for fewer
/// reallocations.
/// Default: 2048
Expand Down Expand Up @@ -245,6 +248,7 @@ impl Default for Settings {
fragments_capacity: 10,
fragments_grow: true,
fragment_size: u16::max_value() as usize,
max_fragment_size: usize::max_value(),
in_buffer_capacity: 2048,
in_buffer_grow: true,
out_buffer_capacity: 2048,
Expand Down