GeezerVison is showing github.com — lib: throw from localStorage getter on missing storage path · nodejs/node@4fbb1ab
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: lib/internal/webstorage.js +9-26Lines changed: 9 additions & 26 deletions | Original file line number | Diff line number | Diff line change |
|---|
| 1 | 1 | 'use strict'; | 2 | 2 | const { | 3 | 3 | ObjectDefineProperties, | 4 | | - Proxy, | 5 | 4 | } = primordials; | 6 | 5 | const { getOptionValue } = require('internal/options'); | | 6 | +const { lazyDOMException } = require('internal/util'); | 7 | 7 | const { kConstructorKey, Storage } = internalBinding('webstorage'); | 8 | 8 | const { getValidatedPath } = require('internal/fs/utils'); | 9 | 9 | const kInMemoryPath = ':memory:'; | @@ -21,34 +21,17 @@ ObjectDefineProperties(module.exports, { | 21 | 21 | enumerable: true, | 22 | 22 | get() { | 23 | 23 | if (lazyLocalStorage === undefined) { | | 24 | + // For consistency with the web specification, throw from the accessor | | 25 | + // if the local storage path is not provided. | 24 | 26 | const location = getOptionValue('--localstorage-file'); | 25 | | - | 26 | 27 | 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 | + ); | 51 | 32 | } | | 33 | + | | 34 | + lazyLocalStorage = new Storage(kConstructorKey, getValidatedPath(location)); | 52 | 35 | } | 53 | 36 |
| 54 | 37 | return lazyLocalStorage; | |
Collapse file Copy file name to clipboardExpand all lines: test/common/index.js +18-1Lines changed: 18 additions & 1 deletion | Original file line number | Diff line number | Diff line change |
|---|
@@ -59,6 +59,14 @@ const hasSQLite = Boolean(process.versions.sqlite); | 59 | 59 |
| 60 | 60 | const hasQuic = hasCrypto && !!process.features.quic; | 61 | 61 |
| | 62 | +const hasLocalStorage = (() => { | | 63 | + try { | | 64 | + return hasSQLite && globalThis.localStorage !== undefined; | | 65 | + } catch { | | 66 | + return false; | | 67 | + } | | 68 | +})(); | | 69 | + | 62 | 70 | /** | 63 | 71 | * Parse test metadata from the specified file. | 64 | 72 | * @param {string} filename - The name of the file to parse. | @@ -350,7 +358,6 @@ const knownGlobals = new Set([ | 350 | 358 | 'CompressionStream', | 351 | 359 | 'DecompressionStream', | 352 | 360 | 'Storage', | 353 | | - 'localStorage', | 354 | 361 | 'sessionStorage', | 355 | 362 | ].forEach((i) => { | 356 | 363 | if (globalThis[i] !== undefined) { | @@ -365,6 +372,10 @@ if (hasCrypto) { | 365 | 372 | knownGlobals.add(globalThis.SubtleCrypto); | 366 | 373 | } | 367 | 374 |
| | 375 | +if (hasLocalStorage) { | | 376 | + knownGlobals.add(globalThis.localStorage); | | 377 | +} | | 378 | + | 368 | 379 | const { Worker } = require('node:worker_threads'); | 369 | 380 | knownGlobals.add(Worker); | 370 | 381 |
| @@ -389,6 +400,11 @@ if (process.env.NODE_TEST_KNOWN_GLOBALS !== '0') { | 389 | 400 | if (val === 'crypto' && !hasCrypto) { | 390 | 401 | continue; | 391 | 402 | } | | 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 | + } | 392 | 408 | if (!knownGlobals.has(globalThis[val])) { | 393 | 409 | leaked.push(val); | 394 | 410 | } | @@ -933,6 +949,7 @@ const common = { | 933 | 949 | hasQuic, | 934 | 950 | hasInspector, | 935 | 951 | hasSQLite, | | 952 | + hasLocalStorage, | 936 | 953 | invalidArgTypeHelper, | 937 | 954 | isAlive, | 938 | 955 | isASan, | |
Collapse file Copy file name to clipboardExpand all lines: test/common/index.mjs +2Lines changed: 2 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -19,6 +19,7 @@ const { | 19 | 19 | hasQuic, | 20 | 20 | hasInspector, | 21 | 21 | hasSQLite, | | 22 | + hasLocalStorage, | 22 | 23 | hasIntl, | 23 | 24 | hasIPv6, | 24 | 25 | isAIX, | @@ -71,6 +72,7 @@ export { | 71 | 72 | hasQuic, | 72 | 73 | hasInspector, | 73 | 74 | hasSQLite, | | 75 | + hasLocalStorage, | 74 | 76 | hasIntl, | 75 | 77 | hasIPv6, | 76 | 78 | isAIX, | |
Collapse file Copy file name to clipboardExpand all lines: test/parallel/test-assert-checktag.js +2-2Lines changed: 2 additions & 2 deletions | Original file line number | Diff line number | Diff line change |
|---|
| 1 | 1 | 'use strict'; | 2 | | -const { hasCrypto } = require('../common'); | | 2 | +const { hasCrypto, hasLocalStorage } = require('../common'); | 3 | 3 | const { test } = require('node:test'); | 4 | 4 | const assert = require('assert'); | 5 | 5 |
| @@ -12,7 +12,7 @@ const assert = require('assert'); | 12 | 12 | if (process.stdout.isTTY) | 13 | 13 | process.env.NODE_DISABLE_COLORS = '1'; | 14 | 14 |
| 15 | | -test('', { skip: !hasCrypto }, () => { | | 15 | +test({ skip: !hasCrypto || !hasLocalStorage }, () => { | 16 | 16 | // See https://github.com/nodejs/node/issues/10258 | 17 | 17 | { | 18 | 18 | const date = new Date('2016'); | |
Collapse file Copy file name to clipboardExpand all lines: test/parallel/test-webstorage.js +4-4Lines changed: 4 additions & 4 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -41,13 +41,13 @@ test('sessionStorage is not persisted', async () => { | 41 | 41 | assert.strictEqual((await readdir(tmpdir.path)).length, 0); | 42 | 42 | }); | 43 | 43 |
| 44 | | -test('localStorage emits a warning when used without --localstorage-file ', async () => { | | 44 | +test('localStorage throws without --localstorage-file', async () => { | 45 | 45 | const cp = await spawnPromisified(process.execPath, [ | 46 | | - '-pe', 'localStorage.length', | | 46 | + '-e', 'localStorage', | 47 | 47 | ]); | 48 | | - assert.strictEqual(cp.code, 0); | | 48 | + assert.strictEqual(cp.code, 1); | 49 | 49 | 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:/); | 51 | 51 | }); | 52 | 52 |
| 53 | 53 | test('localStorage is not persisted if it is unused', async () => { | |
|