Skip to content

Commit 65f3e9e

Browse files
committed
fix: handle non-string values in SecureStorage changes gracefully
1 parent 82b41d1 commit 65f3e9e

File tree

4 files changed

+40
-10
lines changed

4 files changed

+40
-10
lines changed

src/providers/AbstractStorage.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ export default abstract class AbstractStorage<T extends StorageState> implements
221221

222222
return await callWithPromise<T[K] | undefined>(resolve => {
223223
this.storage.get(fullKey, result => {
224-
resolve(result[fullKey]);
224+
resolve(result[fullKey] as T[K] | undefined);
225225
});
226226
});
227227
}
@@ -278,12 +278,14 @@ export default abstract class AbstractStorage<T extends StorageState> implements
278278
protected async triggerChange<P extends T>(key: string, changes: StorageChange, options: StorageWatchOptions<P>) {
279279
const {newValue, oldValue} = changes;
280280

281-
const originalKey = this.getOriginalKey(key);
281+
const originalKey = this.getOriginalKey(key) as keyof P;
282+
const nextValue = newValue as P[keyof P] | undefined;
283+
const prevValue = oldValue as P[keyof P] | undefined;
282284

283285
if (typeof options === "function") {
284-
options(newValue, oldValue, originalKey);
286+
options(nextValue, prevValue, originalKey);
285287
} else if (options[originalKey]) {
286-
options[originalKey]?.(newValue, oldValue);
288+
options[originalKey]?.(nextValue, prevValue);
287289
}
288290
}
289291

src/providers/SecureStorage.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,32 @@ describe("watch method", () => {
207207
expect(globalCallback).toHaveBeenCalledWith(80, 50, "volume");
208208
expect(globalCallback).toHaveBeenCalledWith("dark", "light", "theme");
209209
});
210+
211+
test("ignores non-string storage change values without logging watch errors", async () => {
212+
const keyCallback = jest.fn();
213+
const globalCallback = jest.fn();
214+
const consoleErrorSpy = jest.spyOn(console, "error").mockImplementation(() => {});
215+
216+
try {
217+
securedStorage.watch({theme: keyCallback});
218+
securedStorage.watch(globalCallback);
219+
220+
global.simulateStorageChange({
221+
storage: securedStorage,
222+
key: "theme",
223+
oldValue: null,
224+
newValue: {theme: "dark"},
225+
});
226+
227+
await new Promise(resolve => setTimeout(resolve));
228+
229+
expect(keyCallback).toHaveBeenCalledWith(undefined, undefined);
230+
expect(globalCallback).toHaveBeenCalledWith(undefined, undefined, "theme");
231+
expect(consoleErrorSpy).not.toHaveBeenCalled();
232+
} finally {
233+
consoleErrorSpy.mockRestore();
234+
}
235+
});
210236
});
211237

212238
// Static factory methods tests migrated from AbstractStorage.static.test.ts

src/providers/SecureStorage.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,8 @@ export default class SecureStorage<T extends StorageState> extends AbstractStora
110110
changes: StorageChange,
111111
options: StorageWatchOptions<P>
112112
): Promise<void> {
113-
const newValue = changes.newValue !== undefined ? await this.decrypt(changes.newValue) : undefined;
114-
const oldValue = changes.oldValue !== undefined ? await this.decrypt(changes.oldValue) : undefined;
113+
const newValue = typeof changes.newValue === "string" ? await this.decrypt(changes.newValue) : undefined;
114+
const oldValue = typeof changes.oldValue === "string" ? await this.decrypt(changes.oldValue) : undefined;
115115

116116
await this.triggerChange(key, {newValue, oldValue}, options);
117117
}

tests/jest.storage.setup.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -131,10 +131,12 @@ const createStorageArea = (): chrome.storage.StorageArea => {
131131
return area;
132132
};
133133

134-
chrome.storage.local = createStorageArea() as chrome.storage.LocalStorageArea;
135-
chrome.storage.sync = createStorageArea() as chrome.storage.SyncStorageArea;
136-
chrome.storage.managed = createStorageArea();
137-
chrome.storage.session = createStorageArea() as chrome.storage.SessionStorageArea;
134+
Object.defineProperties(chrome.storage, {
135+
local: {value: createStorageArea() as chrome.storage.LocalStorageArea, writable: true, configurable: true},
136+
sync: {value: createStorageArea() as chrome.storage.SyncStorageArea, writable: true, configurable: true},
137+
managed: {value: createStorageArea(), writable: true, configurable: true},
138+
session: {value: createStorageArea() as chrome.storage.SessionStorageArea, writable: true, configurable: true},
139+
});
138140

139141
chrome.storage.onChanged.addListener = jest.fn(cb => listeners.add(cb));
140142
chrome.storage.onChanged.removeListener = jest.fn(cb => listeners.delete(cb));

0 commit comments

Comments
 (0)