validator.js icon indicating copy to clipboard operation
validator.js copied to clipboard

isURL false positive.

Open charlesomer opened this issue 3 years ago • 5 comments

Describe the bug console.log(validator.isURL('FAQ.md', {require_valid_protocol: true})) Returns true when I believe it should be false?

Additional context Validator.js version: 13.6.0 Node.js version: v16.4.1 OS platform: macOS

charlesomer avatar Jul 06 '21 22:07 charlesomer

PR welcome.

profnandaa avatar Jul 16 '21 11:07 profnandaa

@profnandaa I am picking this issue up.. but before that I need some clarification...

  1. Why is this a bug?

    • Is it because the url is having FAQ.md where .md is perhaps invalid
    • or is it because require_valid_protocol:true was passed and the protocol is missing?
      • If this is a bug because the protocol is missing, then I believe the user must be sending require_protocol: true param.
  2. By looking at the doc, it looks like the purpose of require_valid_protocol is not to validate whether a protocol is present or not, but to validate that if a protocol is present it must be in list of valid protocols.

    • And if we are intending to change that behaviour, it might introduce a breaking a change, as our default config for isUrl is with require_valid_protocol: true which will break the existing behaviour
    • and sending require_valid_protocol will imply that the protocol must always be present.

anirudh-modi avatar Jul 17 '21 06:07 anirudh-modi

As, described by @anirudh-modi above require_valid_protocol is not to validate to verify the presence of a protocol. Which however can be achieved instead by using require_protocol option. By using both the options at once the expected behavior can be achieved:

console.log(
	validator.isURL('FAQ.md', {
		require_protocol: true,
		require_valid_protocol: true,
	})
);
// returns false

If the bug is because of some different reason, please mention more in here. I would like to work on this if that's the case. Else I think we can close this issue.

Prajwalrajbasnet avatar Oct 02 '21 18:10 Prajwalrajbasnet

I'm seeing the behavior for isURL changed recently. I have a unit test that expects the following to return false,

const urlOptions = {
  protocols: [ 'http', 'https' ],
  require_tld: false,
  require_protocol: false,
  require_host: true,
  require_valid_protocol: true,
  allow_underscores: true,
  host_whitelist: false,
  host_blacklist: false,
  allow_trailing_dot: false,
  allow_protocol_relative_urls: false,
  disallow_auth: false
}
> validator.isURL('[email protected]', urlOptions)
true

This was previously returning false

charrismatic avatar Oct 18 '21 14:10 charrismatic

I downgraded the package from 13.6.0 to 13.5.2 and it works as expected

charrismatic avatar Oct 18 '21 17:10 charrismatic