diff --git a/mullvad-relay-selector/src/relay_selector/matcher.rs b/mullvad-relay-selector/src/relay_selector/matcher.rs index ad46353af659..71edb0fac122 100644 --- a/mullvad-relay-selector/src/relay_selector/matcher.rs +++ b/mullvad-relay-selector/src/relay_selector/matcher.rs @@ -69,17 +69,18 @@ impl RelayMatcher { .filter(|relay| self.endpoint_matcher.is_matching_relay(relay)); // The last filtering to be done is on the `include_in_country` attribute found on each - // relay. A regular, user-facing relay will have `include_in_country` set to true. - // If a relay has `include_in_country` set to false, they are purposely hidden than - // other relays. We should only consider those if there are no regular candidates left. - let ignore_include_in_country = !shortlist.clone().any(|relay| relay.include_in_country); - shortlist - .filter(|relay| { - self.locations - .matches_with_opts(relay, ignore_include_in_country) - }) - .cloned() - .collect() + // relay. When the location constraint is based on country, a relay which has `include_in_country` + // set to true should always be prioritized over relays which has this flag set to false. + // We should only consider relays with `include_in_country` set to false if there are no + // other candidates left. + if !shortlist.clone().any(|relay| relay.include_in_country) { + shortlist.cloned().collect() + } else { + shortlist + .filter(|relay| filter_on_include_in_country(&self.locations, relay)) + .cloned() + .collect() + } } } @@ -211,8 +212,20 @@ pub const fn filter_on_active(relay: &Relay) -> bool { /// Returns whether `relay` satisfy the location constraint posed by `filter`. pub fn filter_on_location(filter: &Constraint, relay: &Relay) -> bool { - let ignore_include_in_countries = true; - filter.matches_with_opts(relay, ignore_include_in_countries) + filter.matches_with_opts(relay, true) +} + +/// Returns whether `relay` has the `include_in_country` flag set to true. +/// +/// # Note +/// This filter only applies if the underlying [location constraint][`ResolvedLocationConstraint`] +/// is based on country. I.e., this filter does not have any effect if the underlying constraint +/// is scoped to a specific hostname or city. +pub fn filter_on_include_in_country( + filter: &Constraint, + relay: &Relay, +) -> bool { + filter.matches_with_opts(relay, false) } /// Returns whether `relay` satisfy the ownership constraint posed by `filter`. diff --git a/mullvad-types/src/relay_constraints.rs b/mullvad-types/src/relay_constraints.rs index 02e7807e1485..742ad6e17b3c 100644 --- a/mullvad-types/src/relay_constraints.rs +++ b/mullvad-types/src/relay_constraints.rs @@ -160,6 +160,15 @@ impl Set> for Constraint { + /// # Note + /// + /// The following statements are true iff `self` is based on [country][`GeographicLocationConstraint::Country`]. + /// + /// * If `ignore_include_in_country` is true, the outcome of `matches_with_opts` is only based on the geographical location of `relay`. + /// + /// * If `ignore_include_in_country` is false, any [relay][`Relay`] which has the + /// `include_in_country` property set to false will cause `matches_with_opts` to return false. + /// Otherwise, the outcome is based on the geographical location of `relay`. pub fn matches_with_opts(&self, relay: &Relay, ignore_include_in_country: bool) -> bool { match self { Constraint::Any => true, @@ -347,6 +356,16 @@ impl GeographicLocationConstraint { GeographicLocationConstraint::Hostname(country.into(), city.into(), hostname.into()) } + /// # Note + /// + /// The following statements are true iff `self` is based on [country][`GeographicLocationConstraint::Country`]. + /// + /// * If `ignore_include_in_country` is true, the `include_in_country` property of `relay` is + /// disregarded. The outcome of `matches_with_opts` is only based on the geographical location of `relay`. + /// + /// * If `ignore_include_in_country` is false, any [relay][`Relay`] which has the + /// `include_in_country` property set to false will cause `matches_with_opts` to return false + /// regardless of the geographical location of `relay`. pub fn matches_with_opts(&self, relay: &Relay, ignore_include_in_country: bool) -> bool { match self { GeographicLocationConstraint::Country(ref country) => {