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 });