From ce14e40968fcc1ed3257d240f0ed55923e15f290 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jon=20H=C3=A4ggblad?= Date: Tue, 15 Nov 2022 11:24:29 +0100 Subject: [PATCH 1/2] websocket-requests: fix deserialize length check --- clients/native/websocket-requests/src/requests.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clients/native/websocket-requests/src/requests.rs b/clients/native/websocket-requests/src/requests.rs index ea44059250..565a184df9 100644 --- a/clients/native/websocket-requests/src/requests.rs +++ b/clients/native/websocket-requests/src/requests.rs @@ -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 { - // we need to have at least 1 (tag) + 1 (reply flag) + Recipient::LEN + sizeof bytes - if b.len() < 2 + Recipient::LEN + size_of::() { + // we need to have at least 1 (tag) + 1 (reply flag) + Recipient::LEN + 2*sizeof bytes + if b.len() < 2 + Recipient::LEN + 2 * size_of::() { return Err(error::Error::new( ErrorKind::TooShortRequest, "not enough data provided to recover 'send'".to_string(), From 0d71ac5e75580a1c0acb557d35d51916b29b9e92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jon=20H=C3=A4ggblad?= Date: Tue, 15 Nov 2022 11:24:55 +0100 Subject: [PATCH 2/2] websocket-requests: fix lints in file --- clients/native/src/client/mod.rs | 2 +- clients/native/src/websocket/handler.rs | 70 +++++++++---------- .../native/websocket-requests/src/requests.rs | 12 ++-- 3 files changed, 40 insertions(+), 44 deletions(-) diff --git a/clients/native/src/client/mod.rs b/clients/native/src/client/mod.rs index df77a7fb77..f2aee9829f 100644 --- a/clients/native/src/client/mod.rs +++ b/clients/native/src/client/mod.rs @@ -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); diff --git a/clients/native/src/websocket/handler.rs b/clients/native/src/websocket/handler.rs index f43fbca4c8..9830b6fdd0 100644 --- a/clients/native/src/websocket/handler.rs +++ b/clients/native/src/websocket/handler.rs @@ -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, with_reply_surb: bool, connection_id: u64, ) -> Option { // 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) -> Option { + fn handle_binary_message(&mut self, msg: &[u8]) -> Option { 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, - ) -> Vec> { - 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, - ) -> Vec> { - 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, @@ -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, +) -> Vec> { + 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, +) -> Vec> { + reconstructed_messages + .into_iter() + .map(ServerResponse::Received) + .map(|resp| Ok(WsMessage::Text(resp.into_text()))) + .collect() +} diff --git a/clients/native/websocket-requests/src/requests.rs b/clients/native/websocket-requests/src/requests.rs index 565a184df9..f681b4264c 100644 --- a/clients/native/websocket-requests/src/requests.rs +++ b/clients/native/websocket-requests/src/requests.rs @@ -127,7 +127,7 @@ impl ClientRequest { } // REPLY_REQUEST_TAG || surb_len || surb || message_len || message - fn serialize_reply(message: Vec, reply_surb: ReplySurb) -> Vec { + fn serialize_reply(message: Vec, reply_surb: &ReplySurb) -> Vec { 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 { 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) -> Result { - Self::deserialize(&raw_req) + pub fn try_from_binary(raw_req: &[u8]) -> Result { + Self::deserialize(raw_req) } pub fn try_from_text(raw_req: String) -> Result {