From 61b6590593238a5f251cbf5aa16233fdd3954802 Mon Sep 17 00:00:00 2001 From: Kelvin Lu Date: Wed, 16 Sep 2020 17:37:03 -0700 Subject: [PATCH] fix(cookies): respect the options passed into cookies when testing to see if they're enabled (#294) * fix(cookies): respect the options passed into cookies when testing to see if they're enabled * some shuffling around --- src/base-cookie.js | 13 +++---- src/metadata-storage.js | 79 ++++++++++++++++++++++++++-------------- test/amplitude-client.js | 2 +- 3 files changed, 58 insertions(+), 36 deletions(-) diff --git a/src/base-cookie.js b/src/base-cookie.js index 997f959b..79c4bb13 100644 --- a/src/base-cookie.js +++ b/src/base-cookie.js @@ -23,10 +23,10 @@ const get = (name) => { }; const set = (name, value, opts) => { - let expires = value !== null ? opts.expirationDays : -1 ; + let expires = value !== null ? opts.expirationDays : -1; if (expires) { const date = new Date(); - date.setTime(date.getTime() + (expires * 24 * 60 * 60 * 1000)); + date.setTime(date.getTime() + expires * 24 * 60 * 60 * 1000); expires = date; } let str = name + '=' + value; @@ -46,15 +46,14 @@ const set = (name, value, opts) => { document.cookie = str; }; - // test that cookies are enabled - navigator.cookiesEnabled yields false positives in IE, need to test directly -const areCookiesEnabled = () => { +const areCookiesEnabled = (opts = {}) => { const uid = String(new Date()); try { const cookieName = Constants.COOKIE_TEST_PREFIX + base64Id(); - set(cookieName, uid, {}); + set(cookieName, uid, opts); const _areCookiesEnabled = get(cookieName + '=') === uid; - set(cookieName, null, {}); + set(cookieName, null, opts); return _areCookiesEnabled; } catch (e) {} return false; @@ -63,5 +62,5 @@ const areCookiesEnabled = () => { export default { set, get, - areCookiesEnabled + areCookiesEnabled, }; diff --git a/src/metadata-storage.js b/src/metadata-storage.js index f78ede01..5e085211 100644 --- a/src/metadata-storage.js +++ b/src/metadata-storage.js @@ -10,25 +10,42 @@ import localStorage from './localstorage'; // jshint ignore:line import topDomain from './top-domain'; /** -* MetadataStorage involves SDK data persistance -* storage priority: cookies -> localStorage -> in memory -* if in localStorage, unable track users between subdomains -* if in memory, then memory can't be shared between different tabs -*/ - class MetadataStorage { - constructor({storageKey, disableCookies, domain, secure, sameSite, expirationDays}) { + * MetadataStorage involves SDK data persistance + * storage priority: cookies -> localStorage -> in memory + * if in localStorage, unable track users between subdomains + * if in memory, then memory can't be shared between different tabs + */ +class MetadataStorage { + constructor({ + storageKey, + disableCookies, + domain, + secure, + sameSite, + expirationDays, + }) { this.storageKey = storageKey; - this.disableCookieStorage = !baseCookie.areCookiesEnabled() || disableCookies; this.domain = domain; this.secure = secure; this.sameSite = sameSite; this.expirationDays = expirationDays; - this.cookieDomain =''; + + this.cookieDomain = ''; if (!BUILD_COMPAT_REACT_NATIVE) { const writableTopDomain = topDomain(getLocation().href); - this.cookieDomain = domain || (writableTopDomain ? '.' + writableTopDomain : null); + this.cookieDomain = + domain || (writableTopDomain ? '.' + writableTopDomain : null); } + + this.disableCookieStorage = + disableCookies || + !baseCookie.areCookiesEnabled({ + domain: this.cookieDomain, + secure: this.secure, + sameSite: this.sameSite, + expirationDays: this.expirationDays, + }); } getCookieStorageKey() { @@ -36,16 +53,26 @@ import topDomain from './top-domain'; return this.storageKey; } - const suffix = this.domain.charAt(0) === '.' ? this.domain.substring(1) : this.domain; + const suffix = + this.domain.charAt(0) === '.' ? this.domain.substring(1) : this.domain; return `${this.storageKey}${suffix ? `_${suffix}` : ''}`; } /* - * Data is saved as delimited values rather than JSO to minimize cookie space - * Should not change order of the items - */ - save({ deviceId, userId, optOut, sessionId, lastEventTime, eventId, identifyId, sequenceNumber }) { + * Data is saved as delimited values rather than JSO to minimize cookie space + * Should not change order of the items + */ + save({ + deviceId, + userId, + optOut, + sessionId, + lastEventTime, + eventId, + identifyId, + sequenceNumber, + }) { const value = [ deviceId, Base64.encode(userId || ''), // used to convert not unicode to alphanumeric since cookies only use alphanumeric @@ -54,22 +81,18 @@ import topDomain from './top-domain'; lastEventTime ? lastEventTime.toString(32) : '0', // last time an event was set eventId ? eventId.toString(32) : '0', identifyId ? identifyId.toString(32) : '0', - sequenceNumber ? sequenceNumber.toString(32) : '0' - ].join('.'); + sequenceNumber ? sequenceNumber.toString(32) : '0', + ].join('.'); if (this.disableCookieStorage) { localStorage.setItem(this.storageKey, value); } else { - baseCookie.set( - this.getCookieStorageKey(), - value, - { - domain: this.cookieDomain, - secure: this.secure, - sameSite: this.sameSite, - expirationDays: this.expirationDays - } - ); + baseCookie.set(this.getCookieStorageKey(), value, { + domain: this.cookieDomain, + secure: this.secure, + sameSite: this.sameSite, + expirationDays: this.expirationDays, + }); } } @@ -105,7 +128,7 @@ import topDomain from './top-domain'; lastEventTime: parseInt(values[4], 32), eventId: parseInt(values[5], 32), identifyId: parseInt(values[6], 32), - sequenceNumber: parseInt(values[7], 32) + sequenceNumber: parseInt(values[7], 32), }; } } diff --git a/test/amplitude-client.js b/test/amplitude-client.js index 53813cb9..e79b42c9 100644 --- a/test/amplitude-client.js +++ b/test/amplitude-client.js @@ -391,7 +391,7 @@ describe('AmplitudeClient', function() { identifyId: 60, sequenceNumber: 70 } - const storage = new MetadataStorage({storageKey: cookieName, disableCookieStorage: true}); + const storage = new MetadataStorage({storageKey: cookieName, disableCookies: true}); storage.save(cookieData); clock.tick(10);