Don't crash if we can't use localStorage

Our settings are not a fatal requirement, we can fall back on the
default values if they can't be accessed. A scenario where we've seen
this happen is when cookies are disabled in the browser. It seems
localStorage is disabled along with cookies in these settings.

So, lets log a message about the failure and otherwise silently
continue in this case.

Fixes issue #1577.
This commit is contained in:
Samuel Mannehed 2022-11-02 10:23:36 +01:00
parent ca6527c1bf
commit a30f609de4
2 changed files with 85 additions and 6 deletions

View File

@ -6,16 +6,16 @@
* See README.md for usage and integration instructions. * See README.md for usage and integration instructions.
*/ */
import { initLogging as mainInitLogging } from '../core/util/logging.js'; import * as Log from '../core/util/logging.js';
// init log level reading the logging HTTP param // init log level reading the logging HTTP param
export function initLogging(level) { export function initLogging(level) {
"use strict"; "use strict";
if (typeof level !== "undefined") { if (typeof level !== "undefined") {
mainInitLogging(level); Log.initLogging(level);
} else { } else {
const param = document.location.href.match(/logging=([A-Za-z0-9._-]*)/); const param = document.location.href.match(/logging=([A-Za-z0-9._-]*)/);
mainInitLogging(param || undefined); Log.initLogging(param || undefined);
} }
} }
@ -146,7 +146,7 @@ export function writeSetting(name, value) {
if (window.chrome && window.chrome.storage) { if (window.chrome && window.chrome.storage) {
window.chrome.storage.sync.set(settings); window.chrome.storage.sync.set(settings);
} else { } else {
localStorage.setItem(name, value); localStorageSet(name, value);
} }
} }
@ -156,7 +156,7 @@ export function readSetting(name, defaultValue) {
if ((name in settings) || (window.chrome && window.chrome.storage)) { if ((name in settings) || (window.chrome && window.chrome.storage)) {
value = settings[name]; value = settings[name];
} else { } else {
value = localStorage.getItem(name); value = localStorageGet(name);
settings[name] = value; settings[name] = value;
} }
if (typeof value === "undefined") { if (typeof value === "undefined") {
@ -181,6 +181,70 @@ export function eraseSetting(name) {
if (window.chrome && window.chrome.storage) { if (window.chrome && window.chrome.storage) {
window.chrome.storage.sync.remove(name); window.chrome.storage.sync.remove(name);
} else { } else {
localStorage.removeItem(name); localStorageRemove(name);
}
}
let loggedMsgs = [];
function logOnce(msg, level = "warn") {
if (!loggedMsgs.includes(msg)) {
switch (level) {
case "error":
Log.Error(msg);
break;
case "warn":
Log.Warn(msg);
break;
case "debug":
Log.Debug(msg);
break;
default:
Log.Info(msg);
}
loggedMsgs.push(msg);
}
}
let cookiesMsg = "Couldn't access noVNC settings, are cookies disabled?";
function localStorageGet(name) {
let r;
try {
r = localStorage.getItem(name);
} catch (e) {
if (e instanceof DOMException) {
logOnce(cookiesMsg);
logOnce("'localStorage.getItem(" + name + ")' failed: " + e,
"debug");
} else {
throw e;
}
}
return r;
}
function localStorageSet(name, value) {
try {
localStorage.setItem(name, value);
} catch (e) {
if (e instanceof DOMException) {
logOnce(cookiesMsg);
logOnce("'localStorage.setItem(" + name + "," + value +
")' failed: " + e, "debug");
} else {
throw e;
}
}
}
function localStorageRemove(name) {
try {
localStorage.removeItem(name);
} catch (e) {
if (e instanceof DOMException) {
logOnce(cookiesMsg);
logOnce("'localStorage.removeItem(" + name + ")' failed: " + e,
"debug");
} else {
throw e;
}
} }
} }

View File

@ -92,6 +92,11 @@ describe('WebUtil', function () {
expect(window.localStorage.setItem).to.have.been.calledWithExactly('test', 'value'); expect(window.localStorage.setItem).to.have.been.calledWithExactly('test', 'value');
expect(WebUtil.readSetting('test')).to.equal('value'); expect(WebUtil.readSetting('test')).to.equal('value');
}); });
it('should not crash when local storage save fails', function () {
localStorage.setItem.throws(new DOMException());
expect(WebUtil.writeSetting('test', 'value')).to.not.throw;
});
}); });
describe('setSetting', function () { describe('setSetting', function () {
@ -137,6 +142,11 @@ describe('WebUtil', function () {
WebUtil.writeSetting('test', 'something else'); WebUtil.writeSetting('test', 'something else');
expect(WebUtil.readSetting('test')).to.equal('something else'); expect(WebUtil.readSetting('test')).to.equal('something else');
}); });
it('should not crash when local storage read fails', function () {
localStorage.getItem.throws(new DOMException());
expect(WebUtil.readSetting('test')).to.not.throw;
});
}); });
// this doesn't appear to be used anywhere // this doesn't appear to be used anywhere
@ -145,6 +155,11 @@ describe('WebUtil', function () {
WebUtil.eraseSetting('test'); WebUtil.eraseSetting('test');
expect(window.localStorage.removeItem).to.have.been.calledWithExactly('test'); expect(window.localStorage.removeItem).to.have.been.calledWithExactly('test');
}); });
it('should not crash when local storage remove fails', function () {
localStorage.removeItem.throws(new DOMException());
expect(WebUtil.eraseSetting('test')).to.not.throw;
});
}); });
}); });