improve url validation in settings

[jcalado]

It doesn't look to me like URI.tryParse is doing enough to verify if a URL is ok. It might be better to just check against a regex like https?:\/\/

[jcalado]

Everything that does not specifically mention protocol. If you only set it to host.tld it doesn't work. http://host.tld does, and so does https://host.tld

On Sat, 13 May 2023 at 01:11, austinried @.***> wrote:

@.**** requested changes on this pull request.

The problem with bypassing the Uri.tryParse() check is going to be further down the line when Uri.parse() is used, since the HttpClient relies on a valid Uri for requests.

Can you give some specific examples of URLs that don't pass here but should work?

— Reply to this email directly, view it on GitHub https://github.com/austinried/subtracks/pull/173#pullrequestreview-1425282743, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAGR6JGKWQ7TYMCZXTOITLXF3GS5ANCNFSM6AAAAAAX73I7ZY . You are receiving this because you authored the thread.Message ID: @.***>

[austinried]

Ah yeah, I see now that's actually passing the Uri.tryParse() but not working later with the HTTP client in those cases. In 1.x this is solved by adding the scheme to the URL if it's missing, I think we could do something similar here by try/catching Uri.parse() and checking the scheme, then prepending a default if it's missing.

[austinried]

Actually thinking about this more, even without automatically adding the scheme this does seem like an improvement on the existing validation so we can merge it in.

[austinried]

Looks good, thank you!