url: implement parse method for safer URL parsing #52280

thisalihassan · 2024-03-30T19:38:24Z

Implement the static parse method as per the WHATWG URL specification. Unlike the URL constructor, URL.parse does not throw on invalid input, instead returning null. This behavior allows safer parsing of URLs without the need for try-catch blocks around constructor calls. The implementation follows the steps outlined in the WHATWG URL standard, ensuring compatibility and consistency with web platform URL parsing APIs.

Fixes: #52208
Refs: whatwg/url#825

Implement the static parse method as per the WHATWG URL specification. Unlike the URL constructor, URL.parse does not throw on invalid input, instead returning null. This behavior allows safer parsing of URLs without the need for try-catch blocks around constructor calls. The implementation follows the steps outlined in the WHATWG URL standard, ensuring compatibility and consistency with web platform URL parsing APIs. Fixes: nodejs#52208 Refs: whatwg/url#825

nodejs-github-bot · 2024-03-30T19:38:29Z

Review requested:

@nodejs/url
@nodejs/web-standards

mertcanaltin · 2024-03-30T19:42:44Z

Can you add a test 🚀

panva · 2024-03-30T19:44:16Z

Can you add a test 🚀

And run git node wpt url to update the WPT test suite.

lib/internal/url.js

thisalihassan · 2024-03-30T19:45:34Z

Can you add a test 🚀

And run git node wpt url to update the WPT test suite.

thanks, sure

lib/internal/url.js

mcollina

lgtm

thisalihassan · 2024-04-02T20:56:56Z

@nodejs/url @nodejs/web-standards any feedback on this?

lemire

I encourage the author of this contribution to consider providing a benchmark. It would be interesting to compare this method with the more conventional one and make sure that, at least, it is no slower. Such a benchmark could be provided as part of a follow-up contribution.

src/node_url.cc

lib/internal/url.js

thisalihassan · 2024-04-05T07:50:22Z

I will be doing a benchmark to compare both approaches afterwards, this is facinating to me which one is faster/slower. but can we request CI on this? @nodejs/url @nodejs/web-standards

nodejs-github-bot · 2024-04-05T08:17:00Z

CI: https://ci.nodejs.org/job/node-test-pull-request/58133/

thisalihassan · 2024-04-05T17:06:43Z

on node-test-commit-osx tests are failing due to no space left on device

@nodejs/url @nodejs/web-standards

nodejs-github-bot · 2024-04-08T09:41:44Z

CI: https://ci.nodejs.org/job/node-test-pull-request/58207/

nodejs-github-bot · 2024-04-12T03:22:19Z

CI: https://ci.nodejs.org/job/node-test-pull-request/58300/

nodejs-github-bot · 2024-04-13T11:06:14Z

CI: https://ci.nodejs.org/job/node-test-pull-request/58343/

nodejs-github-bot · 2024-04-13T12:49:00Z

Commit Queue failed

