Skip to content

Commit

Permalink
refactor(header): Allow compilation on 32 bit architectures
Browse files Browse the repository at this point in the history
- Don't assume that usize is equivalant to a u64
- Require 32-bit or 64-bit architecture at compile time

BREAKING CHANGE: this changes the data_len field in MsgHeader from usize
to u32
  • Loading branch information
nashley authored and lionel-faber committed Mar 25, 2021
1 parent 1a62311 commit 67ab0a2
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 27 deletions.
2 changes: 1 addition & 1 deletion src/connections.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ async fn handle_endpoint_verification_req(
Ok(Ok(WireMsg::EndpointEchoResp(_)))
);

let message = WireMsg::EndpointVerficationResp(verified);
let message = WireMsg::EndpointVerificationResp(verified);
message.write_to_stream(send_stream).await?;
trace!("Responded to Endpoint verification request");

Expand Down
4 changes: 2 additions & 2 deletions src/endpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ impl Endpoint {
send.send(WireMsg::EndpointVerificationReq(addr)).await?;
let response = WireMsg::read_from_stream(&mut recv.quinn_recv_stream).await?;
match response {
WireMsg::EndpointVerficationResp(valid) => {
WireMsg::EndpointVerificationResp(valid) => {
if valid {
info!("Endpoint verification successful! {} is reachable.", addr);
} else {
Expand Down Expand Up @@ -195,7 +195,7 @@ impl Endpoint {
self.public_addr.unwrap_or(self.local_addr)
}

/// Get our connection adddress to give to others for them to connect to us.
/// Get our connection address to give to others for them to connect to us.
///
/// Attempts to use UPnP to automatically find the public endpoint and forward a port.
/// Will use hard coded contacts to ask for our endpoint. If no contact is given then we'll
Expand Down
13 changes: 8 additions & 5 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ pub enum Error {
/// of the peers provided as a list of contacts.
#[error("Network bootstrap failed")]
BootstrapFailure,
/// No peers/contects found in the bootstrap nodes list.
/// No peers/contacts found in the bootstrap nodes list.
#[error("No nodes/peers found defined for bootstrapping")]
EmptyBootstrapNodesList,
/// The path provided is not valid for the operation.
Expand Down Expand Up @@ -55,7 +55,7 @@ pub enum Error {
NoEchoServerEndpointDefined,
/// Timeout occurred when awaiting for a response from
/// any of the peers contacted for the echo service.
#[error("No response reeived from echo services")]
#[error("No response received from echo services")]
NoEchoServiceResponse,
/// Failure occurred when sending an echo request.
#[error("{0}")]
Expand All @@ -70,7 +70,7 @@ pub enum Error {
#[error("Base64 decode")]
Base64Decode(#[from] base64::DecodeError),
/// An error occurred which could be resolved by changing some config value.
#[error("Configuration {}", 0)]
#[error("Configuration {0}")]
Configuration(String),
/// The message type flag decoded in an incoming stream is invalid/unsupported.
#[error("Invalid message type flag found in message's header: {0}")]
Expand All @@ -79,7 +79,7 @@ pub enum Error {
#[error("Stream write error")]
StreamWrite(#[from] quinn::WriteError),
/// The expected amount of message bytes couldn't be read from the stream.
#[error("Failed to read expected nummber of message bytes: {}", 0)]
#[error("Failed to read expected number of message bytes: {0}")]
StreamRead(#[from] quinn::ReadExactError),
/// Failure when trying to map a new port using IGD for automatic port forwarding.
#[error("Could not use IGD for automatic port forwarding")]
Expand All @@ -95,7 +95,7 @@ pub enum Error {
IgdNotSupported,
/// An error was encountered when trying to either generate
/// or serialise a self-signed certificate.
#[error("Self-signed certificate generation error: {}", 0)]
#[error("Self-signed certificate generation error: {0}")]
CertificateGen(#[from] rcgen::RcgenError),
/// Response message received contains an empty payload.
#[error("Empty response message received from peer")]
Expand All @@ -112,4 +112,7 @@ pub enum Error {
/// Missing connection
#[error("No connection to the dest peer")]
MissingConnection,
/// A supposedly impossible internal error occurred
#[error("Unexpected internal error: {0}")]
UnexpectedError(String),
}
53 changes: 34 additions & 19 deletions src/wire_msg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use crate::{
use bytes::Bytes;
use log::trace;
use serde::{Deserialize, Serialize};
use std::convert::TryFrom;
use std::{fmt, net::SocketAddr};

const MSG_HEADER_LEN: usize = 9;
Expand All @@ -25,7 +26,7 @@ pub enum WireMsg {
EndpointEchoReq,
EndpointEchoResp(SocketAddr),
EndpointVerificationReq(SocketAddr),
EndpointVerficationResp(bool),
EndpointVerificationResp(bool),
UserMsg(Bytes),
}

Expand All @@ -41,7 +42,22 @@ impl WireMsg {

let msg_header = MsgHeader::from_bytes(header_bytes);
log::debug!("reading data of length: {}", msg_header.data_len());
let mut data: Vec<u8> = vec![0; msg_header.data_len()];
// https://github.com/rust-lang/rust/issues/70460 for work on a cleaner alternative:
#[cfg(not(any(target_pointer_width = "32", target_pointer_width = "64")))]
{
compile_error!("You need an architecture capable of addressing 32-bit pointers");
}
let data_length = match usize::try_from(msg_header.data_len()) {
Ok(s) => s,
Err(_) => {
return Err(Error::UnexpectedError(format!(
// this should never happen due to preceding compilation error
"unable to convert u32 '{}' to usize",
msg_header.data_len()
)));
}
};
let mut data: Vec<u8> = vec![0; data_length];
let msg_flag = msg_header.usr_msg_flag();

recv.read_exact(&mut data).await?;
Expand Down Expand Up @@ -91,7 +107,7 @@ impl fmt::Display for WireMsg {
WireMsg::EndpointVerificationReq(ref sa) => {
write!(f, "WireMsg::EndpointVerificationReq({})", sa)
}
WireMsg::EndpointVerficationResp(valid) => write!(
WireMsg::EndpointVerificationResp(valid) => write!(
f,
"WireMsg::EndpointEchoResp({})",
if valid { "Valid" } else { "Invalid" }
Expand All @@ -107,27 +123,26 @@ impl fmt::Display for WireMsg {
#[derive(Debug)]
struct MsgHeader {
version: u16,
data_len: usize,
data_len: u32,
usr_msg_flag: u8,
#[allow(unused)]
reserved: [u8; 2],
}

impl MsgHeader {
pub fn new(msg: &Bytes, usr_msg_flag: u8) -> Result<Self> {
let data_len = msg.len();
if data_len > u32::MAX as usize {
return Err(Error::MaxLengthExceeded(data_len));
match u32::try_from(msg.len()) {
Err(_) => Err(Error::MaxLengthExceeded(msg.len())),
Ok(data_len) => Ok(Self {
version: MSG_PROTOCOL_VERSION,
data_len,
usr_msg_flag,
reserved: [0, 0],
}),
}
Ok(Self {
version: MSG_PROTOCOL_VERSION,
data_len,
usr_msg_flag,
reserved: [0, 0],
})
}

pub fn data_len(&self) -> usize {
pub fn data_len(&self) -> u32 {
self.data_len
}

Expand All @@ -141,10 +156,10 @@ impl MsgHeader {
[
version[0],
version[1],
data_len[4],
data_len[5],
data_len[6],
data_len[7],
data_len[0],
data_len[1],
data_len[2],
data_len[3],
self.usr_msg_flag,
0,
0,
Expand All @@ -153,7 +168,7 @@ impl MsgHeader {

pub fn from_bytes(bytes: [u8; MSG_HEADER_LEN]) -> Self {
let version = u16::from_be_bytes([bytes[0], bytes[1]]);
let data_len = usize::from_be_bytes([0, 0, 0, 0, bytes[2], bytes[3], bytes[4], bytes[5]]);
let data_len = u32::from_be_bytes([bytes[2], bytes[3], bytes[4], bytes[5]]);
let usr_msg_flag = bytes[6];
Self {
version,
Expand Down

0 comments on commit 67ab0a2

Please sign in to comment.