From a8d88e812207ecd36c1560f7004edb9195d39e13 Mon Sep 17 00:00:00 2001 From: Mohamed Sayed Date: Wed, 27 May 2026 19:37:54 +0300 Subject: [PATCH 1/5] lib: brand-check Navigator getters to throw ERR_INVALID_THIS Fixes: https://github.com/nodejs/node/issues/63513 Signed-off-by: Mohamed Sayed --- lib/internal/navigator.js | 11 ++++++++++- test/parallel/test-navigator.js | 30 ++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/lib/internal/navigator.js b/lib/internal/navigator.js index 2904779a47e64b..a227929e143faa 100644 --- a/lib/internal/navigator.js +++ b/lib/internal/navigator.js @@ -11,6 +11,7 @@ const { const { ERR_ILLEGAL_CONSTRUCTOR, + ERR_INVALID_THIS, } = require('internal/errors').codes; const { @@ -79,7 +80,9 @@ function getNavigatorPlatform(arch, platform) { } class Navigator { - // Private properties are used to avoid brand validations. + // Private properties also serve as a brand check via the `#field in obj` + // operator in each getter, so callers get a proper TypeError instead of + // an internal "private field" error when invoked on a non-Navigator `this`. #availableParallelism; #locks; #userAgent; @@ -97,6 +100,7 @@ class Navigator { * @returns {number} */ get hardwareConcurrency() { + if (!(#availableParallelism in this)) throw new ERR_INVALID_THIS('Navigator'); this.#availableParallelism ??= getAvailableParallelism(); return this.#availableParallelism; } @@ -105,6 +109,7 @@ class Navigator { * @returns {LockManager} */ get locks() { + if (!(#locks in this)) throw new ERR_INVALID_THIS('Navigator'); this.#locks ??= require('internal/locks').locks; return this.#locks; } @@ -113,6 +118,7 @@ class Navigator { * @returns {string} */ get language() { + if (!(#languages in this)) throw new ERR_INVALID_THIS('Navigator'); // The default locale might be changed dynamically, so always invoke the // binding. return getDefaultLocale() || 'en-US'; @@ -122,6 +128,7 @@ class Navigator { * @returns {Array} */ get languages() { + if (!(#languages in this)) throw new ERR_INVALID_THIS('Navigator'); this.#languages ??= ObjectFreeze([this.language]); return this.#languages; } @@ -130,6 +137,7 @@ class Navigator { * @returns {string} */ get userAgent() { + if (!(#userAgent in this)) throw new ERR_INVALID_THIS('Navigator'); this.#userAgent ??= `Node.js/${StringPrototypeSlice(nodeVersion, 1, StringPrototypeIndexOf(nodeVersion, '.'))}`; return this.#userAgent; } @@ -138,6 +146,7 @@ class Navigator { * @returns {string} */ get platform() { + if (!(#platform in this)) throw new ERR_INVALID_THIS('Navigator'); this.#platform ??= getNavigatorPlatform(arch, platform); return this.#platform; } diff --git a/test/parallel/test-navigator.js b/test/parallel/test-navigator.js index 01c5595b93cd12..35b1d5e093936c 100644 --- a/test/parallel/test-navigator.js +++ b/test/parallel/test-navigator.js @@ -147,3 +147,33 @@ Object.defineProperty(navigator, 'language', { value: 'for-testing' }); assert.strictEqual(navigator.language, 'for-testing'); assert.strictEqual(navigator.languages.length, 1); assert.strictEqual(navigator.languages[0] !== 'for-testing', true); + +{ + const { Navigator } = globalThis; + const getterNames = [ + 'hardwareConcurrency', + 'locks', + 'language', + 'languages', + 'userAgent', + 'platform', + ]; + for (const name of getterNames) { + assert.throws( + () => Navigator.prototype[name], + { + name: 'TypeError', + code: 'ERR_INVALID_THIS', + }, + `expected TypeError when reading ${name} on Navigator.prototype`, + ); + assert.throws( + () => Reflect.get(Navigator.prototype, name, {}), + { + name: 'TypeError', + code: 'ERR_INVALID_THIS', + }, + `expected TypeError when reading ${name} on a plain object`, + ); + } +} From 57fda3bac63e4159b2c86223f8375ad19a92d44a Mon Sep 17 00:00:00 2001 From: Mohamed Sayed Date: Thu, 28 May 2026 07:25:26 +0300 Subject: [PATCH 2/5] fix: iterate Navigator.prototype instead of lists Signed-off-by: Mohamed Sayed --- test/parallel/test-navigator.js | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/test/parallel/test-navigator.js b/test/parallel/test-navigator.js index 35b1d5e093936c..828d2221d181f7 100644 --- a/test/parallel/test-navigator.js +++ b/test/parallel/test-navigator.js @@ -150,15 +150,7 @@ assert.strictEqual(navigator.languages[0] !== 'for-testing', true); { const { Navigator } = globalThis; - const getterNames = [ - 'hardwareConcurrency', - 'locks', - 'language', - 'languages', - 'userAgent', - 'platform', - ]; - for (const name of getterNames) { + for (const name of Object.keys(Navigator.prototype)) { assert.throws( () => Navigator.prototype[name], { From a35e81bc68d2b6cca3d49cdaef24fc9ad79daac4 Mon Sep 17 00:00:00 2001 From: Mohamed Sayed Date: Sun, 31 May 2026 18:19:17 +0300 Subject: [PATCH 3/5] fix: keep explicit brand check and rely on private field throw Signed-off-by: Mohamed Sayed --- lib/internal/navigator.js | 13 +++++-------- test/parallel/test-navigator.js | 10 ++-------- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/lib/internal/navigator.js b/lib/internal/navigator.js index a227929e143faa..64e9600167f050 100644 --- a/lib/internal/navigator.js +++ b/lib/internal/navigator.js @@ -80,9 +80,8 @@ function getNavigatorPlatform(arch, platform) { } class Navigator { - // Private properties also serve as a brand check via the `#field in obj` - // operator in each getter, so callers get a proper TypeError instead of - // an internal "private field" error when invoked on a non-Navigator `this`. + // Private properties are used to avoid brand validations: reading a private + // field from a non-Navigator receiver throws a TypeError on its own. #availableParallelism; #locks; #userAgent; @@ -100,7 +99,6 @@ class Navigator { * @returns {number} */ get hardwareConcurrency() { - if (!(#availableParallelism in this)) throw new ERR_INVALID_THIS('Navigator'); this.#availableParallelism ??= getAvailableParallelism(); return this.#availableParallelism; } @@ -109,7 +107,6 @@ class Navigator { * @returns {LockManager} */ get locks() { - if (!(#locks in this)) throw new ERR_INVALID_THIS('Navigator'); this.#locks ??= require('internal/locks').locks; return this.#locks; } @@ -118,6 +115,9 @@ class Navigator { * @returns {string} */ get language() { + // `language` does not read a private field, so brand-check explicitly + // to keep parity with the other getters when called on a non-Navigator + // receiver (e.g. `Navigator.prototype.language`). if (!(#languages in this)) throw new ERR_INVALID_THIS('Navigator'); // The default locale might be changed dynamically, so always invoke the // binding. @@ -128,7 +128,6 @@ class Navigator { * @returns {Array} */ get languages() { - if (!(#languages in this)) throw new ERR_INVALID_THIS('Navigator'); this.#languages ??= ObjectFreeze([this.language]); return this.#languages; } @@ -137,7 +136,6 @@ class Navigator { * @returns {string} */ get userAgent() { - if (!(#userAgent in this)) throw new ERR_INVALID_THIS('Navigator'); this.#userAgent ??= `Node.js/${StringPrototypeSlice(nodeVersion, 1, StringPrototypeIndexOf(nodeVersion, '.'))}`; return this.#userAgent; } @@ -146,7 +144,6 @@ class Navigator { * @returns {string} */ get platform() { - if (!(#platform in this)) throw new ERR_INVALID_THIS('Navigator'); this.#platform ??= getNavigatorPlatform(arch, platform); return this.#platform; } diff --git a/test/parallel/test-navigator.js b/test/parallel/test-navigator.js index 828d2221d181f7..82e3fe903d455f 100644 --- a/test/parallel/test-navigator.js +++ b/test/parallel/test-navigator.js @@ -153,18 +153,12 @@ assert.strictEqual(navigator.languages[0] !== 'for-testing', true); for (const name of Object.keys(Navigator.prototype)) { assert.throws( () => Navigator.prototype[name], - { - name: 'TypeError', - code: 'ERR_INVALID_THIS', - }, + { name: 'TypeError' }, `expected TypeError when reading ${name} on Navigator.prototype`, ); assert.throws( () => Reflect.get(Navigator.prototype, name, {}), - { - name: 'TypeError', - code: 'ERR_INVALID_THIS', - }, + { name: 'TypeError' }, `expected TypeError when reading ${name} on a plain object`, ); } From d0a014b21a17394e7f51340eb4d9b31aa2e2dd56 Mon Sep 17 00:00:00 2001 From: Mohamed Sayed Date: Mon, 1 Jun 2026 20:28:53 +0300 Subject: [PATCH 4/5] fix: depend on this.#attribute throw on navigator Signed-off-by: Mohamed Sayed --- lib/internal/navigator.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/navigator.js b/lib/internal/navigator.js index 64e9600167f050..ad9038831f2ce1 100644 --- a/lib/internal/navigator.js +++ b/lib/internal/navigator.js @@ -118,7 +118,7 @@ class Navigator { // `language` does not read a private field, so brand-check explicitly // to keep parity with the other getters when called on a non-Navigator // receiver (e.g. `Navigator.prototype.language`). - if (!(#languages in this)) throw new ERR_INVALID_THIS('Navigator'); + this.#languages; // eslint-disable-line no-unused-expressions // The default locale might be changed dynamically, so always invoke the // binding. return getDefaultLocale() || 'en-US'; From 19c4154b9aeef71fc938750561884d4209a8e9ab Mon Sep 17 00:00:00 2001 From: Mohamed Sayed Date: Mon, 1 Jun 2026 22:17:23 +0300 Subject: [PATCH 5/5] fix: unused ERR_INVALID_THIS import Signed-off-by: Mohamed Sayed --- lib/internal/navigator.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/internal/navigator.js b/lib/internal/navigator.js index ad9038831f2ce1..d8681c89990494 100644 --- a/lib/internal/navigator.js +++ b/lib/internal/navigator.js @@ -11,7 +11,6 @@ const { const { ERR_ILLEGAL_CONSTRUCTOR, - ERR_INVALID_THIS, } = require('internal/errors').codes; const { @@ -118,7 +117,7 @@ class Navigator { // `language` does not read a private field, so brand-check explicitly // to keep parity with the other getters when called on a non-Navigator // receiver (e.g. `Navigator.prototype.language`). - this.#languages; // eslint-disable-line no-unused-expressions + this.#languages; // eslint-disable-line no-unused-expressions // The default locale might be changed dynamically, so always invoke the // binding. return getDefaultLocale() || 'en-US';