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

isURL validates true to 'myemail@mail.com' #1375

Closed
Arsikod opened this issue Jul 10, 2020 · 5 comments
Closed

isURL validates true to 'myemail@mail.com' #1375

Arsikod opened this issue Jul 10, 2020 · 5 comments

Comments

@Arsikod
Copy link

Arsikod commented Jul 10, 2020

console.log(validator.isURL("mymail@123.com")) returns true.
But it is not a valid url.

@Arsikod Arsikod changed the title isEmail validates true to 'myemail@mail.com' isURL validates true to 'myemail@mail.com' Jul 10, 2020
@heanzyzabala
Copy link
Contributor

Hi @profnandaa & @Arsikod,

I checked the code. It seems like we have to modify the logic in line: 96

split = url.split('@');
if (split.length > 1) {
if (options.disallow_auth) {
return false;
}
auth = split.shift();
if (auth.indexOf(':') >= 0 && auth.split(':').length > 2) {
return false;
}
}

The implementation fails to check if the url is in basic auth form without password, that looks like this:

http://user:@www.foobar.com/

For the fix, we just have to check if it's in the right form.

   if (auth.indexOf(':') === -1 || (auth.indexOf(':') >= 0 && auth.split(':').length > 2)) { 
     return false; 
   } 

This way, mymail@123.com will pass the condition and return false.

I'll submit a PR if you guys agree with the change. Thanks!

@anay-kava
Copy link

+1 - this is a serious issue.
Just checking if the fix would be included in the official release soon?

@tux-tn
Copy link
Member

tux-tn commented Aug 28, 2020

@anay-kava feel free to submit a PR concerning this issue

@heanzyzabala
Copy link
Contributor

Hi. Please see #1425.

@profnandaa
Copy link
Member

closed in #1425

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants