Skip to content
New issue

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

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

Already on GitHub? # to your account

isUrl function incorrectly detects Url string #23

Closed
Anqboy opened this issue Apr 30, 2021 · 2 comments
Closed

isUrl function incorrectly detects Url string #23

Anqboy opened this issue Apr 30, 2021 · 2 comments
Labels

Comments

@Anqboy
Copy link

Anqboy commented Apr 30, 2021

function isUrl(string) {
var urlRegexp = /^(https?://|ftps?://)?([a-z0-9%-]+.){1,}([a-z0-9-]+)?(:(\d{1,5}))?(/([a-z0-9-._~:/?#[]@!$&'()*+,;=%]+)?)?$/i;
return urlRegexp.test(string);
}

Since the protocol segment is set to be optional, the function will take strings like ""foo.bar"" as Url. Is this an expected behavior? Why not set the protocol segment required?

@OvervCW
Copy link

OvervCW commented May 5, 2021

I ran into a similar issue where decimal numbers inside a string were detected as URLs, e.g. "123.456". Requiring the protocol or at least the prefix www. seems like a good approach to me. Perhaps this could be supported through a separate mode that enforces a more strict definitions of URLs.

@abodelot abodelot added the bug label Jun 15, 2021
@abodelot
Copy link
Owner

abodelot commented Jun 15, 2021

Hi! That's a good point. I'll enforce the presence of http(s) protocol for url detection.
There are other issues with url detection, such as anchors and query parameters, that should be fixed as well.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants