Skip to content

Commit c57a40c

Browse files
Renegade334RafaelGSS
authored andcommitted
lib: do not provide an empty Proxy from localStorage getter
PR-URL: #60776 Refs: #60303 Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
1 parent 9149ba4 commit c57a40c

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
@@ -70,6 +70,9 @@ const hasSQLite = Boolean(process.versions.sqlite);
7070

7171
const hasQuic = hasCrypto && !!process.features.quic;
7272

73+
const hasLocalStorage = hasSQLite &&
74+
process.execArgv.find((arg) => arg.startsWith('--localstorage-file=')) !== undefined;
75+
7376
/**
7477
* Parse test metadata from the specified file.
7578
* @param {string} filename - The name of the file to parse.
@@ -362,7 +365,6 @@ const knownGlobals = new Set([
362365
'CompressionStream',
363366
'DecompressionStream',
364367
'Storage',
365-
'localStorage',
366368
'sessionStorage',
367369
].forEach((i) => {
368370
if (globalThis[i] !== undefined) {
@@ -377,6 +379,10 @@ if (hasCrypto) {
377379
knownGlobals.add(globalThis.SubtleCrypto);
378380
}
379381

382+
if (hasLocalStorage) {
383+
knownGlobals.add(globalThis.localStorage);
384+
}
385+
380386
const { Worker } = require('node:worker_threads');
381387
knownGlobals.add(Worker);
382388

@@ -401,6 +407,11 @@ if (process.env.NODE_TEST_KNOWN_GLOBALS !== '0') {
401407
if (val === 'crypto' && !hasCrypto) {
402408
continue;
403409
}
410+
// globalThis.localStorage is a getter that warns if Node.js was
411+
// executed without a --localstorage-file path.
412+
if (val === 'localStorage' && !hasLocalStorage) {
413+
continue;
414+
}
404415
if (!knownGlobals.has(globalThis[val])) {
405416
leaked.push(val);
406417
}
@@ -952,6 +963,7 @@ const common = {
952963
hasQuic,
953964
hasInspector,
954965
hasSQLite,
966+
hasLocalStorage,
955967
invalidArgTypeHelper,
956968
isAlive,
957969
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)