Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit fa70327

Browse filesBrowse files
mcollinamcollina
authored
lib: return undefined for localStorage without file
Make localStorage return undefined and emit a warning when --localstorage-file is not provided. Mark the property as non-enumerable to avoid breaking {...globalThis}. Fixes: #60303 PR-URL: #61333 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
1 parent 7698480 commit fa70327
Copy full SHA for fa70327

5 files changed

+58-22Lines changed: 58 additions & 22 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎lib/internal/process/pre_execution.js‎

Copy file name to clipboardExpand all lines: lib/internal/process/pre_execution.js
+19-1Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -404,8 +404,26 @@ function setupWebStorage() {
404404

405405
// https://html.spec.whatwg.org/multipage/webstorage.html#webstorage
406406
exposeLazyInterfaces(globalThis, 'internal/webstorage', ['Storage']);
407+
408+
// localStorage is non-enumerable when --localstorage-file is not provided
409+
// to avoid breaking {...globalThis} operations.
410+
const localStorageFile = getOptionValue('--localstorage-file');
411+
let lazyLocalStorage;
412+
ObjectDefineProperty(globalThis, 'localStorage', {
413+
__proto__: null,
414+
enumerable: localStorageFile !== '',
415+
configurable: true,
416+
get() {
417+
lazyLocalStorage ??= require('internal/webstorage').localStorage;
418+
return lazyLocalStorage;
419+
},
420+
set(value) {
421+
lazyLocalStorage = value;
422+
},
423+
});
424+
407425
defineReplaceableLazyAttribute(globalThis, 'internal/webstorage', [
408-
'localStorage', 'sessionStorage',
426+
'sessionStorage',
409427
]);
410428
}
411429

Collapse file

‎lib/internal/webstorage.js‎

Copy file name to clipboardExpand all lines: lib/internal/webstorage.js
+16-11Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ const {
33
ObjectDefineProperties,
44
} = primordials;
55
const { getOptionValue } = require('internal/options');
6-
const { lazyDOMException } = require('internal/util');
76
const { kConstructorKey, Storage } = internalBinding('webstorage');
87
const { getValidatedPath } = require('internal/fs/utils');
98
const kInMemoryPath = ':memory:';
@@ -12,26 +11,32 @@ module.exports = { Storage };
1211

1312
let lazyLocalStorage;
1413
let lazySessionStorage;
14+
let localStorageWarned = false;
15+
16+
// Check at load time if localStorage file is provided to determine enumerability.
17+
// If not provided, localStorage is non-enumerable to avoid breaking {...globalThis}.
18+
const localStorageLocation = getOptionValue('--localstorage-file');
1519

1620
ObjectDefineProperties(module.exports, {
1721
__proto__: null,
1822
localStorage: {
1923
__proto__: null,
2024
configurable: true,
21-
enumerable: true,
25+
enumerable: localStorageLocation !== '',
2226
get() {
2327
if (lazyLocalStorage === undefined) {
24-
// For consistency with the web specification, throw from the accessor
25-
// if the local storage path is not provided.
26-
const location = getOptionValue('--localstorage-file');
27-
if (location === '') {
28-
throw lazyDOMException(
29-
'Cannot initialize local storage without a `--localstorage-file` path',
30-
'SecurityError',
31-
);
28+
if (localStorageLocation === '') {
29+
if (!localStorageWarned) {
30+
localStorageWarned = true;
31+
process.emitWarning(
32+
'localStorage is not available because --localstorage-file was not provided.',
33+
'ExperimentalWarning',
34+
);
35+
}
36+
return undefined;
3237
}
3338

34-
lazyLocalStorage = new Storage(kConstructorKey, getValidatedPath(location));
39+
lazyLocalStorage = new Storage(kConstructorKey, getValidatedPath(localStorageLocation));
3540
}
3641

3742
return lazyLocalStorage;
Collapse file

‎test/common/index.js‎

Copy file name to clipboardExpand all lines: test/common/index.js
+4-5Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,11 +71,10 @@ const hasSQLite = Boolean(process.versions.sqlite);
7171
const hasQuic = hasCrypto && !!process.features.quic;
7272

7373
const hasLocalStorage = (() => {
74-
try {
75-
return hasSQLite && globalThis.localStorage !== undefined;
76-
} catch {
77-
return false;
78-
}
74+
// Check enumerable property to avoid triggering the getter which emits a warning.
75+
// localStorage is enumerable only when --localstorage-file is provided.
76+
const desc = Object.getOwnPropertyDescriptor(globalThis, 'localStorage');
77+
return hasSQLite && desc?.enumerable === true;
7978
})();
8079

8180
/**
Collapse file

‎test/parallel/test-global.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-global.js
+6-1Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,12 @@ for (const moduleName of builtinModules) {
6161
'navigator',
6262
];
6363
if (common.hasSQLite) {
64-
expected.push('localStorage', 'sessionStorage');
64+
// sessionStorage is always enumerable when SQLite is available.
65+
// localStorage is only enumerable when --localstorage-file is provided.
66+
expected.push('sessionStorage');
67+
if (common.hasLocalStorage) {
68+
expected.push('localStorage');
69+
}
6570
}
6671
assert.deepStrictEqual(new Set(Object.keys(globalThis)), new Set(expected));
6772
expected.forEach((value) => {
Collapse file

‎test/parallel/test-webstorage.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-webstorage.js
+13-4Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,22 @@ 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 returns undefined and warns without --localstorage-file', async () => {
4545
const cp = await spawnPromisified(process.execPath, [
46-
'-e', 'localStorage',
46+
'-pe', 'localStorage',
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.stdout, /undefined/);
51+
assert.match(cp.stderr, /ExperimentalWarning:.*localStorage is not available/);
52+
});
53+
54+
test('localStorage is not enumerable without --localstorage-file', async () => {
55+
const cp = await spawnPromisified(process.execPath, [
56+
'-pe', 'Object.keys(globalThis).includes("localStorage")',
57+
]);
58+
assert.strictEqual(cp.code, 0);
59+
assert.match(cp.stdout, /false/);
5160
});
5261

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

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.