From a1f59f6fb577577e9b4398812f99b506651a62ba Mon Sep 17 00:00:00 2001 From: Avocado Date: Fri, 11 Sep 2026 11:25:14 +0900 Subject: [PATCH] inspector: report an error when DOM storage is unavailable getDOMStorageItems() falls back to reading the live Storage object when its cached map is empty. When getWebStorage() cannot produce one, the fallback was skipped and the command still answered with Success and an empty entries array, so a frontend could not tell an empty store from a store that cannot be read. getWebStorage() returns nothing whenever globalThis.localStorage is missing or is not an object, which is the case for any process started without --localstorage-file. Return a ServerError in that case, matching how the other failure paths in the same command report problems. Reading the storage key in that same configuration aborted the process: localstorage_file is an empty string, and std::filesystem::absolute() throws on it under libstdc++ while returning the current directory under libc++. Node builds without exceptions, so the throw terminated the process. Use the error_code overloads of absolute() and weakly_canonical() and report the failure through DispatchResponse. Fixes: https://github.com/nodejs/node/issues/65895 Signed-off-by: Avocado --- src/inspector/dom_storage_agent.cc | 8 ++- src/inspector/storage_agent.cc | 17 ++++-- src/inspector/storage_agent.h | 6 +- ...test-inspector-dom-storage-unavailable.mjs | 58 +++++++++++++++++++ test/parallel/test-inspector-dom-storage.js | 6 ++ 5 files changed, 87 insertions(+), 8 deletions(-) create mode 100644 test/fixtures/test-inspector-dom-storage-unavailable.mjs diff --git a/src/inspector/dom_storage_agent.cc b/src/inspector/dom_storage_agent.cc index 3708d3b59975..caf7ca98f7d3 100644 --- a/src/inspector/dom_storage_agent.cc +++ b/src/inspector/dom_storage_agent.cc @@ -101,10 +101,12 @@ protocol::DispatchResponse DOMStorageAgent::getDOMStorageItems( std::optional storage_map_fallback; if (storage_map->empty()) { auto web_storage_obj = getWebStorage(is_local_storage); - if (web_storage_obj) { - storage_map_fallback = web_storage_obj.value()->GetAll(); - storage_map = &storage_map_fallback.value(); + if (!web_storage_obj) { + return protocol::DispatchResponse::ServerError( + "Could not read DOM storage items"); } + storage_map_fallback = web_storage_obj.value()->GetAll(); + storage_map = &storage_map_fallback.value(); } auto result = diff --git a/src/inspector/storage_agent.cc b/src/inspector/storage_agent.cc index 62fc2d9e858d..ac3cbbded98d 100644 --- a/src/inspector/storage_agent.cc +++ b/src/inspector/storage_agent.cc @@ -17,13 +17,22 @@ void StorageAgent::Wire(protocol::UberDispatcher* dispatcher) { DispatchResponse StorageAgent::getStorageKey( std::optional frameId, protocol::String* storageKey) { auto local_storage_file = env_->options()->localstorage_file; - *storageKey = node::url::FromFilePath(to_absolute_path(local_storage_file)); + auto absolute_path = to_absolute_path(local_storage_file); + if (!absolute_path) { + return protocol::DispatchResponse::ServerError( + "Could not resolve the storage key path"); + } + *storageKey = node::url::FromFilePath(*absolute_path); return protocol::DispatchResponse::Success(); } -std::string StorageAgent::to_absolute_path(const std::filesystem::path& input) { - std::filesystem::path abs = - std::filesystem::weakly_canonical(std::filesystem::absolute(input)); +std::optional StorageAgent::to_absolute_path( + const std::filesystem::path& input) { + std::error_code error; + std::filesystem::path abs = std::filesystem::absolute(input, error); + if (error) return std::nullopt; + abs = std::filesystem::weakly_canonical(abs, error); + if (error) return std::nullopt; return abs.generic_string(); } diff --git a/src/inspector/storage_agent.h b/src/inspector/storage_agent.h index 46680182672f..f2f5a59aceac 100644 --- a/src/inspector/storage_agent.h +++ b/src/inspector/storage_agent.h @@ -1,6 +1,9 @@ #ifndef SRC_INSPECTOR_STORAGE_AGENT_H_ #define SRC_INSPECTOR_STORAGE_AGENT_H_ +#include +#include +#include #include "env.h" #include "node/inspector/protocol/Storage.h" @@ -22,7 +25,8 @@ class StorageAgent : public protocol::Storage::Backend { StorageAgent& operator=(const StorageAgent&) = delete; private: - std::string to_absolute_path(const std::filesystem::path& input); + std::optional to_absolute_path( + const std::filesystem::path& input); std::unique_ptr frontend_; Environment* env_; }; diff --git a/test/fixtures/test-inspector-dom-storage-unavailable.mjs b/test/fixtures/test-inspector-dom-storage-unavailable.mjs new file mode 100644 index 000000000000..902086e8a483 --- /dev/null +++ b/test/fixtures/test-inspector-dom-storage-unavailable.mjs @@ -0,0 +1,58 @@ +import '../common/index.mjs'; +import assert from 'assert'; +import { Session } from 'node:inspector/promises'; + +// getDOMStorageItems only looks at isLocalStorage, so the storage key is not +// needed to address a store. Storage.getStorageKey is deliberately not used +// here: without --localstorage-file it has no path to resolve, and what it +// does then differs between platforms. +const storageKey = ''; + +// Without --localstorage-file, globalThis.localStorage is undefined, so the +// agent cannot read the store. Reading its items must report an error instead +// of answering successfully with an empty list, which is indistinguishable +// from a store that exists and happens to be empty. +{ + const session = new Session(); + await session.connect(); + await session.post('DOMStorage.enable'); + + await assert.rejects( + session.post('DOMStorage.getDOMStorageItems', { + storageId: { + isLocalStorage: true, + securityOrigin: '', + storageKey, + }, + }), + { + code: 'ERR_INSPECTOR_COMMAND', + message: /Could not read DOM storage items/, + }, + ); + + session.disconnect(); +} + +// sessionStorage is always backed by an in-memory store, so it stays readable +// and answers with an empty list until items are added. +{ + const session = new Session(); + await session.connect(); + await session.post('DOMStorage.enable'); + + const storageId = { isLocalStorage: false, securityOrigin: '', storageKey }; + + const empty = await session.post('DOMStorage.getDOMStorageItems', { + storageId, + }); + assert.deepStrictEqual(empty.entries, []); + + sessionStorage.setItem('key', 'value'); + const result = await session.post('DOMStorage.getDOMStorageItems', { + storageId, + }); + assert.deepStrictEqual(result.entries, [['key', 'value']]); + + session.disconnect(); +} diff --git a/test/parallel/test-inspector-dom-storage.js b/test/parallel/test-inspector-dom-storage.js index be3d8a0ed7c2..6feafd622003 100644 --- a/test/parallel/test-inspector-dom-storage.js +++ b/test/parallel/test-inspector-dom-storage.js @@ -14,3 +14,9 @@ spawnSyncAndExitWithoutError(process.execPath, [ '--localstorage-file=./localstorage.db', fixtures.path('test-inspector-dom-storage.mjs'), ], { cwd: tmpdir.path }); + +spawnSyncAndExitWithoutError(process.execPath, [ + '--inspect=0', + '--experimental-storage-inspection', + fixtures.path('test-inspector-dom-storage-unavailable.mjs'), +], { cwd: tmpdir.path });