doc: deprecate url.parse() #44919

Trott · 2022-10-08T04:00:50Z

This is a documentation-deprecation only and it is possible that it will not proceed to a runtime-deprecation any time in the foreseeable future. But url.parse() is not standardized and prone to errors that have security implications.

Refs: #44911 (comment)

Trott · 2022-10-08T04:00:58Z

@nodejs/tsc

mcollina

lgtm

lpinca · 2022-10-08T06:09:58Z

I fear that this will create the same issue described here #44911 (comment). Users will read "deprecated" and open PRs to replace url.parse() with new URL() even though the usage of url.parse() is perfectly safe in that particular context. Anyway I don't know how to solve that.

mcollina · 2022-10-08T06:30:25Z

doc/api/url.md

-
-Use with caution.
+and incorrect handling of usernames and passwords. Use the [WHATWG URL][] API
+instead.


I would recommend adding something like:

Security disclaimer: if you use this API, treat your URLs as trusted or validate them before usage. Security reports that shows exploits about url.parse() will not be issued new CVEs.

Definitely+1 on adding the disclaimer

I like the stronger warning and will incorporate that. I'm thinking about the "no new CVEs will be issued" part. That discourages people from reporting potentially legitimate issues with url.parse() that we may not be aware of. However, I also understand not wanting to deal with an endless stream of all-very-similar-or-the-same url.parse() reports for things we probably won't even fix. So I think I can be persuaded. We have 1/3 of @nodejs/security-triage weighing in above. Curious what the rest of those folks think.

The problem is that currently url.parse() is unsafe to use with untrusted input. If we want to get new CVE issued for it, we should make sure it's safe to use. However, doing so would break all the legitimate users that treat its input as trusted.

I don't see any other option than deprecation.

jasnell · 2022-10-08T07:15:36Z

+0 ... Although I prefer this, we've done this before and enough people complained that we switched it to legacy status. Not sure there's much benefit to a doc deprecation at this point.

mcollina · 2022-10-08T09:30:55Z

I fear that this will create the same issue described here #44911 (comment). Users will read "deprecated" and open PRs to replace url.parse() with new URL() even though the usage of url.parse() is perfectly safe in that particular context. Anyway I don't know how to solve that.

This API is essentially unsafe to use and exposes folks to security risks. This is similar to new Buffer(). At this stage, I would even consider a runtime deprecation.

Note that legacy does not stop people opening PRs mentioning it's a deprecated API that should not be used.

Trott · 2022-10-08T15:37:07Z

@nodejs/security-triage

mcollina · 2022-10-08T18:36:12Z

cc @nodejs/tsc for visibility

RafaelGSS

LGTM.

juanarbol

LGTM

aduh95

If we're fixing Legacy -> legacy in this PR, it should probably also be fixed in the text of DEP0116. BTW it might make sense to add an entry in the history of DEP0116 to clarify that url.parse() is deprecated again.

This is a documentation-deprecation only and it is possible that it will not proceed to a runtime-deprecation any time in the foreseeable future. But url.parse() is not standardized and prone to errors that have security implications. Refs: nodejs#44911 (comment)

Trott · 2022-10-11T18:21:16Z

I've added "no CVEs for url.parse()" language in a fixup commit. @mcollina and everyone else, PTAL.

doc/api/deprecations.md

aduh95

LGTM expept the off by one errors in the pr-url

mhdawson

LGTM

Co-authored-by: James M Snell <[email protected]>

Notable Changes: doc: * graduate webcrypto to stable (Filip Skokan) #44897 esm: * remove specifier resolution flag (Geoffrey Booth) #44859 http: * (SEMVER-MAJOR) use Keep-Alive by default in global agents (Paolo Insogna) #43522 build: * (SEMVER-MAJOR) remove dtrace & etw support (Ben Noordhuis) #43652 * (SEMVER-MAJOR) remove systemtap support (Ben Noordhuis) #43651 Deprecation and Removals: * deprecate url.parse() (Rich Trott) #44919 * (SEMVER-MAJOR) runtime deprecate exports double slash maps (Guy Bedford) #44495 * (SEMVER-MAJOR) runtime deprecate coercion to integer in `process.exit()` (Daeyeon Jeong) #44711 Semver-Major Commits: * [53f73d1] - (SEMVER-MAJOR) build: enable V8's trap handler on Windows (Michaël Zasso) #44741 * [06aaf8a] - (SEMVER-MAJOR) build: reset embedder string to "-node.0" (Michaël Zasso) #44741 * [aa3a572] - (SEMVER-MAJOR) build: remove dtrace & etw support (Ben Noordhuis) #43652 * [38f1e27] - (SEMVER-MAJOR) build: remove systemtap support (Ben Noordhuis) #43651 * [2849283] - (SEMVER-MAJOR) crypto: remove non-standard `webcrypto.Crypto.prototype.CryptoKey` (Antoine du Hamel) #42083 * [a1653ac] - (SEMVER-MAJOR) crypto: do not allow to call setFips from the worker thread (Sergey Petushkov) #43624 * [fd36a8d] - (SEMVER-MAJOR) deps: update llhttp to 8.1.0 (Paolo Insogna) #44967 * [89ecddd] - (SEMVER-MAJOR) deps: bump minimum ICU version to 71 (Michaël Zasso) #44741 * [66fe446] - (SEMVER-MAJOR) deps: V8: cherry-pick 0cccb6f27d78 (Michaël Zasso) #44741 * [88ed027] - (SEMVER-MAJOR) deps: V8: cherry-pick 7ddb8399f9f1 (Michaël Zasso) #44741 * [26c651c] - (SEMVER-MAJOR) deps: V8: cherry-pick 1b3a4f0c34a1 (Michaël Zasso) #44741 * [c8ff2df] - (SEMVER-MAJOR) deps: V8: cherry-pick b161a0823165 (Michaël Zasso) #44741 * [7a8fa2d] - (SEMVER-MAJOR) deps: fix V8 build on Windows with MSVC (Michaël Zasso) #44741 * [83b0aaa] - (SEMVER-MAJOR) deps: fix V8 build on SmartOS (Michaël Zasso) #44741 * [7a952e8] - (SEMVER-MAJOR) deps: silence irrelevant V8 warning (Michaël Zasso) #44741 * [6bd756d] - (SEMVER-MAJOR) deps: update V8 to 10.7.193.13 (Michaël Zasso) #44741 * [03fb789] - (SEMVER-MAJOR) events: add null check for the signal of EventTarget (Masashi Hirano) #43153 * [a4fa526] - (SEMVER-MAJOR) fs: add directory autodetection to fsPromises.symlink() (Livia Medeiros) #42894 * [bb4891d] - (SEMVER-MAJOR) fs: add validateBuffer to improve error (Hirotaka Tagawa / wafuwafu13) #44769 * [950a441] - (SEMVER-MAJOR) fs: remove coercion to string in writing methods (Livia Medeiros) #42796 * [41a6d82] - (SEMVER-MAJOR) fs: harden fs.readSync(buffer, options) typecheck (LiviaMedeiros) #42772 * [2275faa] - (SEMVER-MAJOR) fs: harden fs.read(params, callback) typecheck (LiviaMedeiros) #42772 * [29953a0] - (SEMVER-MAJOR) fs: harden filehandle.read(params) typecheck (LiviaMedeiros) #42772 * [4267b92] - (SEMVER-MAJOR) http: use Keep-Alive by default in global agents (Paolo Insogna) #43522 * [0324529] - (SEMVER-MAJOR) inspector: introduce inspector/promises API (Erick Wendel) #44250 * [8027099] - (SEMVER-MAJOR) lib: enable global CustomEvent by default (Daeyeon Jeong) #44860 * [f529f73] - (SEMVER-MAJOR) lib: brand check event handler property receivers (Chengzhong Wu) #44483 * [6de2673] - (SEMVER-MAJOR) lib: enable global WebCrypto by default (Antoine du Hamel) #42083 * [73ba883] - (SEMVER-MAJOR) lib: use private field in AbortController (Joyee Cheung) #43820 * [7dd2f41] - (SEMVER-MAJOR) module: runtime deprecate exports double slash maps (Guy Bedford) #44495 * [22c39b1] - (SEMVER-MAJOR) path: the dot will be added(path.format) if it is not specified in `ext` (theanarkh) #44349 * [587367d] - (SEMVER-MAJOR) perf_hooks: expose webperf global scope interfaces (Chengzhong Wu) #44483 * [364c0e1] - (SEMVER-MAJOR) perf_hooks: fix webperf idlharness (Chengzhong Wu) #44483 * [ada2d05] - (SEMVER-MAJOR) process: runtime deprecate coercion to integer in `process.exit()` (Daeyeon Jeong) #44711 * [e0ab8dd] - (SEMVER-MAJOR) process: make process.config read only (Sergey Petushkov) #43627 * [481a959] - (SEMVER-MAJOR) readline: remove `question` method from `InterfaceConstructor` (Antoine du Hamel) #44606 * [c9602ce] - (SEMVER-MAJOR) src: use new v8::OOMErrorCallback API (Michaël Zasso) #44741 * [19a70c1] - (SEMVER-MAJOR) src: override CreateJob instead of PostJob (Clemens Backes) #44741 * [fd52c62] - (SEMVER-MAJOR) src: use V8_ENABLE_SANDBOX macro (Michaël Zasso) #44741 * [c10988d] - (SEMVER-MAJOR) src: use non-deprecated V8 inspector API (Michaël Zasso) #44741 * [3efe901] - (SEMVER-MAJOR) src: update NODE_MODULE_VERSION to 111 (Michaël Zasso) #44741 * [77e5856] - (SEMVER-MAJOR) src: turn embedder api overload into default argument (Alena Khineika) #43629 * [dabda03] - (SEMVER-MAJOR) src: per-environment time origin value (Chengzhong Wu) #43781 * [2e49b99] - (SEMVER-MAJOR) src,test: disable freezing V8 flags on initialization (Clemens Backes) #44741 * [2b32985] - (SEMVER-MAJOR) stream: use null for the error argument (Luigi Pinca) #44312 * [36805e8] - (SEMVER-MAJOR) test: adapt test-repl for V8 update (Michaël Zasso) #44741 * [96ef257] - (SEMVER-MAJOR) test: adapt test-repl-pretty-*stack to V8 changes (Michaël Zasso) #44741 * [71c193e] - (SEMVER-MAJOR) test: adapt to new JSON SyntaxError messages (Michaël Zasso) #44741 * [b5f1564] - (SEMVER-MAJOR) test: rename always-opt flag to always-turbofan (Michaël Zasso) #44741 * [1acf033] - (SEMVER-MAJOR) test: fix test-hash-seed for new V8 versions (Michaël Zasso) #44741 * [57ff476] - (SEMVER-MAJOR) test: remove duplicate test (Luigi Pinca) #44051 * [77def91] - (SEMVER-MAJOR) tls,http2: send fatal alert on ALPN mismatch (Tobias Nießen) #44031 * [4860ad9] - (SEMVER-MAJOR) tools: update V8 gypfiles for 10.7 (Michaël Zasso) #44741 PR-URL: #44626 Co-authored-by: Ruy Adorno <[email protected]>

