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 proxies added doubles sometimes #2397

Merged
merged 1 commit into from
Nov 14, 2024
Merged

fix proxies added doubles sometimes #2397

merged 1 commit into from
Nov 14, 2024

Conversation

r10s
Copy link
Member

@r10s r10s commented Nov 14, 2024

this PR fixes the duplicated proxy entries mentioned at #2388 (review) - we first thought it is a core-bug, but eventually was not.

this was a hard to find bug ... i hope i could explain it correctly :)

core normalizes proxy-url
when calling set_config_from_qr(URL_ORG).

by calling set_config(URL_ORG) instead of URL_NORM=get_config() after adding, we discarded the normalisation.

selecting later by set_config_from_qr(URL_ORG) finally results in URL_ORG as well as URL_NORM being added to the list.

one could argue,
that normalisation should also take place at set_config(), however, this is not the case.

it also seems more resilient to refresh the list on adding, so that really the state and the URLs used on core are shown.

core normalizes proxy-url
when calling `set_config_from_qr(URL_ORG)`.

by calling `set_config(URL_ORG)` instead of `URL_NORM=get_config()` after adding,
we discarded the normalisation.

when selecting later by `set_config_from_qr(URL_ORG)`
finally results in `URL_ORG` as well as `URL_NORM` being added to the list.

one could argue,
that normalisation should also take place at `set_config()`, however,
this is not the case.

it also seems more resilient to refresh the list on adding,
so that really the state and the URLs used on core are shown.
@r10s r10s requested a review from zeitschlag November 14, 2024 16:46
@r10s r10s added the bug label Nov 14, 2024
Copy link
Collaborator

@zeitschlag zeitschlag left a comment

Choose a reason for hiding this comment

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

Makes sense, thank you!

@r10s r10s merged commit 3232a19 into main Nov 14, 2024
1 check passed
@r10s r10s deleted the r10s/fix-double-proxies branch November 14, 2024 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants