Skip to content

Commit

Permalink
refactor!: Remove random port fallback
Browse files Browse the repository at this point in the history
This is a bit 'too clever' for this library, and it's unlikely that this
fallback was really being used (it would usually be left unspecified, so
always be random).

BREAKING CHANGE: This is a breaking behavioural change. It is now up to
the caller if they want to retry with a different port.
  • Loading branch information
Chris Connelly authored and connec committed Aug 27, 2021
1 parent 2c781c2 commit 9b1612a
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 30 deletions.
32 changes: 3 additions & 29 deletions src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,7 @@ use futures::future;
use std::net::{SocketAddr, UdpSocket};
use std::path::PathBuf;
use std::{collections::HashSet, marker::PhantomData};
use tracing::{debug, error, info, trace};

/// In the absence of a port supplied by the user via the config we will first try using this
/// before using a random port.
const DEFAULT_PORT_TO_TRY: u16 = 0;
use tracing::{debug, error, trace};

/// Default duration of a UPnP lease, in seconds.
pub(crate) const DEFAULT_UPNP_LEASE_DURATION_SEC: u32 = 120;
Expand All @@ -35,7 +31,6 @@ const MAIDSAFE_DOMAIN: &str = "maidsafe.net";
#[derive(Debug, Clone)]
pub struct QuicP2p<I: ConnId> {
local_addr: SocketAddr,
allow_random_port: bool,
bootstrap_cache: BootstrapCache,
endpoint_cfg: quinn::ServerConfig,
client_cfg: quinn::ClientConfig,
Expand Down Expand Up @@ -85,10 +80,7 @@ impl<I: ConnId> QuicP2p<I> {
cfg
);

let (port, allow_random_port) = cfg
.local_port
.map(|p| (p, false))
.unwrap_or((DEFAULT_PORT_TO_TRY, true));
let port = cfg.local_port.unwrap_or_default();

let ip = cfg.local_ip.ok_or(Error::UnspecifiedLocalIp)?;

Expand Down Expand Up @@ -137,7 +129,6 @@ impl<I: ConnId> QuicP2p<I> {

Ok(Self {
local_addr: SocketAddr::new(ip, port),
allow_random_port,
bootstrap_cache,
endpoint_cfg,
client_cfg,
Expand Down Expand Up @@ -255,11 +246,7 @@ impl<I: ConnId> QuicP2p<I> {
.cloned()
.collect();

let (quinn_endpoint, quinn_incoming) = bind(
self.endpoint_cfg.clone(),
self.local_addr,
self.allow_random_port,
)?;
let (quinn_endpoint, quinn_incoming) = bind(self.endpoint_cfg.clone(), self.local_addr)?;

trace!(
"Bound endpoint to local address: {}",
Expand Down Expand Up @@ -340,25 +327,12 @@ impl<I: ConnId> QuicP2p<I> {
pub(crate) fn bind(
endpoint_cfg: quinn::ServerConfig,
local_addr: SocketAddr,
allow_random_port: bool,
) -> Result<(quinn::Endpoint, quinn::Incoming)> {
let mut endpoint_builder = quinn::Endpoint::builder();
let _ = endpoint_builder.listen(endpoint_cfg);

match UdpSocket::bind(&local_addr) {
Ok(udp) => endpoint_builder.with_socket(udp).map_err(Error::Endpoint),
Err(err) if allow_random_port => {
info!(
"Failed to bind to local address: {} - Error: {}. Trying random port instead.",
local_addr, err
);
let bind_addr = SocketAddr::new(local_addr.ip(), 0);

endpoint_builder.bind(&bind_addr).map_err(|e| {
error!("Failed to bind to random port too: {}", e);
Error::Endpoint(e)
})
}
Err(err) => {
error!("{}", err);
Err(Error::CannotAssignPort(local_addr.port()))
Expand Down
2 changes: 1 addition & 1 deletion src/tests/quinn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ impl Peer {
DEFAULT_KEEP_ALIVE_INTERVAL_MSEC,
)?;

let (endpoint, mut incoming) = bind(endpoint_cfg, "127.0.0.1:0".parse()?, true)?;
let (endpoint, mut incoming) = bind(endpoint_cfg, "127.0.0.1:0".parse()?)?;

let (message_tx, message_rx) = if channel_size == 0 {
let (message_tx, message_rx) = unbounded_channel();
Expand Down

0 comments on commit 9b1612a

Please sign in to comment.