From 09bb44146ed988963016038fb6d67565d3283363 Mon Sep 17 00:00:00 2001 From: Martin Thomson Date: Wed, 7 Feb 2024 18:20:11 +1100 Subject: [PATCH] These addresses can now be constants (#1630) * These addresses can now be constants * Missed a few --- neqo-http3/src/connection_client.rs | 9 +-- .../tests/webtransport/mod.rs | 6 +- neqo-http3/tests/webtransport.rs | 6 +- .../src/connection/tests/ackrate.rs | 4 +- .../src/connection/tests/handshake.rs | 16 ++--- .../src/connection/tests/migration.rs | 72 +++++++++---------- neqo-transport/src/connection/tests/mod.rs | 6 +- neqo-transport/src/recovery.rs | 11 ++- test-fixture/src/assertions.rs | 6 +- test-fixture/src/lib.rs | 31 ++++---- 10 files changed, 91 insertions(+), 76 deletions(-) diff --git a/neqo-http3/src/connection_client.rs b/neqo-http3/src/connection_client.rs index 5cc0541c0c..b98533b043 100644 --- a/neqo-http3/src/connection_client.rs +++ b/neqo-http3/src/connection_client.rs @@ -1306,8 +1306,9 @@ mod tests { StreamType, Version, RECV_BUFFER_SIZE, SEND_BUFFER_SIZE, }; use test_fixture::{ - addr, anti_replay, default_server_h3, fixture_init, new_server, now, - CountingConnectionIdGenerator, DEFAULT_ALPN_H3, DEFAULT_KEYS, DEFAULT_SERVER_NAME, + anti_replay, default_server_h3, fixture_init, new_server, now, + CountingConnectionIdGenerator, DEFAULT_ADDR, DEFAULT_ALPN_H3, DEFAULT_KEYS, + DEFAULT_SERVER_NAME, }; use super::{ @@ -1340,8 +1341,8 @@ mod tests { Http3Client::new( DEFAULT_SERVER_NAME, Rc::new(RefCell::new(CountingConnectionIdGenerator::default())), - addr(), - addr(), + DEFAULT_ADDR, + DEFAULT_ADDR, Http3Parameters::default() .connection_parameters( // Disable compatible upgrade, which complicates tests. diff --git a/neqo-http3/src/features/extended_connect/tests/webtransport/mod.rs b/neqo-http3/src/features/extended_connect/tests/webtransport/mod.rs index 51dc47e4c1..3753c3122d 100644 --- a/neqo-http3/src/features/extended_connect/tests/webtransport/mod.rs +++ b/neqo-http3/src/features/extended_connect/tests/webtransport/mod.rs @@ -14,7 +14,7 @@ use neqo_common::event::Provider; use neqo_crypto::AuthenticationStatus; use neqo_transport::{ConnectionParameters, StreamId, StreamType}; use test_fixture::{ - addr, anti_replay, fixture_init, now, CountingConnectionIdGenerator, DEFAULT_ALPN_H3, + anti_replay, fixture_init, now, CountingConnectionIdGenerator, DEFAULT_ADDR, DEFAULT_ALPN_H3, DEFAULT_KEYS, DEFAULT_SERVER_NAME, }; @@ -38,8 +38,8 @@ pub fn default_http3_client(client_params: Http3Parameters) -> Http3Client { Http3Client::new( DEFAULT_SERVER_NAME, Rc::new(RefCell::new(CountingConnectionIdGenerator::default())), - addr(), - addr(), + DEFAULT_ADDR, + DEFAULT_ADDR, client_params, now(), ) diff --git a/neqo-http3/tests/webtransport.rs b/neqo-http3/tests/webtransport.rs index 4e943d86cb..b1e18a5a98 100644 --- a/neqo-http3/tests/webtransport.rs +++ b/neqo-http3/tests/webtransport.rs @@ -15,7 +15,7 @@ use neqo_http3::{ }; use neqo_transport::{StreamId, StreamType}; use test_fixture::{ - addr, anti_replay, fixture_init, now, CountingConnectionIdGenerator, DEFAULT_ALPN_H3, + anti_replay, fixture_init, now, CountingConnectionIdGenerator, DEFAULT_ADDR, DEFAULT_ALPN_H3, DEFAULT_KEYS, DEFAULT_SERVER_NAME, }; @@ -24,8 +24,8 @@ fn connect() -> (Http3Client, Http3Server) { let mut client = Http3Client::new( DEFAULT_SERVER_NAME, Rc::new(RefCell::new(CountingConnectionIdGenerator::default())), - addr(), - addr(), + DEFAULT_ADDR, + DEFAULT_ADDR, Http3Parameters::default().webtransport(true), now(), ) diff --git a/neqo-transport/src/connection/tests/ackrate.rs b/neqo-transport/src/connection/tests/ackrate.rs index 1b83d42acd..f0a1d17cd9 100644 --- a/neqo-transport/src/connection/tests/ackrate.rs +++ b/neqo-transport/src/connection/tests/ackrate.rs @@ -6,7 +6,7 @@ use std::{mem, time::Duration}; -use test_fixture::{addr_v4, assertions}; +use test_fixture::{assertions, DEFAULT_ADDR_V4}; use super::{ super::{ConnectionParameters, ACK_RATIO_SCALE}, @@ -164,7 +164,7 @@ fn migrate_ack_delay() { let mut now = connect_rtt_idle(&mut client, &mut server, DEFAULT_RTT); client - .migrate(Some(addr_v4()), Some(addr_v4()), true, now) + .migrate(Some(DEFAULT_ADDR_V4), Some(DEFAULT_ADDR_V4), true, now) .unwrap(); let client1 = send_something(&mut client, now); diff --git a/neqo-transport/src/connection/tests/handshake.rs b/neqo-transport/src/connection/tests/handshake.rs index a91ecf1b4a..52077c8e88 100644 --- a/neqo-transport/src/connection/tests/handshake.rs +++ b/neqo-transport/src/connection/tests/handshake.rs @@ -18,8 +18,8 @@ use neqo_crypto::{ constants::TLS_CHACHA20_POLY1305_SHA256, generate_ech_keys, AuthenticationStatus, }; use test_fixture::{ - self, addr, assertions, assertions::assert_coalesced_0rtt, datagram, fixture_init, now, - split_datagram, + self, assertions, assertions::assert_coalesced_0rtt, datagram, fixture_init, now, + split_datagram, DEFAULT_ADDR, }; use super::{ @@ -122,8 +122,8 @@ fn no_alpn() { "example.com", &["bad-alpn"], Rc::new(RefCell::new(CountingConnectionIdGenerator::default())), - addr(), - addr(), + DEFAULT_ADDR, + DEFAULT_ADDR, ConnectionParameters::default(), now(), ) @@ -251,8 +251,8 @@ fn chacha20poly1305() { test_fixture::DEFAULT_SERVER_NAME, test_fixture::DEFAULT_ALPN, Rc::new(RefCell::new(EmptyConnectionIdGenerator::default())), - addr(), - addr(), + DEFAULT_ADDR, + DEFAULT_ADDR, ConnectionParameters::default(), now(), ) @@ -730,8 +730,8 @@ fn connect_one_version() { test_fixture::DEFAULT_SERVER_NAME, test_fixture::DEFAULT_ALPN, Rc::new(RefCell::new(CountingConnectionIdGenerator::default())), - addr(), - addr(), + DEFAULT_ADDR, + DEFAULT_ADDR, ConnectionParameters::default().versions(version, vec![version]), now(), ) diff --git a/neqo-transport/src/connection/tests/migration.rs b/neqo-transport/src/connection/tests/migration.rs index 8307a7dd84..7a47ec4156 100644 --- a/neqo-transport/src/connection/tests/migration.rs +++ b/neqo-transport/src/connection/tests/migration.rs @@ -13,9 +13,9 @@ use std::{ use neqo_common::{Datagram, Decoder}; use test_fixture::{ - self, addr, addr_v4, + self, assertions::{assert_v4_path, assert_v6_path}, - fixture_init, new_neqo_qlog, now, + fixture_init, new_neqo_qlog, now, DEFAULT_ADDR, DEFAULT_ADDR_V4, }; use super::{ @@ -94,8 +94,8 @@ fn rebinding_port() { server.stream_close_send(stream_id).unwrap(); let dgram = server.process_output(now()).dgram(); let dgram = dgram.unwrap(); - assert_eq!(dgram.source(), addr()); - assert_eq!(dgram.destination(), new_port(addr())); + assert_eq!(dgram.source(), DEFAULT_ADDR); + assert_eq!(dgram.destination(), new_port(DEFAULT_ADDR)); } /// This simulates an attack where a valid packet is forwarded on @@ -109,7 +109,7 @@ fn path_forwarding_attack() { let mut now = now(); let dgram = send_something(&mut client, now); - let dgram = change_path(&dgram, addr_v4()); + let dgram = change_path(&dgram, DEFAULT_ADDR_V4); server.process_input(&dgram, now); // The server now probes the new (primary) path. @@ -188,7 +188,7 @@ fn migrate_immediate() { let now = now(); client - .migrate(Some(addr_v4()), Some(addr_v4()), true, now) + .migrate(Some(DEFAULT_ADDR_V4), Some(DEFAULT_ADDR_V4), true, now) .unwrap(); let client1 = send_something(&mut client, now); @@ -229,7 +229,7 @@ fn migrate_rtt() { let now = connect_rtt_idle(&mut client, &mut server, RTT); client - .migrate(Some(addr_v4()), Some(addr_v4()), true, now) + .migrate(Some(DEFAULT_ADDR_V4), Some(DEFAULT_ADDR_V4), true, now) .unwrap(); // The RTT might be increased for the new path, so allow a little flexibility. let rtt = client.paths.rtt(); @@ -245,7 +245,7 @@ fn migrate_immediate_fail() { let mut now = now(); client - .migrate(Some(addr_v4()), Some(addr_v4()), true, now) + .migrate(Some(DEFAULT_ADDR_V4), Some(DEFAULT_ADDR_V4), true, now) .unwrap(); let probe = client.process_output(now).dgram().unwrap(); @@ -293,7 +293,7 @@ fn migrate_same() { let now = now(); client - .migrate(Some(addr()), Some(addr()), true, now) + .migrate(Some(DEFAULT_ADDR), Some(DEFAULT_ADDR), true, now) .unwrap(); let probe = client.process_output(now).dgram().unwrap(); @@ -320,7 +320,7 @@ fn migrate_same_fail() { let mut now = now(); client - .migrate(Some(addr()), Some(addr()), true, now) + .migrate(Some(DEFAULT_ADDR), Some(DEFAULT_ADDR), true, now) .unwrap(); let probe = client.process_output(now).dgram().unwrap(); @@ -375,7 +375,7 @@ fn migration(mut client: Connection) { let now = now(); client - .migrate(Some(addr_v4()), Some(addr_v4()), false, now) + .migrate(Some(DEFAULT_ADDR_V4), Some(DEFAULT_ADDR_V4), false, now) .unwrap(); let probe = client.process_output(now).dgram().unwrap(); @@ -449,8 +449,8 @@ fn migration_client_empty_cid() { test_fixture::DEFAULT_SERVER_NAME, test_fixture::DEFAULT_ALPN, Rc::new(RefCell::new(EmptyConnectionIdGenerator::default())), - addr(), - addr(), + DEFAULT_ADDR, + DEFAULT_ADDR, ConnectionParameters::default(), now(), ) @@ -568,22 +568,22 @@ fn preferred_address(hs_client: SocketAddr, hs_server: SocketAddr, preferred: So /// Migration works for a new port number. #[test] fn preferred_address_new_port() { - let a = addr(); + let a = DEFAULT_ADDR; preferred_address(a, a, new_port(a)); } /// Migration works for a new address too. #[test] fn preferred_address_new_address() { - let mut preferred = addr(); + let mut preferred = DEFAULT_ADDR; preferred.set_ip(IpAddr::V6(Ipv6Addr::new(0xfe80, 0, 0, 0, 0, 0, 0, 2))); - preferred_address(addr(), addr(), preferred); + preferred_address(DEFAULT_ADDR, DEFAULT_ADDR, preferred); } /// Migration works for IPv4 addresses. #[test] fn preferred_address_new_port_v4() { - let a = addr_v4(); + let a = DEFAULT_ADDR_V4; preferred_address(a, a, new_port(a)); } @@ -623,7 +623,7 @@ fn preferred_address_ignore_loopback() { /// A preferred address in the wrong address family is ignored. #[test] fn preferred_address_ignore_different_family() { - preferred_address_ignored(PreferredAddress::new_any(Some(addr_v4()), None)); + preferred_address_ignored(PreferredAddress::new_any(Some(DEFAULT_ADDR_V4), None)); } /// Disabling preferred addresses at the client means that it ignores a perfectly @@ -631,7 +631,7 @@ fn preferred_address_ignore_different_family() { #[test] fn preferred_address_disabled_client() { let mut client = new_client(ConnectionParameters::default().disable_preferred_address()); - let mut preferred = addr(); + let mut preferred = DEFAULT_ADDR; preferred.set_ip(IpAddr::V6(Ipv6Addr::new(0xfe80, 0, 0, 0, 0, 0, 0, 2))); let spa = PreferredAddress::new_any(None, Some(preferred)); let mut server = new_server(ConnectionParameters::default().preferred_address(spa)); @@ -643,7 +643,7 @@ fn preferred_address_disabled_client() { fn preferred_address_empty_cid() { fixture_init(); - let spa = PreferredAddress::new_any(None, Some(new_port(addr()))); + let spa = PreferredAddress::new_any(None, Some(new_port(DEFAULT_ADDR))); let res = Connection::new_server( test_fixture::DEFAULT_KEYS, test_fixture::DEFAULT_ALPN, @@ -706,33 +706,33 @@ fn preferred_address_client() { fn migration_invalid_state() { let mut client = default_client(); assert!(client - .migrate(Some(addr()), Some(addr()), false, now()) + .migrate(Some(DEFAULT_ADDR), Some(DEFAULT_ADDR), false, now()) .is_err()); let mut server = default_server(); assert!(server - .migrate(Some(addr()), Some(addr()), false, now()) + .migrate(Some(DEFAULT_ADDR), Some(DEFAULT_ADDR), false, now()) .is_err()); connect_force_idle(&mut client, &mut server); assert!(server - .migrate(Some(addr()), Some(addr()), false, now()) + .migrate(Some(DEFAULT_ADDR), Some(DEFAULT_ADDR), false, now()) .is_err()); client.close(now(), 0, "closing"); assert!(client - .migrate(Some(addr()), Some(addr()), false, now()) + .migrate(Some(DEFAULT_ADDR), Some(DEFAULT_ADDR), false, now()) .is_err()); let close = client.process(None, now()).dgram(); let dgram = server.process(close.as_ref(), now()).dgram(); assert!(server - .migrate(Some(addr()), Some(addr()), false, now()) + .migrate(Some(DEFAULT_ADDR), Some(DEFAULT_ADDR), false, now()) .is_err()); client.process_input(&dgram.unwrap(), now()); assert!(client - .migrate(Some(addr()), Some(addr()), false, now()) + .migrate(Some(DEFAULT_ADDR), Some(DEFAULT_ADDR), false, now()) .is_err()); } @@ -753,32 +753,32 @@ fn migration_invalid_address() { cant_migrate(None, None); // Providing a zero port number isn't valid. - let mut zero_port = addr(); + let mut zero_port = DEFAULT_ADDR; zero_port.set_port(0); cant_migrate(None, Some(zero_port)); cant_migrate(Some(zero_port), None); // An unspecified remote address is bad. - let mut remote_unspecified = addr(); + let mut remote_unspecified = DEFAULT_ADDR; remote_unspecified.set_ip(IpAddr::V6(Ipv6Addr::from(0))); cant_migrate(None, Some(remote_unspecified)); // Mixed address families is bad. - cant_migrate(Some(addr()), Some(addr_v4())); - cant_migrate(Some(addr_v4()), Some(addr())); + cant_migrate(Some(DEFAULT_ADDR), Some(DEFAULT_ADDR_V4)); + cant_migrate(Some(DEFAULT_ADDR_V4), Some(DEFAULT_ADDR)); // Loopback to non-loopback is bad. - cant_migrate(Some(addr()), Some(loopback())); - cant_migrate(Some(loopback()), Some(addr())); + cant_migrate(Some(DEFAULT_ADDR), Some(loopback())); + cant_migrate(Some(loopback()), Some(DEFAULT_ADDR)); assert_eq!( client - .migrate(Some(addr()), Some(loopback()), true, now()) + .migrate(Some(DEFAULT_ADDR), Some(loopback()), true, now()) .unwrap_err(), Error::InvalidMigration ); assert_eq!( client - .migrate(Some(loopback()), Some(addr()), true, now()) + .migrate(Some(loopback()), Some(DEFAULT_ADDR), true, now()) .unwrap_err(), Error::InvalidMigration ); @@ -864,7 +864,7 @@ fn retire_prior_to_migration_failure() { let original_cid = ConnectionId::from(get_cid(&send_something(&mut client, now()))); client - .migrate(Some(addr_v4()), Some(addr_v4()), false, now()) + .migrate(Some(DEFAULT_ADDR_V4), Some(DEFAULT_ADDR_V4), false, now()) .unwrap(); // The client now probes the new path. @@ -919,7 +919,7 @@ fn retire_prior_to_migration_success() { let original_cid = ConnectionId::from(get_cid(&send_something(&mut client, now()))); client - .migrate(Some(addr_v4()), Some(addr_v4()), false, now()) + .migrate(Some(DEFAULT_ADDR_V4), Some(DEFAULT_ADDR_V4), false, now()) .unwrap(); // The client now probes the new path. diff --git a/neqo-transport/src/connection/tests/mod.rs b/neqo-transport/src/connection/tests/mod.rs index 8a999f4048..d958ecd70c 100644 --- a/neqo-transport/src/connection/tests/mod.rs +++ b/neqo-transport/src/connection/tests/mod.rs @@ -18,7 +18,7 @@ use std::{ use enum_map::enum_map; use neqo_common::{event::Provider, qdebug, qtrace, Datagram, Decoder, Role}; use neqo_crypto::{random, AllowZeroRtt, AuthenticationStatus, ResumptionToken}; -use test_fixture::{self, addr, fixture_init, new_neqo_qlog, now}; +use test_fixture::{self, fixture_init, new_neqo_qlog, now, DEFAULT_ADDR}; use super::{Connection, ConnectionError, ConnectionId, Output, State}; use crate::{ @@ -107,8 +107,8 @@ pub fn new_client(params: ConnectionParameters) -> Connection { test_fixture::DEFAULT_SERVER_NAME, test_fixture::DEFAULT_ALPN, Rc::new(RefCell::new(CountingConnectionIdGenerator::default())), - addr(), - addr(), + DEFAULT_ADDR, + DEFAULT_ADDR, params, now(), ) diff --git a/neqo-transport/src/recovery.rs b/neqo-transport/src/recovery.rs index d90989b486..ec1d7e4a3d 100644 --- a/neqo-transport/src/recovery.rs +++ b/neqo-transport/src/recovery.rs @@ -1027,7 +1027,7 @@ mod tests { }; use neqo_common::qlog::NeqoQlog; - use test_fixture::{addr, now}; + use test_fixture::{now, DEFAULT_ADDR}; use super::{ LossRecovery, LossRecoverySpace, PacketNumberSpace, SendProfile, SentPacket, FAST_PTO_SCALE, @@ -1105,7 +1105,14 @@ mod tests { impl Default for Fixture { fn default() -> Self { const CC: CongestionControlAlgorithm = CongestionControlAlgorithm::NewReno; - let mut path = Path::temporary(addr(), addr(), CC, true, NeqoQlog::default(), now()); + let mut path = Path::temporary( + DEFAULT_ADDR, + DEFAULT_ADDR, + CC, + true, + NeqoQlog::default(), + now(), + ); path.make_permanent( None, ConnectionIdEntry::new(0, ConnectionId::from(&[1, 2, 3]), [0; 16]), diff --git a/test-fixture/src/assertions.rs b/test-fixture/src/assertions.rs index 7e772daabf..dd6d0330ef 100644 --- a/test-fixture/src/assertions.rs +++ b/test-fixture/src/assertions.rs @@ -12,7 +12,7 @@ use std::{ use neqo_common::{Datagram, Decoder}; use neqo_transport::{version::WireVersion, Version}; -use crate::{addr, addr_v4}; +use crate::{DEFAULT_ADDR, DEFAULT_ADDR_V4}; const PACKET_TYPE_MASK: u8 = 0b1011_0000; @@ -161,7 +161,7 @@ pub fn assert_path(dgram: &Datagram, path_addr: SocketAddr) { /// /// When the path doesn't use the default v4 socket address at both ends. pub fn assert_v4_path(dgram: &Datagram, padded: bool) { - assert_path(dgram, addr_v4()); + assert_path(dgram, DEFAULT_ADDR_V4); if padded { assert_eq!(dgram.len(), 1357 /* PATH_MTU_V4 */); } @@ -171,7 +171,7 @@ pub fn assert_v4_path(dgram: &Datagram, padded: bool) { /// /// When the path doesn't use the default v6 socket address at both ends. pub fn assert_v6_path(dgram: &Datagram, padded: bool) { - assert_path(dgram, addr()); + assert_path(dgram, DEFAULT_ADDR); if padded { assert_eq!(dgram.len(), 1337 /* PATH_MTU_V6 */); } diff --git a/test-fixture/src/lib.rs b/test-fixture/src/lib.rs index 2c94767a97..96ed335a83 100644 --- a/test-fixture/src/lib.rs +++ b/test-fixture/src/lib.rs @@ -86,26 +86,33 @@ pub const DEFAULT_KEYS: &[&str] = &["key"]; pub const LONG_CERT_KEYS: &[&str] = &["A long cert"]; pub const DEFAULT_ALPN: &[&str] = &["alpn"]; pub const DEFAULT_ALPN_H3: &[&str] = &["h3"]; +pub const DEFAULT_ADDR: SocketAddr = addr(); +pub const DEFAULT_ADDR_V4: SocketAddr = addr_v4(); // Create a default datagram with the given data. #[must_use] pub fn datagram(data: Vec) -> Datagram { - Datagram::new(addr(), addr(), IpTos::default(), Some(128), data) + Datagram::new( + DEFAULT_ADDR, + DEFAULT_ADDR, + IpTos::default(), + Some(128), + data, + ) } /// Create a default socket address. #[must_use] -pub fn addr() -> SocketAddr { - // These could be const functions, but they aren't... +const fn addr() -> SocketAddr { let v6ip = IpAddr::V6(Ipv6Addr::new(0xfe80, 0, 0, 0, 0, 0, 0, 1)); SocketAddr::new(v6ip, 443) } /// An IPv4 version of the default socket address. #[must_use] -pub fn addr_v4() -> SocketAddr { - let localhost_v4 = IpAddr::V4(Ipv4Addr::from(0xc000_0201)); - SocketAddr::new(localhost_v4, addr().port()) +const fn addr_v4() -> SocketAddr { + let v4ip = IpAddr::V4(Ipv4Addr::new(192, 0, 2, 1)); + SocketAddr::new(v4ip, DEFAULT_ADDR.port()) } /// This connection ID generation scheme is the worst, but it doesn't produce collisions. @@ -154,8 +161,8 @@ pub fn new_client(params: ConnectionParameters) -> Connection { DEFAULT_SERVER_NAME, DEFAULT_ALPN, Rc::new(RefCell::new(CountingConnectionIdGenerator::default())), - addr(), - addr(), + DEFAULT_ADDR, + DEFAULT_ADDR, params.ack_ratio(255), // Tests work better with this set this way. now(), ) @@ -258,8 +265,8 @@ pub fn default_http3_client() -> Http3Client { Http3Client::new( DEFAULT_SERVER_NAME, Rc::new(RefCell::new(CountingConnectionIdGenerator::default())), - addr(), - addr(), + DEFAULT_ADDR, + DEFAULT_ADDR, Http3Parameters::default() .max_table_size_encoder(100) .max_table_size_decoder(100) @@ -281,8 +288,8 @@ pub fn http3_client_with_params(params: Http3Parameters) -> Http3Client { Http3Client::new( DEFAULT_SERVER_NAME, Rc::new(RefCell::new(CountingConnectionIdGenerator::default())), - addr(), - addr(), + DEFAULT_ADDR, + DEFAULT_ADDR, params, now(), )