Notable Changes: doc: * graduate webcrypto to stable (Filip Skokan) #44897 esm: * remove specifier resolution flag (Geoffrey Booth) #44859 http: * (SEMVER-MAJOR) use Keep-Alive by default in global agents (Paolo Insogna) #43522 build: * (SEMVER-MAJOR) remove dtrace & etw support (Ben Noordhuis) #43652 * (SEMVER-MAJOR) remove systemtap support (Ben Noordhuis) #43651 deps: * (SEMVER-MAJOR) deps: update V8 to 10.7.193.13 (Michaël Zasso) #44741 Deprecation and Removals: * deprecate url.parse() (Rich Trott) #44919 * (SEMVER-MAJOR) runtime deprecate exports double slash maps (Guy Bedford) #44495 * (SEMVER-MAJOR) runtime deprecate coercion to integer in `process.exit()` (Daeyeon Jeong) #44711 Semver-Major Commits: * [53f73d1] - (SEMVER-MAJOR) build: enable V8's trap handler on Windows (Michaël Zasso) #44741 * [06aaf8a] - (SEMVER-MAJOR) build: reset embedder string to "-node.0" (Michaël Zasso) #44741 * [aa3a572] - (SEMVER-MAJOR) build: remove dtrace & etw support (Ben Noordhuis) #43652 * [38f1e27] - (SEMVER-MAJOR) build: remove systemtap support (Ben Noordhuis) #43651 * [2849283] - (SEMVER-MAJOR) crypto: remove non-standard `webcrypto.Crypto.prototype.CryptoKey` (Antoine du Hamel) #42083 * [a1653ac] - (SEMVER-MAJOR) crypto: do not allow to call setFips from the worker thread (Sergey Petushkov) #43624 * [fd36a8d] - (SEMVER-MAJOR) deps: update llhttp to 8.1.0 (Paolo Insogna) #44967 * [89ecddd] - (SEMVER-MAJOR) deps: bump minimum ICU version to 71 (Michaël Zasso) #44741 * [66fe446] - (SEMVER-MAJOR) deps: V8: cherry-pick 0cccb6f27d78 (Michaël Zasso) #44741 * [88ed027] - (SEMVER-MAJOR) deps: V8: cherry-pick 7ddb8399f9f1 (Michaël Zasso) #44741 * [26c651c] - (SEMVER-MAJOR) deps: V8: cherry-pick 1b3a4f0c34a1 (Michaël Zasso) #44741 * [c8ff2df] - (SEMVER-MAJOR) deps: V8: cherry-pick b161a0823165 (Michaël Zasso) #44741 * [7a8fa2d] - (SEMVER-MAJOR) deps: fix V8 build on Windows with MSVC (Michaël Zasso) #44741 * [83b0aaa] - (SEMVER-MAJOR) deps: fix V8 build on SmartOS (Michaël Zasso) #44741 * [7a952e8] - (SEMVER-MAJOR) deps: silence irrelevant V8 warning (Michaël Zasso) #44741 * [6bd756d] - (SEMVER-MAJOR) deps: update V8 to 10.7.193.13 (Michaël Zasso) #44741 * [03fb789] - (SEMVER-MAJOR) events: add null check for the signal of EventTarget (Masashi Hirano) #43153 * [a4fa526] - (SEMVER-MAJOR) fs: add directory autodetection to fsPromises.symlink() (Livia Medeiros) #42894 * [bb4891d] - (SEMVER-MAJOR) fs: add validateBuffer to improve error (Hirotaka Tagawa / wafuwafu13) #44769 * [950a441] - (SEMVER-MAJOR) fs: remove coercion to string in writing methods (Livia Medeiros) #42796 * [41a6d82] - (SEMVER-MAJOR) fs: harden fs.readSync(buffer, options) typecheck (LiviaMedeiros) #42772 * [2275faa] - (SEMVER-MAJOR) fs: harden fs.read(params, callback) typecheck (LiviaMedeiros) #42772 * [29953a0] - (SEMVER-MAJOR) fs: harden filehandle.read(params) typecheck (LiviaMedeiros) #42772 * [4267b92] - (SEMVER-MAJOR) http: use Keep-Alive by default in global agents (Paolo Insogna) #43522 * [0324529] - (SEMVER-MAJOR) inspector: introduce inspector/promises API (Erick Wendel) #44250 * [8027099] - (SEMVER-MAJOR) lib: enable global CustomEvent by default (Daeyeon Jeong) #44860 * [f529f73] - (SEMVER-MAJOR) lib: brand check event handler property receivers (Chengzhong Wu) #44483 * [6de2673] - (SEMVER-MAJOR) lib: enable global WebCrypto by default (Antoine du Hamel) #42083 * [73ba883] - (SEMVER-MAJOR) lib: use private field in AbortController (Joyee Cheung) #43820 * [7dd2f41] - (SEMVER-MAJOR) module: runtime deprecate exports double slash maps (Guy Bedford) #44495 * [22c39b1] - (SEMVER-MAJOR) path: the dot will be added(path.format) if it is not specified in `ext` (theanarkh) #44349 * [587367d] - (SEMVER-MAJOR) perf_hooks: expose webperf global scope interfaces (Chengzhong Wu) #44483 * [364c0e1] - (SEMVER-MAJOR) perf_hooks: fix webperf idlharness (Chengzhong Wu) #44483 * [ada2d05] - (SEMVER-MAJOR) process: runtime deprecate coercion to integer in `process.exit()` (Daeyeon Jeong) #44711 * [e0ab8dd] - (SEMVER-MAJOR) process: make process.config read only (Sergey Petushkov) #43627 * [481a959] - (SEMVER-MAJOR) readline: remove `question` method from `InterfaceConstructor` (Antoine du Hamel) #44606 * [c9602ce] - (SEMVER-MAJOR) src: use new v8::OOMErrorCallback API (Michaël Zasso) #44741 * [19a70c1] - (SEMVER-MAJOR) src: override CreateJob instead of PostJob (Clemens Backes) #44741 * [fd52c62] - (SEMVER-MAJOR) src: use V8_ENABLE_SANDBOX macro (Michaël Zasso) #44741 * [c10988d] - (SEMVER-MAJOR) src: use non-deprecated V8 inspector API (Michaël Zasso) #44741 * [3efe901] - (SEMVER-MAJOR) src: update NODE_MODULE_VERSION to 111 (Michaël Zasso) #44741 * [77e5856] - (SEMVER-MAJOR) src: turn embedder api overload into default argument (Alena Khineika) #43629 * [dabda03] - (SEMVER-MAJOR) src: per-environment time origin value (Chengzhong Wu) #43781 * [2e49b99] - (SEMVER-MAJOR) src,test: disable freezing V8 flags on initialization (Clemens Backes) #44741 * [2b32985] - (SEMVER-MAJOR) stream: use null for the error argument (Luigi Pinca) #44312 * [36805e8] - (SEMVER-MAJOR) test: adapt test-repl for V8 update (Michaël Zasso) #44741 * [96ef257] - (SEMVER-MAJOR) test: adapt test-repl-pretty-*stack to V8 changes (Michaël Zasso) #44741 * [71c193e] - (SEMVER-MAJOR) test: adapt to new JSON SyntaxError messages (Michaël Zasso) #44741 * [b5f1564] - (SEMVER-MAJOR) test: rename always-opt flag to always-turbofan (Michaël Zasso) #44741 * [1acf033] - (SEMVER-MAJOR) test: fix test-hash-seed for new V8 versions (Michaël Zasso) #44741 * [57ff476] - (SEMVER-MAJOR) test: remove duplicate test (Luigi Pinca) #44051 * [77def91] - (SEMVER-MAJOR) tls,http2: send fatal alert on ALPN mismatch (Tobias Nießen) #44031 * [4860ad9] - (SEMVER-MAJOR) tools: update V8 gypfiles for 10.7 (Michaël Zasso) #44741 PR-URL: #44626 Co-authored-by: Ruy Adorno <[email protected]>

