Skip to content

Commit 4fbb1ab

Browse files
Renegade334aduh95
authored andcommitted
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

5 files changed

Lines changed: 35 additions & 33 deletions

File tree

‎lib/internal/webstorage.js‎

Lines 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
constkInMemoryPath=':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
constlocation=getOptionValue('--localstorage-file');
25-
2627
if(location===''){
27-
letwarningEmitted=false;
28-
consthandler={
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-
returnundefined;
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-
returnfalse;
45-
},
46-
};
47-
48-
lazyLocalStorage=newProxy({},handler);
49-
}else{
50-
lazyLocalStorage=newStorage(kConstructorKey,getValidatedPath(location));
28+
throwlazyDOMException(
29+
'Cannot initialize local storage without a `--localstorage-file` path',
30+
'SecurityError',
31+
);
5132
}
33+
34+
lazyLocalStorage=newStorage(kConstructorKey,getValidatedPath(location));
5235
}
5336

5437
returnlazyLocalStorage;

‎test/common/index.js‎

Lines 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
consthasQuic=hasCrypto&&!!process.features.quic;
6161

62+
consthasLocalStorage=(()=>{
63+
try{
64+
returnhasSQLite&&globalThis.localStorage!==undefined;
65+
}catch{
66+
returnfalse;
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,

‎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,
@@ -71,6 +72,7 @@ export {
7172
hasQuic,
7273
hasInspector,
7374
hasSQLite,
75+
hasLocalStorage,
7476
hasIntl,
7577
hasIPv6,
7678
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
constassert=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
constdate=newDate('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((awaitreaddir(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
constcp=awaitspawnPromisified(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`wasprovidedwithoutavalidpath/);
50+
assert.match(cp.stderr,/SecurityError:/);
5151
});
5252

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

0 commit comments

Comments
 (0)