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

Only allow HTTP URLs by default #11

Open
callumlocke opened this issue Nov 12, 2019 · 1 comment
Open

Only allow HTTP URLs by default #11

callumlocke opened this issue Nov 12, 2019 · 1 comment

Comments

@callumlocke
Copy link

isAbsoluteUrl('javascript:throw%20document.cookie') // true

Although this might be technically correct (?), it seems like it's not what most people would think of as an absolute URL, and it could lead to a false sense of security that the input has been validated as a 'real' URL.

Possible solutions:

  • Release a new major version that ensures the protocol is http or https
    • Maybe with an option like { allowNonHttp: true } for people who want the old behaviour.
  • Or just add a warning in the readme that this will return true for any protocol, even javascript:.
@sindresorhus
Copy link
Owner

sindresorhus commented Nov 12, 2019

This package is really not meant as a security measure and javascript: is technically an absolute URL. However, I also guess the main use-case is to check HTTP URLs, so I'm willing to change the default behavior. How about an option called httpOnly which defaults to true?

Or just add a warning in the readme that this will return true for any protocol, even javascript:.

We can do this too for in case the user set httpOnly to false.


If anyone wants to work on this, see the feedback in #14 and #12.

@sindresorhus sindresorhus changed the title XSS risk: returns true for "javascript:" protocol Only allow HTTP URLs by default Nov 12, 2019
acagastya added a commit to acagastya/is-absolute-url that referenced this issue Oct 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants