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

URL polyfill does not throw for invalid input #53

Closed
mhassan1 opened this issue Jul 8, 2024 · 7 comments · Fixed by #59
Closed

URL polyfill does not throw for invalid input #53

mhassan1 opened this issue Jul 8, 2024 · 7 comments · Fixed by #59

Comments

@mhassan1
Copy link
Collaborator

mhassan1 commented Jul 8, 2024

The existing URL polyfill does not throw for invalid input when it should; for example, all of these should throw:

new URL()
new URL('')
new URL('/hello')

This issue was raised previously (https://github.com/JakeChampion/polyfill-library/issues/462), but it was never resolved, and there was a PR (https://github.com/JakeChampion/polyfill-library/pull/627) that was closed without comment.

There seem to be a bunch of existing URL polyfills out there, so maybe we should switch to a different one.

@mhassan1
Copy link
Collaborator Author

mhassan1 commented Jul 8, 2024

I've looked around at a few other URL polyfills:

@romainmenke
Copy link
Member

If we switch to a different polyfill, then my preference would be to use https://www.npmjs.com/package/whatwg-url

The contributors there are also browser implementers, I trust them more to get this right.

Can we handle transpiling?

@zloirock
Copy link

zloirock commented Jul 9, 2024

npmjs.com/package/core-js-url-browser - lots of tests fail

Could you write specific issues with it?

@mhassan1
Copy link
Collaborator Author

mhassan1 commented Jul 9, 2024

@zloirock Here's an example of a failing test:

[...new URLSearchParams('?%C2').keys()][0]
// core-js -> "%C2"
// native -> "�"

@romainmenke
Copy link
Member

Is that with core-js itself or with the npmjs.com/package/core-js-url-browser npm package? (that package seems archived/deprecated?)

@zloirock
Copy link

zloirock commented Jul 9, 2024

@mhassan1 yes, I know that core-js has some specific issues with URL, mainly encoding specific, like zloirock/core-js#1223, and the feature detection relaxed to forgive some engines bugs. I mean the list of failing tests that could help to fix it.

@mhassan1
Copy link
Collaborator Author

mhassan1 commented Jul 9, 2024

@romainmenke That's with core-js; for example, in Node.js:

delete global.URLSearchParams;
var URLSearchParams = require('core-js/web/url-search-params');
[...new URLSearchParams('?%C2').keys()][0];
// -> "%C2"

@zloirock Here's the list of failing tests:

  • parses and sorts: %FE%FF
  • parses and sorts: %FF%FE
  • parses and sorts: %C2
  • parses and sorts: %C2x
  • parses and sorts: _charset_=windows-1252&test=%C2x

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

Successfully merging a pull request may close this issue.

3 participants