This is a documentation-deprecation only and it is possible that it will not proceed to a runtime-deprecation any time in the foreseeable future. But url.parse() is not standardized and prone to errors that have security implications. Refs: #44911 (comment) PR-URL: #44919 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Danielle Adams <[email protected]>

Notable changes: Add support for externally shared js builtins: By default Node.js is built so that all dependencies are bundled into the Node.js binary itself. Some Node.js distributions prefer to manage dependencies externally. There are existing build options that allow dependencies with native code to be externalized. This commit adds additional options so that dependencies with JavaScript code (including WASM) can also be externalized. This addition does not affect binaries shipped by the Node.js project but will allow other distributions to externalize additional dependencies when needed. Contributed by Michael Dawson in #44376 fs.watch recursive support on Linux: `fs.watch` supports recursive watch using the `recursive: true` option. ```js const watcher = fs.watch(testDirectory, { recursive: true }); watcher.on('change', function(event, filename) { }); ``` Contributed by Yagiz Nizipli in #45098 Support function mocking on Node.js test runner: The `node:test` module supports mocking during testing via a top-level `mock` object. ```js test('spies on an object method', (t) => { const number = { value: 5, add(a) { return this.value + a; }, }; t.mock.method(number, 'add'); assert.strictEqual(number.add(3), 8); assert.strictEqual(number.add.mock.calls.length, 1); }); ``` Contributed by Colin Ihrig in #45326 Other notable changes: buffer: * (SEMVER-MINOR) introduce File (Khafra) #45139 build: * disable v8 snapshot compression by default (Joyee Cheung) #45716 crypto: * update root certificates (Luigi Pinca) #45490 deps: * update ICU to 72.1 (Michaël Zasso) #45068 doc: * add doc-only deprecation for headers/trailers setters (Rich Trott) #45697 * add Rafael to the tsc (Michael Dawson) #45691 * deprecate use of invalid ports in `url.parse` (Antoine du Hamel) #45576 * add lukekarrys to collaborators (Luke Karrys) #45180 * add anonrig to collaborators (Yagiz Nizipli) #45002 * deprecate url.parse() (Rich Trott) #44919 lib: * drop fetch experimental warning (Matteo Collina) #45287 net: * (SEMVER-MINOR) add autoSelectFamily and autoSelectFamilyAttemptTimeout options (Paolo Insogna) #44731 * src: * (SEMVER-MINOR) add uvwasi version (Jithil P Ponnan) #45639 * (SEMVER-MINOR) add initial shadow realm support (Chengzhong Wu) #42869 test_runner: * (SEMVER-MINOR) add t.after() hook (Colin Ihrig) #45792 * (SEMVER-MINOR) don't use a symbol for runHook() (Colin Ihrig) #45792 tls: * (SEMVER-MINOR) add "ca" property to certificate object (Ben Noordhuis) #44935 * remove trustcor root ca certificates (Ben Noordhuis) #45776 tools: * update certdata.txt (Luigi Pinca) #45490 util: * add fast path for utf8 encoding (Yagiz Nizipli) #45412 * improve textdecoder decode performance (Yagiz Nizipli) #45294 * (SEMVER-MINOR) add MIME utilities (#21128) (Bradley Farias) #21128 PR-URL: TBD

