Repository: nodejs/node
Pull request: lib: throw from localStorage getter on missing storage path

> #### lib: throw from localStorage getter on missing storage path
> The behavior of the global `localStorage` variable has changed, in the specific case when webstorage is enabled but the `--localstorage-file` option is not provided. In Node.js v25.0.0 and v25.1.0, this would cause `localStorage` to be initialized as an empty object. In v25.2.0, an exception is now thrown when the `localStorage` variable is accessed, which is more consistent with the web standard.

Fixes #60303.

The Storage specification states that the global `localStorage` getter should throw an exception if the storage environment cannot be obtained. Accordingly, a common way of testing for local storage capability in userland is along these lines:

```js
function localStorageIsAvailable() {
  try {
    return typeof localStorage !== 'undefined';
  } catch {
    return false;
  }
}
```

This tests both for the presence of the global and whether the getter succeeds, which also covers scenarios where the storage API is not exposed at all.

Node.js's flagged implementation of `localStorage` also used to follow this behaviour in v24 and earlier. If local storage could not be initialised due to the absence of the `--localstorage-file` command-line argument, the global getter would throw at the point of the variable being accessed. The above test therefore also worked well in v24: it would identify when the variable didn't exist, and would also detect when an accessor exception was thrown.

When webstorage was enabled by default for v25 (#57666), this behaviour was changed: in the absence of `--localstorage-file`, the global getter no longer throws, but instead returns an empty Proxy which returns `undefined` on all property access after emitting a warning. The motivation behind this change of approach was to avoid the test suite throwing during its global tests. However, a throw-on-access global getter already exists (`crypto`), and therefore there already exist mechanisms for getting around this.

The behaviour as it stands is problematic. Exposing `localStorage` as an empty object with no observable properties is an unexpected pattern, is inconsistent with the web, is non-compliant with the spec, and there is no longer a trivial and web-consistent way to observe `localStorage` to work out whether or not it's actually available.

I am therefore proposing, as an out-of-cycle breaking change, to revert to web-compatible behaviour and throw from the global getter if `localStorage` cannot be initialised due to the missing command-line argument. **Edit:** just realised that webstorage wasn't stabilised, so this wouldn't need TSC approval to land.

cc: @nodejs/web-standards

Changed files:
filename: lib/internal/webstorage.js
patch: @@ -1,9 +1,9 @@
 'use strict';
 const {
   ObjectDefineProperties,
-  Proxy,
 } = primordials;
 const { getOptionValue } = require('internal/options');
+const { lazyDOMException } = require('internal/util');
 const { kConstructorKey, Storage } = internalBinding('webstorage');
 const { getValidatedPath } = require('internal/fs/utils');
 const kInMemoryPath = ':memory:';
@@ -21,34 +21,17 @@ 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 lazyDOMException(
+            'Cannot initialize local storage without a `--localstorage-file` path',
+            'SecurityError',
+          );
         }
+
+        lazyLocalStorage = new Storage(kConstructorKey, getValidatedPath(location));
       }
 
       return lazyLocalStorage;

filename: test/common/index.js
patch: @@ -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,7 +358,6 @@ const knownGlobals = new Set([
  'CompressionStream',
  'DecompressionStream',
  'Storage',
- 'localStorage',
  'sessionStorage',
 ].forEach((i) => {
   if (globalThis[i] !== undefined) {
@@ -365,6 +372,10 @@ if (hasCrypto) {
   knownGlobals.add(globalThis.SubtleCrypto);
 }
 
+if (hasLocalStorage) {
+  knownGlobals.add(globalThis.localStorage);
+}
+
 const { Worker } = require('node:worker_threads');
 knownGlobals.add(Worker);
 
@@ -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,

filename: test/common/index.mjs
patch: @@ -19,6 +19,7 @@ const {
   hasQuic,
   hasInspector,
   hasSQLite,
+  hasLocalStorage,
   hasIntl,
   hasIPv6,
   isAIX,
@@ -71,6 +72,7 @@ export {
   hasQuic,
   hasInspector,
   hasSQLite,
+  hasLocalStorage,
   hasIntl,
   hasIPv6,
   isAIX,

filename: test/parallel/test-assert-checktag.js
patch: @@ -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');

filename: test/parallel/test-webstorage.js
patch: @@ -41,13 +41,13 @@ 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, /SecurityError:/);
 });
 
 test('localStorage is not persisted if it is unused', async () => {