Skip to content

Dataframe reads all of payload. #128

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

Merged
merged 3 commits into from
May 29, 2017
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions src/client/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,6 @@ mod async_imports {
#[cfg(feature="async")]
use self::async_imports::*;

// TODO: add extra funcs for future stuff, like auto ping and auto close


/// Build clients with a builder-style API
/// This makes it easy to create and configure a websocket
/// connection:
Expand Down Expand Up @@ -662,7 +659,6 @@ impl<'u> ClientBuilder<'u> {
Box::new(future)
}

// TODO: add timeout option for connecting
// TODO: add conveniences like .response_to_pings, .send_close, etc.
/// Asynchronously create an insecure (plain TCP) connection to the client.
///
Expand Down Expand Up @@ -974,6 +970,4 @@ mod tests {
assert!(protos.contains(&"electric".to_string()));
assert!(!protos.contains(&"rust-websocket".to_string()));
}

// TODO: a few more
}
5 changes: 0 additions & 5 deletions src/codec/ws.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@ use ws::message::Message as MessageTrait;
use ws::util::header::read_header;
use result::WebSocketError;

// TODO: IMPORTANT: check if frame_size is correct,
// do not call .reserve with the entire size

/// Even though a websocket connection may look perfectly symmetrical
/// in reality there are small differences between clients and servers.
/// This type is passed to the codecs to inform them of what role they are in
Expand Down Expand Up @@ -290,8 +287,6 @@ impl<M> Encoder for MessageCodec<M>
}
}

// TODO: add tests to check boundary cases for reading dataframes

#[cfg(test)]
mod tests {
use super::*;
Expand Down
27 changes: 24 additions & 3 deletions src/dataframe.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//! Module containing the default implementation of data frames.
use std::io::{Read, Write};
use std::io::{self, Read, Write};
use result::{WebSocketResult, WebSocketError};
use ws::dataframe::DataFrame as DataFrameable;
use ws::util::header::DataFrameHeader;
Expand Down Expand Up @@ -61,7 +61,6 @@ impl DataFrame {
if !should_be_masked {
return Err(WebSocketError::DataFrameError("Expected unmasked data frame"));
}
// TODO: move data to avoid copy?
mask::mask_data(mask, &body)
}
None => {
Expand All @@ -87,7 +86,10 @@ impl DataFrame {
let header = try!(dfh::read_header(reader));

let mut data: Vec<u8> = Vec::with_capacity(header.len as usize);
reader.take(header.len).read_to_end(&mut data)?;
let read = reader.take(header.len).read_to_end(&mut data)?;
if (read as u64) < header.len {
return Err(io::Error::new(io::ErrorKind::UnexpectedEof, "incomplete payload").into());
}

DataFrame::read_dataframe_body(header, data, should_be_masked)
}
Expand Down Expand Up @@ -212,6 +214,25 @@ mod tests {
};
assert_eq!(obtained, expected);
}

#[test]
fn read_incomplete_payloads() {
let mut data = vec![0x8au8, 0x08, 0x19, 0xac, 0xab, 0x8a, 0x52, 0x4e, 0x05, 0x00];
let payload = vec![25, 172, 171, 138, 82, 78, 5, 0];
let short_header = DataFrame::read_dataframe(&mut &data[..1], false);
let short_payload = DataFrame::read_dataframe(&mut &data[..6], false);
let full_payload = DataFrame::read_dataframe(&mut &data[..], false);
data.push(0xff);
let more_payload = DataFrame::read_dataframe(&mut &data[..], false);

match (short_header.unwrap_err(), short_payload.unwrap_err()) {
(WebSocketError::NoDataAvailable, WebSocketError::NoDataAvailable) => (),
_ => assert!(false),
};
assert_eq!(full_payload.unwrap().data, payload);
assert_eq!(more_payload.unwrap().data, payload);
}

#[bench]
fn bench_read_dataframe(b: &mut Bencher) {
let data = b"The quick brown fox jumps over the lazy dog";
Expand Down
6 changes: 1 addition & 5 deletions src/receiver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,10 +132,6 @@ impl ws::Receiver for Receiver {
}
}

// TODO: this is slow, easy fix
let buffer = self.buffer.clone();
self.buffer.clear();

Ok(buffer)
Ok(::std::mem::replace(&mut self.buffer, Vec::new()))
}
}