Notable changes: Add support for externally shared js builtins: By default Node.js is built so that all dependencies are bundled into the Node.js binary itself. Some Node.js distributions prefer to manage dependencies externally. There are existing build options that allow dependencies with native code to be externalized. This commit adds additional options so that dependencies with JavaScript code (including WASM) can also be externalized. This addition does not affect binaries shipped by the Node.js project but will allow other distributions to externalize additional dependencies when needed. Contributed by Michael Dawson in #44376 fs.watch recursive support on Linux: `fs.watch` supports recursive watch using the `recursive: true` option. ```js const watcher = fs.watch(testDirectory, { recursive: true }); watcher.on('change', function(event, filename) { }); ``` Contributed by Yagiz Nizipli in #45098 Support function mocking on Node.js test runner: The `node:test` module supports mocking during testing via a top-level `mock` object. ```js test('spies on an object method', (t) => { const number = { value: 5, add(a) { return this.value + a; }, }; t.mock.method(number, 'add'); assert.strictEqual(number.add(3), 8); assert.strictEqual(number.add.mock.calls.length, 1); }); ``` Contributed by Colin Ihrig in #45326 Other notable changes: buffer: * (SEMVER-MINOR) introduce File (Khafra) #45139 build: * disable v8 snapshot compression by default (Joyee Cheung) #45716 crypto: * update root certificates (Luigi Pinca) #45490 deps: * update ICU to 72.1 (Michaël Zasso) #45068 doc: * add doc-only deprecation for headers/trailers setters (Rich Trott) #45697 * add Rafael to the tsc (Michael Dawson) #45691 * deprecate use of invalid ports in `url.parse` (Antoine du Hamel) #45576 * add lukekarrys to collaborators (Luke Karrys) #45180 * add anonrig to collaborators (Yagiz Nizipli) #45002 * deprecate url.parse() (Rich Trott) #44919 lib: * drop fetch experimental warning (Matteo Collina) #45287 net: * (SEMVER-MINOR) add autoSelectFamily and autoSelectFamilyAttemptTimeout options (Paolo Insogna) #44731 * src: * (SEMVER-MINOR) add uvwasi version (Jithil P Ponnan) #45639 * (SEMVER-MINOR) add initial shadow realm support (Chengzhong Wu) #42869 test_runner: * (SEMVER-MINOR) add t.after() hook (Colin Ihrig) #45792 * (SEMVER-MINOR) don't use a symbol for runHook() (Colin Ihrig) #45792 tls: * (SEMVER-MINOR) add "ca" property to certificate object (Ben Noordhuis) #44935 * remove trustcor root ca certificates (Ben Noordhuis) #45776 tools: * update certdata.txt (Luigi Pinca) #45490 util: * add fast path for utf8 encoding (Yagiz Nizipli) #45412 * improve textdecoder decode performance (Yagiz Nizipli) #45294 * (SEMVER-MINOR) add MIME utilities (#21128) (Bradley Farias) #21128 PR-URL: #46025

