Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Relayselector not resolving locations under certain constraints #6062

Merged

Conversation

rablador
Copy link
Contributor

@rablador rablador commented Apr 3, 2024

Steps to reproduce the issue:

  1. Connect to a relay
  2. Filter providers, select 31173
  3. Tap switch location
  4. Select USA as a country
  5. Enter blocked mode
  6. Tap switch location again
  7. Add 100 TB as a provider
  8. Select USA
  9. Enter blocked connection, despite having locations match in the USA

The underlying issue is that relays can be set to not be included in a country when a user sets a country location constraint, i.e. there are relays that should not be considered for selection even though they'd match a location constraint. However, it can be the case that all relays provided by a specific provider are marked to not be included in a country, which is the case for 100TB and US. In this case, the relay selector should disregard the flag and allow the relays anyway. The iOS behavior here should mimic desktop - https://github.com/mullvad/mullvadvpn-app/blob/main/mullvad-relay-selector/src/relay_selector/matcher.rs#L17-L62


This change is Reviewable

@rablador rablador added bug iOS Issues related to iOS labels Apr 3, 2024
Copy link

linear bot commented Apr 3, 2024

@rablador rablador changed the title Relayselector cannot resolve locations under certain constraints Fix Relayselector not resolving locations under certain constraints Apr 3, 2024
@rablador rablador requested review from buggmagnet and acb-mv April 3, 2024 15:00
@rablador rablador force-pushed the relayselector-cannot-resolve-locations-under-certain-ios-580 branch from 592105d to 57d3712 Compare April 3, 2024 15:01
Copy link
Collaborator

@pinkisemils pinkisemils left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

Copy link
Contributor

@acb-mv acb-mv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

Copy link
Contributor

@buggmagnet buggmagnet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@buggmagnet buggmagnet force-pushed the relayselector-cannot-resolve-locations-under-certain-ios-580 branch from 57d3712 to f3afd74 Compare April 8, 2024 08:23
@buggmagnet buggmagnet merged commit dd47834 into main Apr 8, 2024
6 of 7 checks passed
@buggmagnet buggmagnet deleted the relayselector-cannot-resolve-locations-under-certain-ios-580 branch April 8, 2024 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug iOS Issues related to iOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants