Merge pull request #1758 from nymtech/jon/fix/deserialize-length-check

Fix deserialize length check
This commit is contained in:
Jon Häggblad
2022-11-17 16:09:54 +01:00
committed by GitHub
3 changed files with 43 additions and 47 deletions
+1 -1
View File
@@ -291,7 +291,7 @@ impl NymClient {
msg_input,
closed_connection_tx,
buffer_requester,
self.as_mix_recipient(),
&self.as_mix_recipient(),
);
websocket::Listener::new(self.config.get_listening_port()).start(websocket_handler);
+33 -37
View File
@@ -69,13 +69,13 @@ impl Handler {
msg_input: InputMessageSender,
closed_connection_tx: ClosedConnectionSender,
buffer_requester: ReceivedBufferRequestSender,
self_full_address: Recipient,
self_full_address: &Recipient,
) -> Self {
Handler {
msg_input,
closed_connection_tx,
buffer_requester,
self_full_address,
self_full_address: *self_full_address,
socket: None,
received_response_type: Default::default(),
}
@@ -83,14 +83,14 @@ impl Handler {
fn handle_send(
&mut self,
recipient: Recipient,
recipient: &Recipient,
message: Vec<u8>,
with_reply_surb: bool,
connection_id: u64,
) -> Option<ServerResponse> {
// the ack control is now responsible for chunking, etc.
let lane = TransmissionLane::ConnectionId(connection_id);
let input_msg = InputMessage::new_fresh(recipient, message, with_reply_surb, lane);
let input_msg = InputMessage::new_fresh(*recipient, message, with_reply_surb, lane);
self.msg_input.unbounded_send(input_msg).unwrap();
None
@@ -125,7 +125,7 @@ impl Handler {
message,
with_reply_surb,
connection_id,
} => self.handle_send(recipient, message, with_reply_surb, connection_id),
} => self.handle_send(&recipient, message, with_reply_surb, connection_id),
ClientRequest::Reply {
message,
reply_surb,
@@ -150,7 +150,7 @@ impl Handler {
response.map(|resp| WsMessage::text(resp.into_text()))
}
fn handle_binary_message(&mut self, msg: Vec<u8>) -> Option<WsMessage> {
fn handle_binary_message(&mut self, msg: &[u8]) -> Option<WsMessage> {
debug!("Handling binary message request");
self.received_response_type = ReceivedResponseType::Binary;
@@ -170,37 +170,11 @@ impl Handler {
// old version of this file.
match raw_request {
WsMessage::Text(text_message) => self.handle_text_message(text_message),
WsMessage::Binary(binary_message) => self.handle_binary_message(binary_message),
WsMessage::Binary(binary_message) => self.handle_binary_message(&binary_message),
_ => None,
}
}
// I'm still not entirely sure why `send_all` requires `TryStream` rather than `Stream`, but
// let's just play along for now
fn prepare_reconstructed_binary(
&self,
reconstructed_messages: Vec<ReconstructedMessage>,
) -> Vec<Result<WsMessage, WsError>> {
reconstructed_messages
.into_iter()
.map(ServerResponse::Received)
.map(|resp| Ok(WsMessage::Binary(resp.into_binary())))
.collect()
}
// I'm still not entirely sure why `send_all` requires `TryStream` rather than `Stream`, but
// let's just play along for now
fn prepare_reconstructed_text(
&self,
reconstructed_messages: Vec<ReconstructedMessage>,
) -> Vec<Result<WsMessage, WsError>> {
reconstructed_messages
.into_iter()
.map(ServerResponse::Received)
.map(|resp| Ok(WsMessage::Text(resp.into_text())))
.collect()
}
async fn push_websocket_received_plaintexts(
&mut self,
reconstructed_messages: Vec<ReconstructedMessage>,
@@ -209,10 +183,8 @@ impl Handler {
// if it's text or binary, but for time being we use the naive assumption that if
// client is sending Message::Text it expects text back. Same for Message::Binary
let response_messages = match self.received_response_type {
ReceivedResponseType::Binary => {
self.prepare_reconstructed_binary(reconstructed_messages)
}
ReceivedResponseType::Text => self.prepare_reconstructed_text(reconstructed_messages),
ReceivedResponseType::Binary => prepare_reconstructed_binary(reconstructed_messages),
ReceivedResponseType::Text => prepare_reconstructed_text(reconstructed_messages),
};
let mut send_stream = futures::stream::iter(response_messages);
@@ -307,3 +279,27 @@ impl Handler {
self.listen_for_requests(reconstructed_receiver).await;
}
}
// I'm still not entirely sure why `send_all` requires `TryStream` rather than `Stream`, but
// let's just play along for now
fn prepare_reconstructed_binary(
reconstructed_messages: Vec<ReconstructedMessage>,
) -> Vec<Result<WsMessage, WsError>> {
reconstructed_messages
.into_iter()
.map(ServerResponse::Received)
.map(|resp| Ok(WsMessage::Binary(resp.into_binary())))
.collect()
}
// I'm still not entirely sure why `send_all` requires `TryStream` rather than `Stream`, but
// let's just play along for now
fn prepare_reconstructed_text(
reconstructed_messages: Vec<ReconstructedMessage>,
) -> Vec<Result<WsMessage, WsError>> {
reconstructed_messages
.into_iter()
.map(ServerResponse::Received)
.map(|resp| Ok(WsMessage::Text(resp.into_text())))
.collect()
}
@@ -62,10 +62,10 @@ impl ClientRequest {
.collect()
}
// SEND_REQUEST_TAG || with_reply || recipient || data_len || data
// SEND_REQUEST_TAG || with_reply || recipient || conn_id || data_len || data
fn deserialize_send(b: &[u8]) -> Result<Self, error::Error> {
// we need to have at least 1 (tag) + 1 (reply flag) + Recipient::LEN + sizeof<u64> bytes
if b.len() < 2 + Recipient::LEN + size_of::<u64>() {
// we need to have at least 1 (tag) + 1 (reply flag) + Recipient::LEN + 2*sizeof<u64> bytes
if b.len() < 2 + Recipient::LEN + 2 * size_of::<u64>() {
return Err(error::Error::new(
ErrorKind::TooShortRequest,
"not enough data provided to recover 'send'".to_string(),
@@ -127,7 +127,7 @@ impl ClientRequest {
}
// REPLY_REQUEST_TAG || surb_len || surb || message_len || message
fn serialize_reply(message: Vec<u8>, reply_surb: ReplySurb) -> Vec<u8> {
fn serialize_reply(message: Vec<u8>, reply_surb: &ReplySurb) -> Vec<u8> {
let reply_surb_bytes = reply_surb.to_bytes();
let surb_len_bytes = (reply_surb_bytes.len() as u64).to_be_bytes();
let message_len_bytes = (message.len() as u64).to_be_bytes();
@@ -225,7 +225,7 @@ impl ClientRequest {
fn serialize_closed_connection(connection_id: u64) -> Vec<u8> {
let conn_id_bytes = connection_id.to_be_bytes();
std::iter::once(CLOSED_CONNECTION_REQUEST_TAG)
.chain(conn_id_bytes.iter().cloned())
.chain(conn_id_bytes.iter().copied())
.collect()
}
@@ -253,7 +253,7 @@ impl ClientRequest {
ClientRequest::Reply {
message,
reply_surb,
} => Self::serialize_reply(message, reply_surb),
} => Self::serialize_reply(message, &reply_surb),
ClientRequest::SelfAddress => Self::serialize_self_address(),
@@ -290,13 +290,13 @@ impl ClientRequest {
CLOSED_CONNECTION_REQUEST_TAG => Ok(Self::deserialize_closed_connection(b)),
n => Err(error::Error::new(
ErrorKind::UnknownRequest,
format!("type {}", n),
format!("type {n}"),
)),
}
}
pub fn try_from_binary(raw_req: Vec<u8>) -> Result<Self, error::Error> {
Self::deserialize(&raw_req)
pub fn try_from_binary(raw_req: &[u8]) -> Result<Self, error::Error> {
Self::deserialize(raw_req)
}
pub fn try_from_text(raw_req: String) -> Result<Self, error::Error> {