Notable changes: Add support for externally shared js builtins: By default Node.js is built so that all dependencies are bundled into the Node.js binary itself. Some Node.js distributions prefer to manage dependencies externally. There are existing build options that allow dependencies with native code to be externalized. This commit adds additional options so that dependencies with JavaScript code (including WASM) can also be externalized. This addition does not affect binaries shipped by the Node.js project but will allow other distributions to externalize additional dependencies when needed. Contributed by Michael Dawson in #44376 Introduce `File`: The File class is part of the [FileAPI](https://w3c.github.io/FileAPI/). It can be used anywhere a Blob can, for example in `URL.createObjectURL` and `FormData`. It contains two properties that Blobs do not have: `lastModified`, the last time the file was modified in ms, and `name`, the name of the file. Contributed by Khafra in #45139 Support function mocking on Node.js test runner: The `node:test` module supports mocking during testing via a top-level `mock` object. ```js test('spies on an object method', (t) => { const number = { value: 5, add(a) { return this.value + a; }, }; t.mock.method(number, 'add'); assert.strictEqual(number.add(3), 8); assert.strictEqual(number.add.mock.calls.length, 1); }); ``` Contributed by Colin Ihrig in #45326 Other notable changes: build: * disable v8 snapshot compression by default (Joyee Cheung) #45716 crypto: * update root certificates (Luigi Pinca) #45490 deps: * update ICU to 72.1 (Michaël Zasso) #45068 doc: * add doc-only deprecation for headers/trailers setters (Rich Trott) #45697 * add Rafael to the tsc (Michael Dawson) #45691 * deprecate use of invalid ports in `url.parse` (Antoine du Hamel) #45576 * add lukekarrys to collaborators (Luke Karrys) #45180 * add anonrig to collaborators (Yagiz Nizipli) #45002 * deprecate url.parse() (Rich Trott) #44919 lib: * drop fetch experimental warning (Matteo Collina) #45287 net: * (SEMVER-MINOR) add autoSelectFamily and autoSelectFamilyAttemptTimeout options (Paolo Insogna) #44731 * src: * (SEMVER-MINOR) add uvwasi version (Jithil P Ponnan) #45639 * (SEMVER-MINOR) add initial shadow realm support (Chengzhong Wu) #42869 test_runner: * (SEMVER-MINOR) add t.after() hook (Colin Ihrig) #45792 * (SEMVER-MINOR) don't use a symbol for runHook() (Colin Ihrig) #45792 tls: * (SEMVER-MINOR) add "ca" property to certificate object (Ben Noordhuis) #44935 * remove trustcor root ca certificates (Ben Noordhuis) #45776 tools: * update certdata.txt (Luigi Pinca) #45490 util: * add fast path for utf8 encoding (Yagiz Nizipli) #45412 * improve textdecoder decode performance (Yagiz Nizipli) #45294 * (SEMVER-MINOR) add MIME utilities (#21128) (Bradley Farias) #21128 PR-URL: #46025

styfle · 2022-12-31T04:02:49Z

doc/api/url.md

-`url.parse()` is an exception to most of the legacy APIs. Despite its security
-concerns, it is legacy and not deprecated because it is:
-
-* Faster than the alternative WHATWG `URL` parser.


Do we know which one is faster?

I suspect @anonrig might be able to answer that. Generally, I'd say url.parse() is usually faster, but not necessarily by a whole lot in the typical use cases. And it is more susceptible to things like host spoofing than WHATWG URL. But anything @anonrig says to the contrary should be treated as the more informed opinion that it is.

Url.parse is faster but not spec compliant.

When parsing a relative URL, then there is no possibility of host spoofing or username/password issues, right?

I can’t answer that question with 100% uncertainity, and I think nobody can.

Let me rephrase the question:

Has anyone reported bugs or vulnerabilities for relative urls as inputs to url.parse()?

