Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Apr 25, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 4 additions & 10 deletions src/lib/Cache.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { isLocalStorageAvailable } from './util';

class Cache {
//--------------------------------------------------------------------------
// Public
Expand Down Expand Up @@ -115,8 +117,7 @@ class Cache {
}

/**
* 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
Expand All @@ -128,14 +129,7 @@ class Cache {
*/
localStorageAvailable() {
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;
Expand Down
45 changes: 40 additions & 5 deletions src/lib/DownloadReachability.js
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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
*
Expand Down Expand Up @@ -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');
}
}

/**
Expand All @@ -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;
}

/**
Expand All @@ -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));
Expand All @@ -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;
}

/**
Expand Down
1 change: 1 addition & 0 deletions src/lib/__tests__/Cache-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ describe('Cache', () => {
});

it('should set and get correctly', () => {
sandbox.stub(cache, 'localStorageAvailable').returns(true);
const getSpy = sandbox.spy(localStorage, 'getItem');
const setSpy = sandbox.spy(localStorage, 'setItem');

Expand Down
5 changes: 3 additions & 2 deletions src/lib/__tests__/DownloadReachability-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ describe('lib/DownloadReachability', () => {
beforeEach(() => {
sessionStorage.clear();
localStorage.clear();
sandbox.stub(DownloadReachability, 'isStorageAvailable').returns(true);
});

afterEach(() => {
Expand Down Expand Up @@ -92,7 +93,7 @@ describe('lib/DownloadReachability', () => {

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');
Expand All @@ -101,7 +102,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;
});
});

Expand Down
6 changes: 5 additions & 1 deletion src/lib/__tests__/util-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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');
});
Expand All @@ -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');
});
});
Expand Down
18 changes: 18 additions & 0 deletions src/lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}