lib: throw from localStorage getter on missing storage path

Author: Renegade334

doc/api/errors.md

@@ -3354,6 +3354,17 @@ added: v18.1.0
 The `Response` that has been passed to `WebAssembly.compileStreaming` or to
 `WebAssembly.instantiateStreaming` is not a valid WebAssembly response.
 
+<a id="ERR_WEBSTORAGE_MISSING_STORAGE_PATH"></a>
+
+### `ERR_WEBSTORAGE_MISSING_STORAGE_PATH`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+An attempt was made to access the global [`localStorage`][] object, but the
+[`--localstorage-file`][] command-line argument was not provided.
+
 <a id="ERR_WORKER_INIT_FAILED"></a>
 
 ### `ERR_WORKER_INIT_FAILED`
@@ -4370,6 +4381,7 @@ An error occurred trying to allocate memory. This should never happen.
 [`'uncaughtException'`]: process.md#event-uncaughtexception
 [`--disable-proto=throw`]: cli.md#--disable-protomode
 [`--force-fips`]: cli.md#--force-fips
+[`--localstorage-file`]: cli.md#--localstorage-filefile
 [`--no-addons`]: cli.md#--no-addons
 [`--unhandled-rejections`]: cli.md#--unhandled-rejectionsmode
 [`Class: assert.AssertionError`]: assert.md#class-assertassertionerror
@@ -4412,6 +4424,7 @@ An error occurred trying to allocate memory. This should never happen.
 [`http`]: http.md
 [`https`]: https.md
 [`libuv Error handling`]: https://docs.libuv.org/en/v1.x/errors.html
+[`localStorage`]: globals.md#localstorage
 [`net.Socket.write()`]: net.md#socketwritedata-encoding-callback
 [`net`]: net.md
 [`new URL(input)`]: url.md#new-urlinput-base

lib/internal/errors.js