url.parse() used to treat the \bhttp://example.com/\b as a hostless URL. WHATWG URL strips leading and trailing backspaces and treats it as a fully-qualified URL. So that's an example where url.parse() mishandling a URL and treating it like a hostless URL could lead to problems. (This issue has been fixed.)

I see. I'm thinking more along the lines of URLs that start with /, like req.url

function handler(req, res) { if (req.url.startsWith('/')) { const parsed = url.parse(req.url) parsed.search = '?v=2' const result = url.format(parsed) res.end(`Modified url: ${result}`) } else { throw new Error('Expected req.url to be relative') } }

This is the case for which url.parse() and url.format() is useful, and theres not a good solution today using new URL().

styfle · 2022-12-31T04:03:16Z

doc/api/url.md

-concerns, it is legacy and not deprecated because it is:
-
-* Faster than the alternative WHATWG `URL` parser.
-* Easier to use with regards to relative URLs than the alternative WHATWG `URL` API.


Do we have a solution for relative URLs using WHATWG?

If you know the base URL, then yes. If not, you have to fake it, perhaps with an identifiably bogus base URL. In that second case, it's a solution, but perhaps not a particularly elegant one.

My opinion only, but a lot of the current problems with url.parse() result from the design decision (probably sensible at the time) to handle both full URLs and relative URLs without knowing the base URL.

Ref: #12682
Ref: whatwg/url#531

Thanks @Trott

My worry is that marking url.parse() as deprecated is that we introduce new problems when someone assumes new URL() will work as a replacement but it wont for relative URLS. And in fact, I think most code I've seen is intending to use it with relative urls like that of url.parse(req.url).

I remember when this was "deprecated" the first time, it confused a lot of people and then it was reverted to say "legacy" which I think was the right decision.

Or maybe the solution here is to document how to parse relative urls, such as req.url.

There's whatwg/url#421 that would fit the use-case, unfortunately the proposal look rather stalled.

I’ve seen that one too.

It has a label “needs implementer interest”. Does that need to be a browser or can we say Node.js is interested in implementing?

The proposal comes from Node.js, I think it's implied that Node.js is interested, I think we need other implementers as well.

@aduh95 The discussion around whatwg/url#421 more or less has been continued in whatwg/url#531.

@styfle I have spent a lot of effort to find a solution for relative URLs that agrees with the WHATWG Standard, see e.g. this comment and also this URL Specification and this library.

This was indeed far from trivial due to design decisions made in the WHATWG standard.

I have an idea for a simple API ready, but I stopped working on it for a while. In part because it is unlikely that the WHATWG standard can incrementally add support for relative URLs. This leads to an understandable resistance, which nonetheless is frustrating to me and something that I don’t yet know how to solve.

The other reason is simply being preoccupied with other work at the moment.

Suggestions and ideas are appreciated!

Looks interesting. I think WHATWG is probably the wrong place to standardize this since browsers aren't interested. I think WinterCG is a better choice for standardization of relative url parsing since it is focused on server-side JS.

Personally I think that marking this API as deprecated should've been gated on a suitable replacement being made available, and perhaps that would've introduced some incentive to come to a consensus on what that replacement should look like.

Relative URLs are a thing, and we need a suitable API to replace this one.

This is a documentation-deprecation only and it is possible that it will not proceed to a runtime-deprecation any time in the foreseeable future. But url.parse() is not standardized and prone to errors that have security implications. Refs: #44911 (comment) PR-URL: #44919 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Danielle Adams <[email protected]>

Notable changes: Add support for externally shared js builtins: By default Node.js is built so that all dependencies are bundled into the Node.js binary itself. Some Node.js distributions prefer to manage dependencies externally. There are existing build options that allow dependencies with native code to be externalized. This commit adds additional options so that dependencies with JavaScript code (including WASM) can also be externalized. This addition does not affect binaries shipped by the Node.js project but will allow other distributions to externalize additional dependencies when needed. Contributed by Michael Dawson in #44376 Introduce `File`: The File class is part of the [FileAPI](https://w3c.github.io/FileAPI/). It can be used anywhere a Blob can, for example in `URL.createObjectURL` and `FormData`. It contains two properties that Blobs do not have: `lastModified`, the last time the file was modified in ms, and `name`, the name of the file. Contributed by Khafra in #45139 Support function mocking on Node.js test runner: The `node:test` module supports mocking during testing via a top-level `mock` object. ```js test('spies on an object method', (t) => { const number = { value: 5, add(a) { return this.value + a; }, }; t.mock.method(number, 'add'); assert.strictEqual(number.add(3), 8); assert.strictEqual(number.add.mock.calls.length, 1); }); ``` Contributed by Colin Ihrig in #45326 Other notable changes: build: * disable v8 snapshot compression by default (Joyee Cheung) #45716 crypto: * update root certificates (Luigi Pinca) #45490 deps: * update ICU to 72.1 (Michaël Zasso) #45068 doc: * add doc-only deprecation for headers/trailers setters (Rich Trott) #45697 * add Rafael to the tsc (Michael Dawson) #45691 * deprecate use of invalid ports in `url.parse` (Antoine du Hamel) #45576 * add lukekarrys to collaborators (Luke Karrys) #45180 * add anonrig to collaborators (Yagiz Nizipli) #45002 * deprecate url.parse() (Rich Trott) #44919 lib: * drop fetch experimental warning (Matteo Collina) #45287 net: * (SEMVER-MINOR) add autoSelectFamily and autoSelectFamilyAttemptTimeout options (Paolo Insogna) #44731 * src: * (SEMVER-MINOR) add uvwasi version (Jithil P Ponnan) #45639 * (SEMVER-MINOR) add initial shadow realm support (Chengzhong Wu) #42869 test_runner: * (SEMVER-MINOR) add t.after() hook (Colin Ihrig) #45792 * (SEMVER-MINOR) don't use a symbol for runHook() (Colin Ihrig) #45792 tls: * (SEMVER-MINOR) add "ca" property to certificate object (Ben Noordhuis) #44935 * remove trustcor root ca certificates (Ben Noordhuis) #45776 tools: * update certdata.txt (Luigi Pinca) #45490 util: * add fast path for utf8 encoding (Yagiz Nizipli) #45412 * improve textdecoder decode performance (Yagiz Nizipli) #45294 * (SEMVER-MINOR) add MIME utilities (#21128) (Bradley Farias) #21128 PR-URL: #46025

