Skip to content

Commit

Permalink
Fix zero port error (#5021)
Browse files Browse the repository at this point in the history 
* Fix zero port error and add tests

* Tweak indentation

* assign 0 to quic_port if tcp_port == 0

* Remove unnecessary deps
  • Loading branch information
@ackintosh
ackintosh authored Jan 16, 2024
1 parent 0613eb7 commit e10e4b7
Show file tree
Hide file tree
Showing 2 changed files with 109 additions and 6 deletions.
20 changes: 14 additions & 6 deletions beacon_node/src/config.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -973,13 +973,13 @@ pub fn parse_listening_addresses(
.then(unused_port::unused_udp6_port)
.transpose()?
.or(maybe_disc_port)
.unwrap_or(port);
.unwrap_or(tcp_port);

let quic_port = use_zero_ports
.then(unused_port::unused_udp6_port)
.transpose()?
.or(maybe_quic_port)
.unwrap_or(port + 1);
.unwrap_or(if tcp_port == 0 { 0 } else { tcp_port + 1 });

ListenAddress::V6(lighthouse_network::ListenAddr {
addr: ipv6,
Expand All @@ -1002,14 +1002,14 @@ pub fn parse_listening_addresses(
.then(unused_port::unused_udp4_port)
.transpose()?
.or(maybe_disc_port)
.unwrap_or(port);
.unwrap_or(tcp_port);
// use zero ports if required. If not, use the specific quic port. If none given, use
// the tcp port + 1.
let quic_port = use_zero_ports
.then(unused_port::unused_udp4_port)
.transpose()?
.or(maybe_quic_port)
.unwrap_or(port + 1);
.unwrap_or(if tcp_port == 0 { 0 } else { tcp_port + 1 });

ListenAddress::V4(lighthouse_network::ListenAddr {
addr: ipv4,
Expand All @@ -1032,7 +1032,11 @@ pub fn parse_listening_addresses(
.then(unused_port::unused_udp4_port)
.transpose()?
.or(maybe_quic_port)
.unwrap_or(port + 1);
.unwrap_or(if ipv4_tcp_port == 0 {
0
} else {
ipv4_tcp_port + 1
});

// Defaults to 9090 when required
let ipv6_tcp_port = use_zero_ports
Expand All @@ -1048,7 +1052,11 @@ pub fn parse_listening_addresses(
.then(unused_port::unused_udp6_port)
.transpose()?
.or(maybe_quic6_port)
.unwrap_or(ipv6_tcp_port + 1);
.unwrap_or(if ipv6_tcp_port == 0 {
0
} else {
ipv6_tcp_port + 1
});

ListenAddress::DualStack(
lighthouse_network::ListenAddr {
Expand Down
95 changes: 95 additions & 0 deletions lighthouse/tests/beacon_node.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -859,6 +859,23 @@ fn network_port_flag_over_ipv4() {
listen_addr.quic_port,
listen_addr.tcp_port
)),
// quic_port should be 0 if tcp_port is given as 0.
Some((port, 0, port))
);
});

let port = 9000;
CommandLineTest::new()
.flag("port", Some(port.to_string().as_str()))
.run()
.with_config(|config| {
assert_eq!(
config.network.listen_addrs().v4().map(|listen_addr| (
listen_addr.disc_port,
listen_addr.quic_port,
listen_addr.tcp_port
)),
// quic_port should be (tcp_port + 1) if tcp_port is given as non-zero.
Some((port, port + 1, port))
);
});
Expand All @@ -877,8 +894,86 @@ fn network_port_flag_over_ipv6() {
listen_addr.quic_port,
listen_addr.tcp_port
)),
// quic_port should be 0 if tcp_port is given as 0.
Some((port, 0, port))
);
});

let port = 9000;
CommandLineTest::new()
.flag("listen-address", Some("::1"))
.flag("port", Some(port.to_string().as_str()))
.run()
.with_config(|config| {
assert_eq!(
config.network.listen_addrs().v6().map(|listen_addr| (
listen_addr.disc_port,
listen_addr.quic_port,
listen_addr.tcp_port
)),
// quic_port should be (tcp_port + 1) if tcp_port is given as non-zero.
Some((port, port + 1, port))
);
});
}
#[test]
fn network_port_flag_over_ipv4_and_ipv6() {
let port = 0;
let port6 = 0;
CommandLineTest::new()
.flag("listen-address", Some("127.0.0.1"))
.flag("listen-address", Some("::1"))
.flag("port", Some(port.to_string().as_str()))
.flag("port6", Some(port6.to_string().as_str()))
.run()
.with_config(|config| {
assert_eq!(
config.network.listen_addrs().v4().map(|listen_addr| (
listen_addr.disc_port,
listen_addr.quic_port,
listen_addr.tcp_port
)),
// quic_port should be 0 if tcp_port is given as 0.
Some((port, 0, port))
);
assert_eq!(
config.network.listen_addrs().v6().map(|listen_addr| (
listen_addr.disc_port,
listen_addr.quic_port,
listen_addr.tcp_port
)),
// quic_port should be 0 if tcp_port is given as 0.
Some((port6, 0, port6))
);
});

let port = 19000;
let port6 = 29000;
CommandLineTest::new()
.flag("listen-address", Some("127.0.0.1"))
.flag("listen-address", Some("::1"))
.flag("port", Some(port.to_string().as_str()))
.flag("port6", Some(port6.to_string().as_str()))
.run()
.with_config(|config| {
assert_eq!(
config.network.listen_addrs().v4().map(|listen_addr| (
listen_addr.disc_port,
listen_addr.quic_port,
listen_addr.tcp_port
)),
// quic_port should be (tcp_port + 1) if tcp_port is given as non-zero.
Some((port, port + 1, port))
);
assert_eq!(
config.network.listen_addrs().v6().map(|listen_addr| (
listen_addr.disc_port,
listen_addr.quic_port,
listen_addr.tcp_port
)),
// quic_port should be (tcp_port + 1) if tcp_port is given as non-zero.
Some((port6, port6 + 1, port6))
);
});
}
#[test]
Expand Down

0 comments on commit e10e4b7

Please sign in to comment.