From a30f609de48227df08f633bc0443e16678b20d32 Mon Sep 17 00:00:00 2001
From: Samuel Mannehed <samuel@cendio.se>
Date: Wed, 2 Nov 2022 10:23:36 +0100
Subject: [PATCH] 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.
---
 app/webutil.js        | 74 ++++++++++++++++++++++++++++++++++++++++---
 tests/test.webutil.js | 15 +++++++++
 2 files changed, 84 insertions(+), 5 deletions(-)

diff --git a/app/webutil.js b/app/webutil.js
index b94f035d3..6011442cb 100644
--- a/app/webutil.js
+++ b/app/webutil.js
@@ -6,16 +6,16 @@
  * 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
 export function initLogging(level) {
     "use strict";
     if (typeof level !== "undefined") {
-        mainInitLogging(level);
+        Log.initLogging(level);
     } else {
         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) {
         window.chrome.storage.sync.set(settings);
     } 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)) {
         value = settings[name];
     } else {
-        value = localStorage.getItem(name);
+        value = localStorageGet(name);
         settings[name] = value;
     }
     if (typeof value === "undefined") {
@@ -181,6 +181,70 @@ export function eraseSetting(name) {
     if (window.chrome && window.chrome.storage) {
         window.chrome.storage.sync.remove(name);
     } else {
+        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;
+        }
     }
 }
diff --git a/tests/test.webutil.js b/tests/test.webutil.js
index 6f460a3fc..df8227aef 100644
--- a/tests/test.webutil.js
+++ b/tests/test.webutil.js
@@ -92,6 +92,11 @@ describe('WebUtil', function () {
                     expect(window.localStorage.setItem).to.have.been.calledWithExactly('test', '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 () {
@@ -137,6 +142,11 @@ describe('WebUtil', function () {
                     WebUtil.writeSetting('test', '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
@@ -145,6 +155,11 @@ describe('WebUtil', function () {
                     WebUtil.eraseSetting('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;
+                });
             });
         });