Skip to content

Commit a4dee61

Browse files
authored
Revert "lib: throw from localStorage getter on missing storage path"
This reverts commit 4fbb1ab. PR-URL: #60750 Reviewed-By: René <contact.9a5d6388@renegade334.me.uk> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
1 parent 5316b58 commit a4dee61

File tree

5 files changed

+33
-35
lines changed

5 files changed

+33
-35
lines changed

lib/internal/webstorage.js

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
'use strict';
22
const {
33
ObjectDefineProperties,
4+
Proxy,
45
} = primordials;
56
const { getOptionValue } = require('internal/options');
6-
const { lazyDOMException } = require('internal/util');
77
const { kConstructorKey, Storage } = internalBinding('webstorage');
88
const { getValidatedPath } = require('internal/fs/utils');
99
const kInMemoryPath = ':memory:';
@@ -21,17 +21,34 @@ ObjectDefineProperties(module.exports, {
2121
enumerable: true,
2222
get() {
2323
if (lazyLocalStorage === undefined) {
24-
// For consistency with the web specification, throw from the accessor
25-
// if the local storage path is not provided.
2624
const location = getOptionValue('--localstorage-file');
25+
2726
if (location === '') {
28-
throw lazyDOMException(
29-
'Cannot initialize local storage without a `--localstorage-file` path',
30-
'SecurityError',
31-
);
32-
}
27+
let warningEmitted = false;
28+
const handler = {
29+
__proto__: null,
30+
get(target, prop) {
31+
if (!warningEmitted) {
32+
process.emitWarning('`--localstorage-file` was provided without a valid path');
33+
warningEmitted = true;
34+
}
35+
36+
return undefined;
37+
},
38+
set(target, prop, value) {
39+
if (!warningEmitted) {
40+
process.emitWarning('`--localstorage-file` was provided without a valid path');
41+
warningEmitted = true;
42+
}
3343

34-
lazyLocalStorage = new Storage(kConstructorKey, getValidatedPath(location));
44+
return false;
45+
},
46+
};
47+
48+
lazyLocalStorage = new Proxy({}, handler);
49+
} else {
50+
lazyLocalStorage = new Storage(kConstructorKey, getValidatedPath(location));
51+
}
3552
}
3653

3754
return lazyLocalStorage;

test/common/index.js

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -59,14 +59,6 @@ const hasSQLite = Boolean(process.versions.sqlite);
5959

6060
const hasQuic = hasCrypto && !!process.features.quic;
6161

62-
const hasLocalStorage = (() => {
63-
try {
64-
return hasSQLite && globalThis.localStorage !== undefined;
65-
} catch {
66-
return false;
67-
}
68-
})();
69-
7062
/**
7163
* Parse test metadata from the specified file.
7264
* @param {string} filename - The name of the file to parse.
@@ -359,6 +351,7 @@ const knownGlobals = new Set([
359351
'CompressionStream',
360352
'DecompressionStream',
361353
'Storage',
354+
'localStorage',
362355
'sessionStorage',
363356
].forEach((i) => {
364357
if (globalThis[i] !== undefined) {
@@ -373,10 +366,6 @@ if (hasCrypto) {
373366
knownGlobals.add(globalThis.SubtleCrypto);
374367
}
375368

376-
if (hasLocalStorage) {
377-
knownGlobals.add(globalThis.localStorage);
378-
}
379-
380369
const { Worker } = require('node:worker_threads');
381370
knownGlobals.add(Worker);
382371

@@ -401,11 +390,6 @@ if (process.env.NODE_TEST_KNOWN_GLOBALS !== '0') {
401390
if (val === 'crypto' && !hasCrypto) {
402391
continue;
403392
}
404-
// globalThis.localStorage is a getter that throws if Node.js was
405-
// executed without a --localstorage-file path.
406-
if (val === 'localStorage' && !hasLocalStorage) {
407-
continue;
408-
}
409393
if (!knownGlobals.has(globalThis[val])) {
410394
leaked.push(val);
411395
}
@@ -956,7 +940,6 @@ const common = {
956940
hasQuic,
957941
hasInspector,
958942
hasSQLite,
959-
hasLocalStorage,
960943
invalidArgTypeHelper,
961944
isAlive,
962945
isASan,

test/common/index.mjs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ const {
1919
hasQuic,
2020
hasInspector,
2121
hasSQLite,
22-
hasLocalStorage,
2322
hasIntl,
2423
hasIPv6,
2524
isAIX,
@@ -73,7 +72,6 @@ export {
7372
hasQuic,
7473
hasInspector,
7574
hasSQLite,
76-
hasLocalStorage,
7775
hasIntl,
7876
hasIPv6,
7977
isAIX,

test/parallel/test-assert-checktag.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
'use strict';
2-
const { hasCrypto, hasLocalStorage } = require('../common');
2+
const { hasCrypto } = require('../common');
33
const { test } = require('node:test');
44
const assert = require('assert');
55

@@ -12,7 +12,7 @@ const assert = require('assert');
1212
if (process.stdout.isTTY)
1313
process.env.NODE_DISABLE_COLORS = '1';
1414

15-
test({ skip: !hasCrypto || !hasLocalStorage }, () => {
15+
test('', { skip: !hasCrypto }, () => {
1616
// See https://github.com/nodejs/node/issues/10258
1717
{
1818
const date = new Date('2016');

test/parallel/test-webstorage.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,13 @@ test('sessionStorage is not persisted', async () => {
4141
assert.strictEqual((await readdir(tmpdir.path)).length, 0);
4242
});
4343

44-
test('localStorage throws without --localstorage-file', async () => {
44+
test('localStorage emits a warning when used without --localstorage-file ', async () => {
4545
const cp = await spawnPromisified(process.execPath, [
46-
'-e', 'localStorage',
46+
'-pe', 'localStorage.length',
4747
]);
48-
assert.strictEqual(cp.code, 1);
48+
assert.strictEqual(cp.code, 0);
4949
assert.strictEqual(cp.signal, null);
50-
assert.match(cp.stderr, /SecurityError:/);
50+
assert.match(cp.stderr, /Warning: `--localstorage-file` was provided without a valid path/);
5151
});
5252

5353
test('localStorage is not persisted if it is unused', async () => {

0 commit comments

Comments
 (0)