diff --git a/src/h3/lookup.rs b/src/h3/lookup.rs index ee5c65a..8dfd84e 100644 --- a/src/h3/lookup.rs +++ b/src/h3/lookup.rs @@ -403,14 +403,14 @@ mod tests { } #[test] - fn lookup_records_select_primary_group() { + fn lookup_records_selects_first_server_ordered_group() { let response = response_for( "demo.dhttp.net", vec![ - direct("192.0.2.20:4433", false, 1), direct("192.0.2.10:4433", true, 2), direct("192.0.2.11:4433", true, 2), direct("192.0.2.30:4433", true, 3), + direct("192.0.2.20:4433", false, 1), ], ); diff --git a/src/publishers/address.rs b/src/publishers/address.rs index a20cef7..11e977a 100644 --- a/src/publishers/address.rs +++ b/src/publishers/address.rs @@ -334,10 +334,8 @@ fn public_endpoints_from_iface(network: &Network, iface: &BindInterface) -> Vec< .values() .filter_map(|client| { let outer = client.get_outer_addr()?.ok()?; - let bound = current.bound_addr().ok()?; match client.get_nat_type() { Some(Ok(nat_type)) => Some(publish_endpoint_from_stun( - bound, client.agent_addr(), outer, nat_type, @@ -373,12 +371,11 @@ fn public_endpoints_from_iface(network: &Network, iface: &BindInterface) -> Vec< #[cfg(feature = "dquic-network")] fn publish_endpoint_from_stun( - bound: SocketAddr, agent: SocketAddr, outer: SocketAddr, nat_type: NatType, ) -> EndpointAddr { - if nat_type == NatType::FullCone && bound == outer { + if nat_type == NatType::FullCone { EndpointAddr::direct(outer) } else { EndpointAddr::with_agent(agent, outer) @@ -476,14 +473,13 @@ mod tests { #[cfg(feature = "dquic-network")] #[test] - fn full_cone_nat_endpoint_preserves_agent_when_outer_differs_from_bound_addr() { - let bound = "10.110.0.10:45635".parse().expect("valid bound addr"); + fn full_cone_nat_endpoint_is_direct_when_public_addr_differs_from_bound_addr() { let agent = "10.10.0.2:20004".parse().expect("valid agent addr"); let outer = "10.10.0.10:45635".parse().expect("valid outer addr"); - let endpoint = publish_endpoint_from_stun(bound, agent, outer, NatType::FullCone); + let endpoint = publish_endpoint_from_stun(agent, outer, NatType::FullCone); - assert_eq!(endpoint, EndpointAddr::with_agent(agent, outer)); + assert_eq!(endpoint, EndpointAddr::direct(outer)); } #[cfg(feature = "dquic-network")] @@ -492,8 +488,19 @@ mod tests { let bound = "10.10.0.100:45635".parse().expect("valid bound addr"); let agent = "10.10.0.2:20004".parse().expect("valid agent addr"); - let endpoint = publish_endpoint_from_stun(bound, agent, bound, NatType::FullCone); + let endpoint = publish_endpoint_from_stun(agent, bound, NatType::FullCone); assert_eq!(endpoint, EndpointAddr::direct(bound)); } + + #[cfg(feature = "dquic-network")] + #[test] + fn non_full_cone_endpoint_preserves_agent() { + let agent = "10.10.0.2:20004".parse().expect("valid agent addr"); + let outer = "10.10.0.10:45635".parse().expect("valid outer addr"); + + let endpoint = publish_endpoint_from_stun(agent, outer, NatType::RestrictedCone); + + assert_eq!(endpoint, EndpointAddr::with_agent(agent, outer)); + } } diff --git a/src/resolvers.rs b/src/resolvers.rs index 0f014b7..7aca24d 100644 --- a/src/resolvers.rs +++ b/src/resolvers.rs @@ -379,14 +379,6 @@ impl Resolvers { return Err(ResolversError { errors }); } - groups.sort_by_key(|group| { - let primary_rank = match group.chain.kind() { - dhttp_identity::certificate::CertificateChainKind::Primary => 0, - dhttp_identity::certificate::CertificateChainKind::Secondary => 1, - }; - (primary_rank, group.chain.sequence().get()) - }); - Ok(crate::resolvers::endpoint_candidates::EndpointCandidates { groups }) } @@ -625,15 +617,15 @@ mod tests { #[cfg(feature = "resolvers")] #[tokio::test] - async fn aggregate_endpoint_candidates_merge_supported_resolvers() { + async fn aggregate_endpoint_candidates_preserve_resolver_order() { let resolvers = Resolvers::new() .with_candidate_resolver(Arc::new(CandidateResolver { label: "a", - sequence: 0, + sequence: 1, })) .with_candidate_resolver(Arc::new(CandidateResolver { label: "b", - sequence: 1, + sequence: 0, })); let candidates = resolvers @@ -642,8 +634,8 @@ mod tests { .expect("candidate lookup succeeds"); assert_eq!(candidates.groups.len(), 2); - assert_eq!(candidates.groups[0].chain.to_string(), "primary:0"); - assert_eq!(candidates.groups[1].chain.to_string(), "primary:1"); + assert_eq!(candidates.groups[0].chain.to_string(), "primary:1"); + assert_eq!(candidates.groups[1].chain.to_string(), "primary:0"); } #[cfg(feature = "resolvers")] diff --git a/src/resolvers/endpoint_candidates.rs b/src/resolvers/endpoint_candidates.rs index 16c8164..cb40df5 100644 --- a/src/resolvers/endpoint_candidates.rs +++ b/src/resolvers/endpoint_candidates.rs @@ -1,6 +1,6 @@ use std::io; -use dhttp_identity::certificate::{CertificateChainKey, CertificateChainKind}; +use dhttp_identity::certificate::CertificateChainKey; use dquic::{ qbase::net::addr::EndpointAddr as DquicEndpointAddr, qresolve::{Resolve, Source}, @@ -75,14 +75,6 @@ pub(crate) fn grouped_endpoint_candidates( } } - groups.sort_by_key(|(chain_key, _)| { - let primary_rank = match chain_key.kind() { - CertificateChainKind::Primary => 0, - CertificateChainKind::Secondary => 1, - }; - (primary_rank, chain_key.sequence().get()) - }); - groups } @@ -105,7 +97,7 @@ fn effective_chain_key( mod tests { use std::net::SocketAddrV4; - use dhttp_identity::certificate::CertificateSequence; + use dhttp_identity::certificate::{CertificateChainKind, CertificateSequence}; use super::*; @@ -118,11 +110,11 @@ mod tests { } #[test] - fn grouping_returns_multiple_primary_sequences() { + fn grouping_preserves_input_order_between_primary_sequences() { let groups = grouped_endpoint_candidates([ TaggedEndpointCandidate { tag: "wifi", - record: direct("192.0.2.10:4433", true, 0), + record: direct("192.0.2.10:4433", true, 2), fallback_chain_key: None, }, TaggedEndpointCandidate { @@ -132,13 +124,13 @@ mod tests { }, TaggedEndpointCandidate { tag: "wifi-backup", - record: direct("192.0.2.11:4433", true, 0), + record: direct("192.0.2.11:4433", true, 2), fallback_chain_key: None, }, ]); assert_eq!(groups.len(), 2); - assert_eq!(groups[0].0.to_string(), "primary:0"); + assert_eq!(groups[0].0.to_string(), "primary:2"); assert_eq!(groups[0].1.len(), 2); assert_eq!(groups[1].0.to_string(), "primary:1"); assert_eq!(groups[1].1.len(), 1); diff --git a/src/resolvers/endpoint_group.rs b/src/resolvers/endpoint_group.rs index 4cbe4ab..9902525 100644 --- a/src/resolvers/endpoint_group.rs +++ b/src/resolvers/endpoint_group.rs @@ -100,21 +100,17 @@ mod tests { } #[test] - fn selected_endpoint_addrs_prefers_primary_chain_key_group() { + fn selected_endpoint_addrs_uses_first_chain_key_group() { let secondary = direct("192.0.2.20:4433", false, 0); let primary_a = direct("192.0.2.10:4433", true, 2); let primary_b = direct("192.0.2.11:4433", true, 2); let selected = super::selected_endpoint_addrs([secondary, primary_a, primary_b]); - assert_eq!(selected.len(), 2); + assert_eq!(selected.len(), 1); assert_eq!( selected[0], - dquic::qbase::net::addr::EndpointAddr::direct("192.0.2.10:4433".parse().unwrap()) - ); - assert_eq!( - selected[1], - dquic::qbase::net::addr::EndpointAddr::direct("192.0.2.11:4433".parse().unwrap()) + dquic::qbase::net::addr::EndpointAddr::direct("192.0.2.20:4433".parse().unwrap()) ); }