- Loading data for nodejs/node/pull/52280
✔  Done loading data for nodejs/node/pull/52280
----------------------------------- PR info ------------------------------------
Title      url: implement parse method for safer URL parsing (#52280)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     thisalihassan:url-parse-implement -> nodejs:main
Labels     semver-minor, whatwg-url, needs-ci, commit-queue-squash
Commits    4
 - url: implement parse method for safer URL parsing
 - url: change approach for parse method
 - url: fix lint
 - url: resolve feedback
Committers 1
 - Ali Hassan 
PR-URL: https://github.com/nodejs/node/pull/52280
Fixes: https://github.com/nodejs/node/issues/52208
Refs: https://github.com/whatwg/url/pull/825
Reviewed-By: Yagiz Nizipli 
Reviewed-By: Matteo Collina 
Reviewed-By: Daniel Lemire 
Reviewed-By: Benjamin Gruenbaum 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/52280
Fixes: https://github.com/nodejs/node/issues/52208
Refs: https://github.com/whatwg/url/pull/825
Reviewed-By: Yagiz Nizipli 
Reviewed-By: Matteo Collina 
Reviewed-By: Daniel Lemire 
Reviewed-By: Benjamin Gruenbaum 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last approving review:
   ⚠  - url: resolve feedback
   ℹ  This PR was created on Sat, 30 Mar 2024 19:38:24 GMT
   ✔  Approvals: 4
   ✔  - Yagiz Nizipli (@anonrig) (TSC): https://github.com/nodejs/node/pull/52280#pullrequestreview-1992855443
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/52280#pullrequestreview-1970237648
   ✔  - Daniel Lemire (@lemire): https://github.com/nodejs/node/pull/52280#pullrequestreview-1975050150
   ✔  - Benjamin Gruenbaum (@benjamingr) (TSC): https://github.com/nodejs/node/pull/52280#pullrequestreview-1993587775
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2024-04-13T11:06:14Z: https://ci.nodejs.org/job/node-test-pull-request/58343/
- Querying data for job/node-test-pull-request/58343/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/8673630995

nodejs-github-bot · 2024-04-13T12:57:13Z

Landed in 8b4c4e8

nodejs/node#52280

Implement the static parse method as per the WHATWG URL specification. Unlike the URL constructor, URL.parse does not throw on invalid input, instead returning null. This behavior allows safer parsing of URLs without the need for try-catch blocks around constructor calls. The implementation follows the steps outlined in the WHATWG URL standard, ensuring compatibility and consistency with web platform URL parsing APIs. Fixes: #52208 Refs: whatwg/url#825 PR-URL: #52280 Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Daniel Lemire <daniel@lemire.me> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

Notable changes: buffer: * improve `base64` and `base64url` performance (Yagiz Nizipli) #52428 dns: * (SEMVER-MINOR) add order option and support ipv6first (Paolo Insogna) #52492 events,doc: * mark CustomEvent as stable (Daeyeon Jeong) #52618 lib, url: * (SEMVER-MINOR) add a `windows` option to path parsing (Aviv Keller) #52509 module: * (SEMVER-MINOR) implement NODE_COMPILE_CACHE for automatic on-disk code caching (Joyee Cheung) #52535 net: * (SEMVER-MINOR) add CLI option for autoSelectFamilyAttemptTimeout (Paolo Insogna) #52474 src: * (SEMVER-MINOR) add `string_view` overload to snapshot FromBlob (Anna Henningsen) #52595 src,permission: * throw async errors on async APIs (Rafael Gonzaga) #52730 test_runner: * (SEMVER-MINOR) add --test-skip-pattern cli option (Aviv Keller) #52529 url: * (SEMVER-MINOR) implement parse method for safer URL parsing (Ali Hassan) #52280 PR-URL: TODO

Notable changes: buffer: * improve `base64` and `base64url` performance (Yagiz Nizipli) #52428 dns: * (SEMVER-MINOR) add order option and support ipv6first (Paolo Insogna) #52492 events,doc: * mark CustomEvent as stable (Daeyeon Jeong) #52618 lib, url: * (SEMVER-MINOR) add a `windows` option to path parsing (Aviv Keller) #52509 module: * (SEMVER-MINOR) implement NODE_COMPILE_CACHE for automatic on-disk code caching (Joyee Cheung) #52535 net: * (SEMVER-MINOR) add CLI option for autoSelectFamilyAttemptTimeout (Paolo Insogna) #52474 src: * (SEMVER-MINOR) add `string_view` overload to snapshot FromBlob (Anna Henningsen) #52595 src,permission: * throw async errors on async APIs (Rafael Gonzaga) #52730 test_runner: * (SEMVER-MINOR) add --test-skip-pattern cli option (Aviv Keller) #52529 url: * (SEMVER-MINOR) implement parse method for safer URL parsing (Ali Hassan) #52280 PR-URL: #52768

Notable changes: buffer: * improve `base64` and `base64url` performance (Yagiz Nizipli) nodejs#52428 dns: * (SEMVER-MINOR) add order option and support ipv6first (Paolo Insogna) nodejs#52492 events,doc: * mark CustomEvent as stable (Daeyeon Jeong) nodejs#52618 lib, url: * (SEMVER-MINOR) add a `windows` option to path parsing (Aviv Keller) nodejs#52509 module: * (SEMVER-MINOR) implement NODE_COMPILE_CACHE for automatic on-disk code caching (Joyee Cheung) nodejs#52535 net: * (SEMVER-MINOR) add CLI option for autoSelectFamilyAttemptTimeout (Paolo Insogna) nodejs#52474 src: * (SEMVER-MINOR) add `string_view` overload to snapshot FromBlob (Anna Henningsen) nodejs#52595 src,permission: * throw async errors on async APIs (Rafael Gonzaga) nodejs#52730 test_runner: * (SEMVER-MINOR) add --test-skip-pattern cli option (Aviv Keller) nodejs#52529 url: * (SEMVER-MINOR) implement parse method for safer URL parsing (Ali Hassan) nodejs#52280 PR-URL: nodejs#52768

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [core-js](https://github.com/zloirock/core-js) | dependencies | minor | [`3.36.1` -> `3.37.1`](https://renovatebot.com/diffs/npm/core-js/3.36.1/3.37.1) | --- ### Release Notes <details> <summary>zloirock/core-js (core-js)</summary> ### [`v3.37.1`](https://github.com/zloirock/core-js/blob/HEAD/CHANGELOG.md#3371---20240514) [Compare Source](zloirock/core-js@v3.37.0...v3.37.1) - Changes [v3.37.0...v3.37.1](zloirock/core-js@v3.37.0...v3.37.1) - Fixed [`URL.parse`](https://url.spec.whatwg.org/#dom-url-parse) feature detection for some specific cases - Compat data improvements: - [`Set` methods proposal](https://github.com/tc39/proposal-set-methods) added and marked as [supported from FF 127](https://bugzilla.mozilla.org/show_bug.cgi?id=1868423) - [`Symbol.dispose`](https://github.com/tc39/proposal-explicit-resource-management) added and marked as supported from V8 ~ Chromium 125 - [`Math.f16round` and `DataView.prototype.{ getFloat16, setFloat16 }`](https://github.com/tc39/proposal-float16array) added and marked as [supported from Deno 1.43](denoland/deno#23490) - [`URL.parse`](https://url.spec.whatwg.org/#dom-url-parse) added and marked as [supported from Chromium 126](https://chromestatus.com/feature/6301071388704768) - [`URL.parse`](https://url.spec.whatwg.org/#dom-url-parse) added and marked as [supported from NodeJS 22.0](nodejs/node#52280) - [`URL.parse`](https://url.spec.whatwg.org/#dom-url-parse) added and marked as [supported from Deno 1.43](denoland/deno#23318) - Added [Rhino 1.7.15](https://github.com/mozilla/rhino/releases/tag/Rhino1\_7\_15\_Release) compat data, many features marked as supported - Added [NodeJS 22.0](https://nodejs.org/en/blog/release/v22.0.0) compat data mapping - Added [Deno 1.43](https://github.com/denoland/deno/releases/tag/v1.43.0) compat data mapping - Added Electron 31 compat data mapping - Updated [Opera Android 82](https://forums.opera.com/topic/71513/opera-for-android-82) compat data mapping - Added Samsung Internet 26 compat data mapping - Added Oculus Quest Browser 33 compat data mapping ### [`v3.37.0`](https://github.com/zloirock/core-js/blob/HEAD/CHANGELOG.md#3370---20240417) [Compare Source](zloirock/core-js@v3.36.1...v3.37.0) - Changes [v3.36.1...v3.37.0](zloirock/core-js@v3.36.1...v3.37.0) - [New `Set` methods proposal](https://github.com/tc39/proposal-set-methods): - Built-ins: - `Set.prototype.intersection` - `Set.prototype.union` - `Set.prototype.difference` - `Set.prototype.symmetricDifference` - `Set.prototype.isSubsetOf` - `Set.prototype.isSupersetOf` - `Set.prototype.isDisjointFrom` - Moved to stable ES, [April 2024 TC39 meeting](tc39/proposals@bda5a6b) - Added `es.` namespace modules, `/es/` and `/stable/` namespaces entries - [Explicit Resource Management stage 3 proposal](https://github.com/tc39/proposal-explicit-resource-management): - Some minor updates like [explicit-resource-management/217](tc39/proposal-explicit-resource-management#217) - Added [`Math.sumPrecise` stage 2.7 proposal](https://github.com/tc39/proposal-math-sum/): - Built-ins: - `Math.sumPrecise` - [`Promise.try` proposal](https://github.com/tc39/proposal-promise-try): - Built-ins: - `Promise.try` - Added optional arguments support, [promise-try/16](tc39/proposal-promise-try#16) - Moved to stage 2.7, [April 2024 TC39 meeting](tc39/proposals@301fc9c) - [`RegExp.escape` stage 2 proposal](https://github.com/tc39/proposal-regex-escaping): - Moved to hex-escape semantics, [regex-escaping/67](tc39/proposal-regex-escaping#67) - It's not the final change of the way of escaping, waiting for [regex-escaping/77](tc39/proposal-regex-escaping#77) soon - [Pattern matching stage 1 proposal](https://github.com/tc39/proposal-pattern-matching): - Built-ins: - `Symbol.customMatcher` - Once again, [the used well-known symbol was renamed](tc39/proposal-pattern-matching#295) - Added new entries for that - Added [Extractors stage 1 proposal](https://github.com/tc39/proposal-extractors): - Built-ins: - `Symbol.customMatcher` - Since the `Symbol.customMatcher` well-known symbol from the pattern matching proposal is also used in the exactors proposal, added an entry also for this proposal - Added [`URL.parse`](https://url.spec.whatwg.org/#dom-url-parse), [url/825](whatwg/url#825) - Engines bugs fixes: - Added a fix of [Safari `{ Object, Map }.groupBy` bug that does not support iterable primitives](https://bugs.webkit.org/show_bug.cgi?id=271524) - Added a fix of [Safari bug with double call of constructor in `Array.fromAsync`](https://bugs.webkit.org/show_bug.cgi?id=271703) - Compat data improvements: - [`URL.parse`](https://url.spec.whatwg.org/#dom-url-parse) added and marked as supported [from FF 126](https://bugzilla.mozilla.org/show_bug.cgi?id=1887611) - [`URL.parse`](https://url.spec.whatwg.org/#dom-url-parse) added and marked as supported [from Bun 1.1.4](oven-sh/bun#10129) - [`URL.canParse`](https://url.spec.whatwg.org/#dom-url-canparse) fixed and marked as supported [from Bun 1.1.0](oven-sh/bun#9710) - [New `Set` methods](https://github.com/tc39/proposal-set-methods) fixed in JavaScriptCore and marked as supported from Bun 1.1.1 - Added Opera Android 82 compat data mapping </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Reviewed-on: https://git.vylpes.xyz/RabbitLabs/Droplet/pulls/306 Co-authored-by: Renovate Bot <renovate@vylpes.com> Co-committed-by: Renovate Bot <renovate@vylpes.com>

nodejs-github-bot added needs-ci PRs that need a full CI run. whatwg-url Issues and PRs related to the WHATWG URL implementation. labels Mar 30, 2024

thisalihassan marked this pull request as draft March 30, 2024 19:38

lpinca reviewed Mar 30, 2024

View reviewed changes

lib/internal/url.js Outdated Show resolved Hide resolved

anonrig requested changes Mar 30, 2024

View reviewed changes

lib/internal/url.js Show resolved Hide resolved

url: change approach for parse method

b28fc6e

thisalihassan requested review from anonrig, Jamesernator and joyeecheung March 31, 2024 11:29

thisalihassan marked this pull request as ready for review March 31, 2024 11:29

mcollina approved these changes Mar 31, 2024

View reviewed changes

url: fix lint

48e3b9a

thisalihassan requested review from lemire, mcollina and lpinca March 31, 2024 14:02

lemire approved these changes Apr 2, 2024

View reviewed changes

anonrig reviewed Apr 2, 2024

View reviewed changes

src/node_url.cc Outdated Show resolved Hide resolved

src/node_url.cc Outdated Show resolved Hide resolved

lib/internal/url.js Outdated Show resolved Hide resolved

thisalihassan requested a review from anonrig April 4, 2024 15:12

anonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 10, 2024

anonrig approved these changes Apr 10, 2024

View reviewed changes

url: resolve feedback

c304252

thisalihassan force-pushed the url-parse-implement branch from 9af26a6 to c304252 Compare April 11, 2024 21:02

anonrig added request-ci Add this label to start a Jenkins CI on a PR. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Apr 12, 2024

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 12, 2024

anonrig added the semver-minor PRs that contain new features and should be released in the next minor version. label Apr 12, 2024

anonrig added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 13, 2024

nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Apr 13, 2024

anonrig approved these changes Apr 13, 2024

View reviewed changes

anonrig added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Apr 13, 2024

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 13, 2024

nodejs-github-bot merged commit 8b4c4e8 into nodejs:main Apr 13, 2024
69 checks passed

zloirock added a commit to zloirock/core-js that referenced this pull request Apr 22, 2024

URL.parse added and marked as supported from NodeJS 22.0

518bbae

nodejs/node#52280

aduh95 mentioned this pull request Apr 30, 2024

2024-05-02, Version 22.1.0 (Current) #52768

Merged

marco-ippolito added the backport-blocked-v20.x PRs that should land on the v20.x-staging branch but are blocked by another PR's pending backport. label May 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

url: implement parse method for safer URL parsing #52280

url: implement parse method for safer URL parsing #52280

thisalihassan commented Mar 30, 2024

nodejs-github-bot commented Mar 30, 2024

mertcanaltin commented Mar 30, 2024

panva commented Mar 30, 2024

thisalihassan commented Mar 30, 2024

mcollina left a comment

thisalihassan commented Apr 2, 2024

lemire left a comment

thisalihassan commented Apr 5, 2024

nodejs-github-bot commented Apr 5, 2024

thisalihassan commented Apr 5, 2024

nodejs-github-bot commented Apr 8, 2024

nodejs-github-bot commented Apr 12, 2024

nodejs-github-bot commented Apr 13, 2024

nodejs-github-bot commented Apr 13, 2024

nodejs-github-bot commented Apr 13, 2024

url: implement parse method for safer URL parsing #52280

url: implement parse method for safer URL parsing #52280

Conversation

thisalihassan commented Mar 30, 2024

nodejs-github-bot commented Mar 30, 2024

mertcanaltin commented Mar 30, 2024

panva commented Mar 30, 2024

thisalihassan commented Mar 30, 2024

mcollina left a comment

Choose a reason for hiding this comment

thisalihassan commented Apr 2, 2024

lemire left a comment

Choose a reason for hiding this comment

thisalihassan commented Apr 5, 2024

nodejs-github-bot commented Apr 5, 2024

thisalihassan commented Apr 5, 2024

nodejs-github-bot commented Apr 8, 2024

nodejs-github-bot commented Apr 12, 2024

nodejs-github-bot commented Apr 13, 2024

nodejs-github-bot commented Apr 13, 2024

nodejs-github-bot commented Apr 13, 2024