eduvpn / apple

app for iOS and macOS
Other
61 stars 18 forks source link

dealing with copy/paste URLs in app search box #407

Closed ghost closed 3 years ago

ghost commented 3 years ago

image

Is there something that can be done to make this better? For example reject this input as invalid? Or strip the http(s):// prefix and trailing /? I am not sure if there is a "perfect" solution for this. What do you think?

roop commented 3 years ago

We could look for a scheme (/^(.*):\/\//) and if a scheme is present and is not "https", reject the input (i.e. just say "No match found"). Or we could always replace the scheme with "https" and allow the input.

ghost commented 3 years ago

Ah, I see, it already handles it properly when someone copy/pastes a https://, but just not http://.

roop commented 3 years ago

Yes, it just does some simple URL fixing stuff:

https://github.com/eduvpn/apple/blob/a68499c914ea3612b7ebb1007d0fed333f159369/EduVPN/ViewModels/SearchViewModel.swift#L182-L188

Maybe we should just ask it to check for "http://" prefix as well and convert it to "https://".

jeroenleenarts commented 3 years ago

Why manual trimming and stuff?

var components = URLComponents(searchQuery)
if components.scheme == "http" {
    components.scheme = "https"
}
let result  = components.url // type is URL?
roop commented 3 years ago

@jeroenleenarts When we give a scheme-less string (like "vpn.tuxed.net") to URLComponents or URL, it considers that whole string to be a path (we want it to be seen as a host), so the code you suggested needs some more meddling with -- so it's simpler to do it with strings.

let result = components.url // type is URL?

The type is string here because we want to be able to do string matching without any URL normalization.