-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Failed to validate a valid url #1681
Comments
The colon in the username isn't RFC 1738 compliant |
This assumption based on the RFC is not really obvious to me, like said in this section, : Moreover, in Node URL API, this kind of URL (${protocol}://:${password}@${hostname}:${port}) can be parsed and allows to extract the password. |
@getlarge Can you help me on this one |
@Jassi10000 Sure, how could i help you ? |
@getlarge Thanks |
@Jassi10000 In that block, the authentication part of the URL is checked. To check if the fix is working, you can update the list of valid URLs here and try to run the tests. I hope this will help you. |
Hello @getlarge @Jassi10000, sorry I was working on this issue last week but I didn't have time to make the PR. I tried adding a new condition and avoid the With this only URLs without user and password are invalid. const user_password = split[0].split(':');
if (user_password[0] === '' && user_password[1] === '') {
return false;
} |
No worries @MiguelSavignano |
Thank you @MiguelSavignano, @Jassi10000 and @getlarge for your efforts 🙏 |
Describe the bug
isUrl
is failing to validate urls using that format${protocol}://:${password}@${hostname}:${port}
Surprisingly with version 13.5.2 i don't encounter this issue, but it seems to happen with version 13.6.0, i noticed this after digging into class-validator dependencies in package-lock.json
The failure is triggered by the second condition here. Any idea what is the purpose of that condition, what is it suppose to avoid ?
Examples
this type of url shape redis://:p4ssw0rd@server.com:6379 will always return false even though it is a valid format.
The text was updated successfully, but these errors were encountered: