Skip to content

Commit f86755a

Browse files
committed
lib: do not provide an empty Proxy from localStorage getter
1 parent fc996fd commit f86755a

File tree

5 files changed

+28
-32
lines changed

5 files changed

+28
-32
lines changed

lib/internal/webstorage.js

Lines changed: 8 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
'use strict';
22
const {
33
ObjectDefineProperties,
4-
Proxy,
54
} = primordials;
65
const { getOptionValue } = require('internal/options');
76
const { kConstructorKey, Storage } = internalBinding('webstorage');
@@ -13,42 +12,24 @@ module.exports = { Storage };
1312
let lazyLocalStorage;
1413
let lazySessionStorage;
1514

15+
let localStorageWarningEmitted = false;
16+
1617
ObjectDefineProperties(module.exports, {
1718
__proto__: null,
1819
localStorage: {
1920
__proto__: null,
2021
configurable: true,
2122
enumerable: true,
2223
get() {
23-
if (lazyLocalStorage === undefined) {
24+
if (lazyLocalStorage === undefined && !localStorageWarningEmitted) {
2425
const location = getOptionValue('--localstorage-file');
25-
2626
if (location === '') {
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-
}
43-
44-
return false;
45-
},
46-
};
47-
48-
lazyLocalStorage = new Proxy({}, handler);
49-
} else {
50-
lazyLocalStorage = new Storage(kConstructorKey, getValidatedPath(location));
27+
process.emitWarning('Cannot initialize local storage without a `--localstorage-file` path');
28+
localStorageWarningEmitted = true;
29+
return undefined;
5130
}
31+
32+
lazyLocalStorage = new Storage(kConstructorKey, getValidatedPath(location));
5233
}
5334

5435
return lazyLocalStorage;

test/common/index.js

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,9 @@ const hasSQLite = Boolean(process.versions.sqlite);
5959

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

62+
const hasLocalStorage = hasSQLite &&
63+
process.execArgv.find((arg) => arg.startsWith('--localstorage-file=')) !== undefined;
64+
6265
/**
6366
* Parse test metadata from the specified file.
6467
* @param {string} filename - The name of the file to parse.
@@ -351,7 +354,6 @@ const knownGlobals = new Set([
351354
'CompressionStream',
352355
'DecompressionStream',
353356
'Storage',
354-
'localStorage',
355357
'sessionStorage',
356358
].forEach((i) => {
357359
if (globalThis[i] !== undefined) {
@@ -366,6 +368,10 @@ if (hasCrypto) {
366368
knownGlobals.add(globalThis.SubtleCrypto);
367369
}
368370

371+
if (hasLocalStorage) {
372+
knownGlobals.add(globalThis.localStorage);
373+
}
374+
369375
const { Worker } = require('node:worker_threads');
370376
knownGlobals.add(Worker);
371377

@@ -390,6 +396,11 @@ if (process.env.NODE_TEST_KNOWN_GLOBALS !== '0') {
390396
if (val === 'crypto' && !hasCrypto) {
391397
continue;
392398
}
399+
// globalThis.localStorage is a getter that warns if Node.js was
400+
// executed without a --localstorage-file path.
401+
if (val === 'localStorage' && !hasLocalStorage) {
402+
continue;
403+
}
393404
if (!knownGlobals.has(globalThis[val])) {
394405
leaked.push(val);
395406
}
@@ -940,6 +951,7 @@ const common = {
940951
hasQuic,
941952
hasInspector,
942953
hasSQLite,
954+
hasLocalStorage,
943955
invalidArgTypeHelper,
944956
isAlive,
945957
isASan,

test/common/index.mjs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ const {
1919
hasQuic,
2020
hasInspector,
2121
hasSQLite,
22+
hasLocalStorage,
2223
hasIntl,
2324
hasIPv6,
2425
isAIX,
@@ -72,6 +73,7 @@ export {
7273
hasQuic,
7374
hasInspector,
7475
hasSQLite,
76+
hasLocalStorage,
7577
hasIntl,
7678
hasIPv6,
7779
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 } = require('../common');
2+
const { hasCrypto, hasLocalStorage } = 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 }, () => {
15+
test({ skip: !hasCrypto || !hasLocalStorage }, () => {
1616
// See https://github.com/nodejs/node/issues/10258
1717
{
1818
const date = new Date('2016');

test/parallel/test-webstorage.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,12 @@ test('sessionStorage is not persisted', async () => {
4343

4444
test('localStorage emits a warning when used without --localstorage-file ', async () => {
4545
const cp = await spawnPromisified(process.execPath, [
46-
'-pe', 'localStorage.length',
46+
'-pe', 'localStorage',
4747
]);
4848
assert.strictEqual(cp.code, 0);
4949
assert.strictEqual(cp.signal, null);
50-
assert.match(cp.stderr, /Warning: `--localstorage-file` was provided without a valid path/);
50+
assert.match(cp.stdout, /undefined/);
51+
assert.match(cp.stderr, /Warning: Cannot initialize local storage without a `--localstorage-file` path/);
5152
});
5253

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

0 commit comments

Comments
 (0)