Fix: Guard session/local storage usage in download reachability (#784)

src/lib/Cache.js

@@ -1,3 +1,5 @@
+import { isLocalStorageAvailable } from './util';
+
 class Cache {
     //--------------------------------------------------------------------------
     // Public
@@ -25,7 +27,7 @@ class Cache {
     set(key, value, useLocalStorage) {
         this.cache[key] = value;
 
-        if (useLocalStorage && this.localStorageAvailable()) {
+        if (useLocalStorage && this.isLocalStorageAvailable()) {
             localStorage.setItem(this.generateKey(key), JSON.stringify(value));
         }
     }
@@ -38,7 +40,7 @@ class Cache {
      * @return {void}
      */
     unset(key) {
-        if (this.localStorageAvailable()) {
+        if (this.isLocalStorageAvailable()) {
             localStorage.removeItem(this.generateKey(key));
         }
 
@@ -107,16 +109,15 @@ class Cache {
      * @return {boolean} Whether the cache has key
      */
     inLocalStorage(key) {
-        if (!this.localStorageAvailable()) {
+        if (!this.isLocalStorageAvailable()) {
             return false;
         }
 
         return !!localStorage.getItem(this.generateKey(key));
     }
 
     /**
-     * Checks whether localStorage is available or not, derived from
-     * https://goo.gl/XE10Gu.
+     * Checks whether localStorage is available.
      *
      * @NOTE(tjin): This check is cached to not have to write/read from disk
      * every time this check is needed, but this will not catch instances where
@@ -126,16 +127,9 @@ class Cache {
      * @private
      * @return {boolean} Whether or not localStorage is available or not.
      */
-    localStorageAvailable() {
+    isLocalStorageAvailable() {
         if (this.available === undefined) {
-            try {
-                const x = '__storage_test__';
-                localStorage.setItem(x, x);
-                localStorage.removeItem(x);
-                this.available = true;
-            } catch (e) {
-                this.available = false;
-            }
+            this.available = isLocalStorageAvailable();
         }
 
         return this.available;

src/lib/DownloadReachability.js

@@ -1,4 +1,4 @@
-import { openUrlInsideIframe } from './util';
+import { openUrlInsideIframe, isLocalStorageAvailable } from './util';
 
 const DEFAULT_DOWNLOAD_HOST_PREFIX = 'https://dl.';
 const PROD_CUSTOM_HOST_SUFFIX = 'boxcloud.com';
@@ -7,7 +7,28 @@ const DOWNLOAD_HOST_FALLBACK_KEY = 'download_host_fallback';
 const NUMBERED_HOST_PREFIX_REGEX = /^https:\/\/dl\d+\./;
 const CUSTOM_HOST_PREFIX_REGEX = /^https:\/\/[A-Za-z0-9]+./;
 
+let IS_STORAGE_AVAILABLE;
+
 class DownloadReachability {
+    /**
+     * Checks whether localStorage and localStorage are available.
+     *
+     * This check is cached to not have to write/read from disk
+     * every time this check is needed, but this will not catch instances where
+     * localStorage/sessionStorage was available the first time this is called, but becomes
+     * unavailable at a later time.
+     *
+     * @private
+     * @return {boolean} Whether or not localStorage and sessionStorage are available or not.
+     */
+    static isStorageAvailable() {
+        if (IS_STORAGE_AVAILABLE === undefined) {
+            IS_STORAGE_AVAILABLE = isLocalStorageAvailable();
+        }
+
+        return IS_STORAGE_AVAILABLE;
+    }
+
     /**
      * Extracts the hostname from a URL
      *
@@ -61,7 +82,9 @@ class DownloadReachability {
      * @return {void}
      */
     static setDownloadHostFallback() {
-        sessionStorage.setItem(DOWNLOAD_HOST_FALLBACK_KEY, 'true');
+        if (DownloadReachability.isStorageAvailable()) {
+            sessionStorage.setItem(DOWNLOAD_HOST_FALLBACK_KEY, 'true');
+        }
     }
 
     /**
@@ -71,7 +94,11 @@ class DownloadReachability {
      * @return {boolean} Whether the sessionStorage indicates that a download host has been blocked
      */
     static isDownloadHostBlocked() {
-        return sessionStorage.getItem(DOWNLOAD_HOST_FALLBACK_KEY) === 'true';
+        if (DownloadReachability.isStorageAvailable()) {
+            return sessionStorage.getItem(DOWNLOAD_HOST_FALLBACK_KEY) === 'true';
+        }
+
+        return false;
     }
 
     /**
@@ -82,6 +109,10 @@ class DownloadReachability {
      * @return {void}
      */
     static setDownloadHostNotificationShown(downloadHost) {
+        if (!DownloadReachability.isStorageAvailable()) {
+            return;
+        }
+
         const shownHostsArr = JSON.parse(localStorage.getItem(DOWNLOAD_NOTIFICATION_SHOWN_KEY)) || [];
         shownHostsArr.push(downloadHost);
         localStorage.setItem(DOWNLOAD_NOTIFICATION_SHOWN_KEY, JSON.stringify(shownHostsArr));
@@ -92,17 +123,21 @@ class DownloadReachability {
      *
      * @public
      * @param {string} downloadUrl - Content download URL
-     * @return {string|undefined} Which host should we show a notification for, if any
+     * @return {string|null} Which host should we show a notification for, if any
      */
     static getDownloadNotificationToShow(downloadUrl) {
+        if (!DownloadReachability.isStorageAvailable()) {
+            return null;
+        }
+
         const contentHostname = DownloadReachability.getHostnameFromUrl(downloadUrl);
         const shownHostsArr = JSON.parse(localStorage.getItem(DOWNLOAD_NOTIFICATION_SHOWN_KEY)) || [];
 
         return sessionStorage.getItem(DOWNLOAD_HOST_FALLBACK_KEY) === 'true' &&
             !shownHostsArr.includes(contentHostname) &&
             DownloadReachability.isCustomDownloadHost(downloadUrl)
             ? contentHostname
-            : undefined;
+            : null;
     }
 
     /**

src/lib/__tests__/Cache-test.js

@@ -15,6 +15,7 @@ describe('Cache', () => {
     });
 
     it('should set and get correctly', () => {
+        sandbox.stub(cache, 'isLocalStorageAvailable').returns(true);
         const getSpy = sandbox.spy(localStorage, 'getItem');
         const setSpy = sandbox.spy(localStorage, 'setItem');
 

src/lib/__tests__/DownloadReachability-test.js

@@ -13,6 +13,7 @@ describe('lib/DownloadReachability', () => {
     beforeEach(() => {
         sessionStorage.clear();
         localStorage.clear();
+        sandbox.stub(DownloadReachability, 'isStorageAvailable').returns(true);
     });
 
     afterEach(() => {
@@ -63,25 +64,41 @@ describe('lib/DownloadReachability', () => {
 
             expect(sessionStorage.getItem(DOWNLOAD_HOST_FALLBACK_KEY)).to.equal('true');
         });
-    });
 
-    describe('isDownloadHostBlocked()', () => {
-        it('should set the download host fallback key to be true', () => {
-            expect(DownloadReachability.isDownloadHostBlocked()).to.be.false;
+        it('should do nothing if we do not have access to session storage', () => {
+            DownloadReachability.isStorageAvailable.returns(false);
 
+            expect(sessionStorage.getItem(DOWNLOAD_HOST_FALLBACK_KEY)).to.not.equal('true');
             DownloadReachability.setDownloadHostFallback();
+            expect(sessionStorage.getItem(DOWNLOAD_HOST_FALLBACK_KEY)).to.not.equal('true');
+        });
+    });
 
+    describe('isDownloadHostBlocked()', () => {
+        beforeEach(() => {
+            sessionStorage.setItem(DOWNLOAD_HOST_FALLBACK_KEY, 'true');
+        });
+        it('should be true if session storage contains the host blocked key', () => {
             expect(DownloadReachability.isDownloadHostBlocked()).to.be.true;
         });
-    });
 
-    describe('setDownloadHostNotificationShown()', () => {
-        it('should set the download host fallback key to be true', () => {
+        it('should return false if we do not have access to session storage', () => {
+            DownloadReachability.isStorageAvailable.returns(false);
             expect(DownloadReachability.isDownloadHostBlocked()).to.be.false;
+        });
+    });
 
-            DownloadReachability.setDownloadHostFallback();
+    describe('setDownloadHostNotificationShown()', () => {
+        it('should do nothing if we do not have access to local storage', () => {
+            DownloadReachability.isStorageAvailable.returns(false);
+            DownloadReachability.setDownloadHostNotificationShown('foo');
+            expect(localStorage.getItem(DOWNLOAD_NOTIFICATION_SHOWN_KEY)).to.be.null;
+        });
 
-            expect(DownloadReachability.isDownloadHostBlocked()).to.be.true;
+        it('should add the shown host to the array of already shown hosts', () => {
+            const hostShown = 'www.blockedhost.box.com';
+            DownloadReachability.setDownloadHostNotificationShown(hostShown);
+            expect(localStorage.getItem(DOWNLOAD_NOTIFICATION_SHOWN_KEY)).to.equal(`["${hostShown}"]`);
         });
     });
 
@@ -90,9 +107,16 @@ describe('lib/DownloadReachability', () => {
             sessionStorage.setItem('download_host_fallback', 'false');
         });
 
+        it('should do nothing if we do not have access to session storage', () => {
+            DownloadReachability.isStorageAvailable.returns(false);
+            sessionStorage.setItem('download_host_fallback', 'true');
+            const result = DownloadReachability.getDownloadNotificationToShow('https://dl5.boxcloud.com');
+            expect(result).to.equal(null);
+        });
+
         it('should return true if we do not have an entry for the given host and our session indicates we are falling back to the default host', () => {
             let result = DownloadReachability.getDownloadNotificationToShow('https://foo.com');
-            expect(result).to.be.undefined;
+            expect(result).to.be.null;
 
             sessionStorage.setItem('download_host_fallback', 'true');
             result = DownloadReachability.getDownloadNotificationToShow('https://dl5.boxcloud.com');
@@ -101,7 +125,7 @@ describe('lib/DownloadReachability', () => {
             const shownHostsArr = ['dl5.boxcloud.com'];
             localStorage.setItem('download_host_notification_shown', JSON.stringify(shownHostsArr));
             result = DownloadReachability.getDownloadNotificationToShow('https://dl5.boxcloud.com');
-            expect(result).to.be.undefined;
+            expect(result).to.be.null;
         });
     });
 

src/lib/__tests__/util-test.js

@@ -3,6 +3,7 @@ import 'whatwg-fetch';
 import fetchMock from 'fetch-mock';
 import Location from '../Location';
 import * as util from '../util';
+import DownloadReachability from '../DownloadReachability';
 
 const sandbox = sinon.sandbox.create();
 
@@ -336,6 +337,9 @@ describe('lib/util', () => {
     /* eslint-enable no-undef */
 
     describe('createContentUrl()', () => {
+        beforeEach(() => {
+            sandbox.stub(DownloadReachability, 'isDownloadHostBlocked').returns(false);
+        });
         it('should return correct content url when no asset name', () => {
             expect(util.createContentUrl('foo{+asset_path}', null)).to.equal('foo');
         });
@@ -349,7 +353,7 @@ describe('lib/util', () => {
         });
 
         it('should replace the download host with the default if we are falling back', () => {
-            sessionStorage.setItem('download_host_fallback', 'true');
+            DownloadReachability.isDownloadHostBlocked.returns(true);
             expect(util.createContentUrl('https://dl6.boxcloud.com', 'bar')).to.equal('https://dl.boxcloud.com');
         });
     });

src/lib/util.js

@@ -868,3 +868,21 @@ export function convertWatermarkPref(previewWMPref) {
 
     return value;
 }
+
+/**
+ * Checks whether localStorage is available or not, derived from
+ * https://goo.gl/XE10Gu.
+ *
+ *
+ * @return {boolean} Whether or not localStorage is available or not.
+ */
+export function isLocalStorageAvailable() {
+    try {
+        const x = '__storage_test__';
+        localStorage.setItem(x, x);
+        localStorage.removeItem(x);
+        return true;
+    } catch (e) {
+        return false;
+    }
+}