@@ -1904,6 +1904,8 @@ E('ERR_VM_MODULE_NOT_MODULE',
 E('ERR_VM_MODULE_STATUS', 'Module status %s', Error);
 E('ERR_WASI_ALREADY_STARTED', 'WASI instance has already started', Error);
 E('ERR_WEBASSEMBLY_RESPONSE', 'WebAssembly response %s', TypeError);
+E('ERR_WEBSTORAGE_MISSING_STORAGE_PATH',
+  'Cannot initialize local storage without a `--localstorage-file` path', Error);
 E('ERR_WORKER_INIT_FAILED', 'Worker initialization failure: %s', Error);
 E('ERR_WORKER_INVALID_EXEC_ARGV', (errors, msg = 'invalid execArgv flags') =>
   `Initiated Worker with ${msg}: ${ArrayPrototypeJoin(errors, ', ')}`,

lib/internal/webstorage.js

@@ -1,9 +1,13 @@
 'use strict';
 const {
   ObjectDefineProperties,
-  Proxy,
 } = primordials;
 const { getOptionValue } = require('internal/options');
+const {
+  codes: {
+    ERR_WEBSTORAGE_MISSING_STORAGE_PATH,
+  },
+} = require('internal/errors');
 const { kConstructorKey, Storage } = internalBinding('webstorage');
 const { getValidatedPath } = require('internal/fs/utils');
 const kInMemoryPath = ':memory:';
@@ -21,34 +25,14 @@ ObjectDefineProperties(module.exports, {
     enumerable: true,
     get() {
       if (lazyLocalStorage === undefined) {
+        // For consistency with the web specification, throw from the accessor
+        // if the local storage path is not provided.
         const location = getOptionValue('--localstorage-file');
-
         if (location === '') {
-          let warningEmitted = false;
-          const handler = {
-            __proto__: null,
-            get(target, prop) {
-              if (!warningEmitted) {
-                process.emitWarning('`--localstorage-file` was provided without a valid path');
-                warningEmitted = true;
-              }
-
-              return undefined;
-            },
-            set(target, prop, value) {
-              if (!warningEmitted) {
-                process.emitWarning('`--localstorage-file` was provided without a valid path');
-                warningEmitted = true;
-              }
-
-              return false;
-            },
-          };
-
-          lazyLocalStorage = new Proxy({}, handler);
-        } else {
-          lazyLocalStorage = new Storage(kConstructorKey, getValidatedPath(location));
+          throw new ERR_WEBSTORAGE_MISSING_STORAGE_PATH();
         }
+
+        lazyLocalStorage = new Storage(kConstructorKey, getValidatedPath(location));
       }
 
       return lazyLocalStorage;

test/common/index.js

@@ -59,6 +59,14 @@ const hasSQLite = Boolean(process.versions.sqlite);
 
 const hasQuic = hasCrypto && !!process.features.quic;
 
+const hasLocalStorage = (() => {
+  try {
+    return hasSQLite && globalThis.localStorage !== undefined;
+  } catch {
+    return false;
+  }
+})();
+
 /**
  * Parse test metadata from the specified file.
  * @param {string} filename - The name of the file to parse.
@@ -350,14 +358,17 @@ const knownGlobals = new Set([
  'CompressionStream',
  'DecompressionStream',
  'Storage',
- 'localStorage',
  'sessionStorage',
 ].forEach((i) => {
   if (globalThis[i] !== undefined) {
     knownGlobals.add(globalThis[i]);
   }
 });
 
+if (hasLocalStorage) {
+  knownGlobals.add(globalThis.localStorage);
+}
+
 if (hasCrypto) {
   knownGlobals.add(globalThis.crypto);
   knownGlobals.add(globalThis.Crypto);
@@ -389,6 +400,11 @@ if (process.env.NODE_TEST_KNOWN_GLOBALS !== '0') {
       if (val === 'crypto' && !hasCrypto) {
         continue;
       }
+      // globalThis.localStorage is a getter that throws if Node.js was
+      // executed without a --localstorage-file path.
+      if (val === 'localStorage' && !hasLocalStorage) {
+        continue;
+      }
       if (!knownGlobals.has(globalThis[val])) {
         leaked.push(val);
       }
@@ -933,6 +949,7 @@ const common = {
   hasQuic,
   hasInspector,
   hasSQLite,
+  hasLocalStorage,
   invalidArgTypeHelper,
   isAlive,
   isASan,

test/common/index.mjs

@@ -19,6 +19,7 @@ const {
   hasQuic,
   hasInspector,
   hasSQLite,
+  hasLocalStorage,
   hasIntl,
   hasIPv6,
   isAIX,
@@ -71,6 +72,7 @@ export {
   hasQuic,
   hasInspector,
   hasSQLite,
+  hasLocalStorage,
   hasIntl,
   hasIPv6,
   isAIX,

test/parallel/test-assert-checktag.js

@@ -1,5 +1,5 @@
 'use strict';
-const { hasCrypto } = require('../common');
+const { hasCrypto, hasLocalStorage } = require('../common');
 const { test } = require('node:test');
 const assert = require('assert');
 
@@ -12,7 +12,7 @@ const assert = require('assert');
 if (process.stdout.isTTY)
   process.env.NODE_DISABLE_COLORS = '1';
 
-test('', { skip: !hasCrypto }, () => {
+test({ skip: !hasCrypto || !hasLocalStorage }, () => {
   // See https://github.com/nodejs/node/issues/10258
   {
     const date = new Date('2016');

test/parallel/test-webstorage.js

@@ -41,13 +41,16 @@ test('sessionStorage is not persisted', async () => {
   assert.strictEqual((await readdir(tmpdir.path)).length, 0);
 });
 
-test('localStorage emits a warning when used without --localstorage-file ', async () => {
+test('localStorage throws without --localstorage-file', async () => {
   const cp = await spawnPromisified(process.execPath, [
-    '-pe', 'localStorage.length',
+    '-e', 'localStorage',
   ]);
-  assert.strictEqual(cp.code, 0);
+  assert.strictEqual(cp.code, 1);
   assert.strictEqual(cp.signal, null);
-  assert.match(cp.stderr, /Warning: `--localstorage-file` was provided without a valid path/);
+  assert.match(
+    cp.stderr,
+    /Error \[ERR_WEBSTORAGE_MISSING_STORAGE_PATH\]: Cannot initialize local storage without a `--localstorage-file` path/,
+  );
 });
 
 test('localStorage is not persisted if it is unused', async () => {