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 4fbb1ab

Browse filesBrowse files
Renegade334aduh95
Renegade334
authored and
aduh95
committed
lib: throw from localStorage getter on missing storage path
PR-URL: #60351 Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
1 parent 74719da commit 4fbb1ab
Copy full SHA for 4fbb1ab

File tree

Expand file treeCollapse file tree

5 files changed

+35
-33
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

5 files changed

+35
-33
lines changed
Open diff view settings
Collapse file

‎lib/internal/webstorage.js‎

Copy file name to clipboardExpand all lines: lib/internal/webstorage.js
+9-26Lines changed: 9 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
'use strict';
22
const {
33
ObjectDefineProperties,
4-
Proxy,
54
} = primordials;
65
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,34 +21,17 @@ 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.
2426
const location = getOptionValue('--localstorage-file');
25-
2627
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));
28+
throw lazyDOMException(
29+
'Cannot initialize local storage without a `--localstorage-file` path',
30+
'SecurityError',
31+
);
5132
}
33+
34+
lazyLocalStorage = new Storage(kConstructorKey, getValidatedPath(location));
5235
}
5336

5437
return lazyLocalStorage;
Collapse file

‎test/common/index.js‎

Copy file name to clipboardExpand all lines: test/common/index.js
+18-1Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,14 @@ 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+
6270
/**
6371
* Parse test metadata from the specified file.
6472
* @param {string} filename - The name of the file to parse.
@@ -350,7 +358,6 @@ const knownGlobals = new Set([
350358
'CompressionStream',
351359
'DecompressionStream',
352360
'Storage',
353-
'localStorage',
354361
'sessionStorage',
355362
].forEach((i) => {
356363
if (globalThis[i] !== undefined) {
@@ -365,6 +372,10 @@ if (hasCrypto) {
365372
knownGlobals.add(globalThis.SubtleCrypto);
366373
}
367374

375+
if (hasLocalStorage) {
376+
knownGlobals.add(globalThis.localStorage);
377+
}
378+
368379
const { Worker } = require('node:worker_threads');
369380
knownGlobals.add(Worker);
370381

@@ -389,6 +400,11 @@ if (process.env.NODE_TEST_KNOWN_GLOBALS !== '0') {
389400
if (val === 'crypto' && !hasCrypto) {
390401
continue;
391402
}
403+
// globalThis.localStorage is a getter that throws if Node.js was
404+
// executed without a --localstorage-file path.
405+
if (val === 'localStorage' && !hasLocalStorage) {
406+
continue;
407+
}
392408
if (!knownGlobals.has(globalThis[val])) {
393409
leaked.push(val);
394410
}
@@ -933,6 +949,7 @@ const common = {
933949
hasQuic,
934950
hasInspector,
935951
hasSQLite,
952+
hasLocalStorage,
936953
invalidArgTypeHelper,
937954
isAlive,
938955
isASan,
Collapse file

‎test/common/index.mjs‎

Copy file name to clipboardExpand all lines: test/common/index.mjs
+2Lines 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,
@@ -71,6 +72,7 @@ export {
7172
hasQuic,
7273
hasInspector,
7374
hasSQLite,
75+
hasLocalStorage,
7476
hasIntl,
7577
hasIPv6,
7678
isAIX,
Collapse file

‎test/parallel/test-assert-checktag.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-assert-checktag.js
+2-2Lines 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');
Collapse file

‎test/parallel/test-webstorage.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-webstorage.js
+4-4Lines 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 emits a warning when used without --localstorage-file ', async () => {
44+
test('localStorage throws without --localstorage-file', async () => {
4545
const cp = await spawnPromisified(process.execPath, [
46-
'-pe', 'localStorage.length',
46+
'-e', 'localStorage',
4747
]);
48-
assert.strictEqual(cp.code, 0);
48+
assert.strictEqual(cp.code, 1);
4949
assert.strictEqual(cp.signal, null);
50-
assert.match(cp.stderr, /Warning: `--localstorage-file` was provided without a valid path/);
50+
assert.match(cp.stderr, /SecurityError:/);
5151
});
5252

5353
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.