nodejs-github-bot added the doc Issues and PRs related to the documentations. label Oct 8, 2022

Trott added the notable-change PRs with changes that should be highlighted in changelogs. label Oct 8, 2022

GeoffreyBooth approved these changes Oct 8, 2022

View reviewed changes

mcollina approved these changes Oct 8, 2022

View reviewed changes

lpinca approved these changes Oct 8, 2022

View reviewed changes

mcollina reviewed Oct 8, 2022

View reviewed changes

mcollina added the tsc-agenda Issues and PRs to discuss during the meetings of the TSC. label Oct 8, 2022

RafaelGSS approved these changes Oct 8, 2022

View reviewed changes

juanarbol approved these changes Oct 9, 2022

View reviewed changes

aduh95 approved these changes Oct 10, 2022

View reviewed changes

mhdawson mentioned this pull request Oct 10, 2022

Node.js Technical Steering Committee (TSC) Meeting 2022-10-12 nodejs/TSC#1291

Closed

Trott added 2 commits October 11, 2022 11:17

fixup! doc: deprecate url.parse()

8c3fd95

Trott force-pushed the url-dep branch from 3a661f0 to 8c3fd95 Compare October 11, 2022 18:20

fixup! fixup! doc: deprecate url.parse()

8975b00

Trott added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label Oct 11, 2022

Trott added 3 commits October 11, 2022 11:24

fixup! fixup! fixup! doc: deprecate url.parse()

8829ec5

fixup! fixup! fixup! fixup! doc: deprecate url.parse()

5005966

fixup! fixup! fixup! fixup! fixup! doc: deprecate url.parse()

e14b5a5

jasnell reviewed Oct 11, 2022

View reviewed changes

doc/api/deprecations.md Outdated Show resolved Hide resolved

aduh95 approved these changes Oct 11, 2022

View reviewed changes

mhdawson approved these changes Oct 11, 2022

View reviewed changes

Update doc/api/deprecations.md

0ce81e0

Co-authored-by: James M Snell <[email protected]>

danielleadams mentioned this pull request Dec 30, 2022

v18.13.0 release proposal #46025

Merged

styfle reviewed Dec 31, 2022

View reviewed changes

styfle mentioned this pull request Jan 1, 2023

fix(resolution): Fix resolution of querystringed imports in ESM files vercel/nft#336

Closed

styfle mentioned this pull request Jan 4, 2023

URL.relative proposal whatwg/url#421

Open

This was referenced Jan 19, 2023

using Whatwg url to parse relative urls received via http may fail #35458

Closed

URL's on incoming requests nodejs/web-server-frameworks#71

Open

doc: deprecate url.parse() #44919

doc: deprecate url.parse() #44919

Conversation

Trott commented Oct 8, 2022

Trott commented Oct 8, 2022

mcollina left a comment

Choose a reason for hiding this comment

lpinca commented Oct 8, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasnell commented Oct 8, 2022

mcollina commented Oct 8, 2022

Trott commented Oct 8, 2022

mcollina commented Oct 8, 2022

RafaelGSS left a comment

Choose a reason for hiding this comment

juanarbol left a comment

Choose a reason for hiding this comment

aduh95 left a comment

Choose a reason for hiding this comment

Trott commented Oct 11, 2022

aduh95 left a comment

Choose a reason for hiding this comment

mhdawson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

styfle Jan 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Trott Jan 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lpinca commented Oct 8, 2022 •

edited

Loading

styfle Jan 1, 2023 •

edited

Loading

Trott Jan 2, 2023 •

edited

Loading