braintree / sanitize-url

MIT License
307 stars 35 forks source link

question: why not use the native `new URL` for this? #52

Closed tafelnl closed 1 year ago

tafelnl commented 1 year ago

Why does this library not use something like:

  try {
    const DISALLOWED_PROTOCOLS = ['javascript:', 'data:', 'vbscript:'];

    const url = new URL(urlString);

    if (!url) {
      return 'about:blank';
    }

    if (DISALLOWED_PROTOCOLS.includes(url.protocol)) {
      return 'about:blank';
    }

    return url.href;
  } catch (e) {
    // Apparently, url was invalid
    console.error(e);
  }

This is a bit simplified of course. But I guess the idea should be clear. Why rely on regex and that kind of hacky stuff, when using the native new URL seems to be a good fit for the job.

Maybe I'm just being super naive here, but this seems more logical to me.


Can I Use reports good browser support: https://caniuse.com/url

cgdibble commented 1 year ago

You're quite right, really.

The reasoning is mostly tech-debt. Right now, our SDKs use this module extensively for URL sanitization and some of these still have had some rather dated browser support. With IE being completely dropped by Microsoft we have the ability to remove affordances for old IE versions, but we just need to be able to get that work prioritized and done.

Closing this since it's something we will get to, but otherwise is an unknown timeline.