From 0d64a4b18ad59db273963513d2c234c41cb8d35c Mon Sep 17 00:00:00 2001 From: Marvin Liu Date: Fri, 3 Mar 2023 09:54:10 -0800 Subject: [PATCH] fix: skip top domain when cookie is disabled --- src/amplitude-client.js | 3 +-- src/base-cookie.js | 3 +-- src/cookie.js | 3 +-- src/get-host.js | 19 ------------------- src/get-location.js | 7 ------- src/metadata-storage.js | 5 ++--- src/top-domain.js | 3 +-- src/utils.js | 22 +++++++++++++++++++++ test/amplitude-client.js | 41 +++++++++++++++++++++++++++++++++++----- test/utils.js | 18 ++++++++++++++++++ 10 files changed, 82 insertions(+), 42 deletions(-) delete mode 100644 src/get-host.js delete mode 100644 src/get-location.js diff --git a/src/amplitude-client.js b/src/amplitude-client.js index 1d704d74..bc92b96c 100644 --- a/src/amplitude-client.js +++ b/src/amplitude-client.js @@ -15,7 +15,6 @@ import utils from './utils'; import UUID from './uuid'; import base64Id from './base64Id'; import DEFAULT_OPTIONS from './options'; -import getHost from './get-host'; import baseCookie from './base-cookie'; import { AmplitudeServerZone, getEventLogApi } from './server-zone'; import ConfigManager from './config-manager'; @@ -309,7 +308,7 @@ AmplitudeClient.prototype._runNewSessionStartCallbacks = function () { }; AmplitudeClient.prototype.deleteLowerLevelDomainCookies = function () { - const host = getHost(); + const host = utils.getHost(); const cookieHost = this.options.domain && this.options.domain[0] === '.' ? this.options.domain.slice(1) : this.options.domain; diff --git a/src/base-cookie.js b/src/base-cookie.js index 832558f3..b087ce01 100644 --- a/src/base-cookie.js +++ b/src/base-cookie.js @@ -1,5 +1,4 @@ import Constants from './constants'; -import base64Id from './base64Id'; import utils from './utils'; const get = (name) => { @@ -94,7 +93,7 @@ const sortByEventTime = (cookies) => { // test that cookies are enabled - navigator.cookiesEnabled yields false positives in IE, need to test directly const areCookiesEnabled = (opts = {}) => { - const cookieName = Constants.COOKIE_TEST_PREFIX + base64Id(); + const cookieName = Constants.COOKIE_TEST_PREFIX; if (typeof document === 'undefined') { return false; } diff --git a/src/cookie.js b/src/cookie.js index d647991a..0ae2bc9e 100644 --- a/src/cookie.js +++ b/src/cookie.js @@ -4,7 +4,6 @@ import Base64 from './base64'; import utils from './utils'; -import getLocation from './get-location'; import baseCookie from './base-cookie'; import topDomain from './top-domain'; @@ -31,7 +30,7 @@ var options = function (opts) { _options.secure = opts.secure; _options.sameSite = opts.sameSite; - var domain = !utils.isEmptyString(opts.domain) ? opts.domain : '.' + topDomain(getLocation().href); + var domain = !utils.isEmptyString(opts.domain) ? opts.domain : '.' + topDomain(utils.getLocation().href); var token = Math.random(); _options.domain = domain; set('amplitude_test', token); diff --git a/src/get-host.js b/src/get-host.js deleted file mode 100644 index b0affaac..00000000 --- a/src/get-host.js +++ /dev/null @@ -1,19 +0,0 @@ -import GlobalScope from './global-scope'; - -const getHost = (url) => { - const defaultHostname = GlobalScope.location ? GlobalScope.location.hostname : ''; - if (url) { - if (typeof document !== 'undefined') { - const a = document.createElement('a'); - a.href = url; - return a.hostname || defaultHostname; - } - if (typeof URL === 'function') { - const u = new URL(url); - return u.hostname || defaultHostname; - } - } - return defaultHostname; -}; - -export default getHost; diff --git a/src/get-location.js b/src/get-location.js deleted file mode 100644 index ad8ba3d4..00000000 --- a/src/get-location.js +++ /dev/null @@ -1,7 +0,0 @@ -import GlobalScope from './global-scope'; - -const getLocation = () => { - return GlobalScope.location; -}; - -export default getLocation; diff --git a/src/metadata-storage.js b/src/metadata-storage.js index e5648660..46420da2 100644 --- a/src/metadata-storage.js +++ b/src/metadata-storage.js @@ -6,7 +6,6 @@ import Base64 from './base64'; import baseCookie from './base-cookie'; import Constants from './constants'; -import getLocation from './get-location'; import ampLocalStorage from './localstorage'; import topDomain from './top-domain'; import utils from './utils'; @@ -35,8 +34,8 @@ class MetadataStorage { this.expirationDays = expirationDays; this.cookieDomain = ''; - const loc = getLocation() ? getLocation().href : undefined; - const writableTopDomain = topDomain(loc); + const loc = utils.getLocation() ? utils.getLocation().href : undefined; + const writableTopDomain = !disableCookies ? topDomain(loc) : ''; this.cookieDomain = domain || (writableTopDomain ? '.' + writableTopDomain : null); if (storageOptionExists[storage]) { diff --git a/src/top-domain.js b/src/top-domain.js index c033de4d..0f1865b4 100644 --- a/src/top-domain.js +++ b/src/top-domain.js @@ -1,11 +1,10 @@ import baseCookie from './base-cookie'; import base64Id from './base64Id'; -import getHost from './get-host'; import utils from './utils'; // Utility that finds top level domain to write to const topDomain = (url) => { - const host = getHost(url); + const host = utils.getHost(url); const parts = host.split('.'); const levels = []; const cname = '_tldtest_' + base64Id(); diff --git a/src/utils.js b/src/utils.js index 89ee99c8..7981783d 100644 --- a/src/utils.js +++ b/src/utils.js @@ -287,6 +287,26 @@ const validateSessionId = (sessionId) => { return false; }; +const getLocation = () => { + return GlobalScope.location; +}; + +const getHost = (url) => { + const defaultHostname = GlobalScope.location ? GlobalScope.location.hostname : ''; + if (url) { + if (typeof document !== 'undefined') { + const a = document.createElement('a'); + a.href = url; + return a.hostname || defaultHostname; + } + if (typeof URL === 'function') { + const u = new URL(url); + return u.hostname || defaultHostname; + } + } + return defaultHostname; +}; + export default { setLogLevel, getLogLevel, @@ -303,4 +323,6 @@ export default { validateDeviceId, validateTransport, validateSessionId, + getLocation, + getHost, }; diff --git a/test/amplitude-client.js b/test/amplitude-client.js index e647e59c..1803b6cb 100644 --- a/test/amplitude-client.js +++ b/test/amplitude-client.js @@ -2739,7 +2739,7 @@ describe('AmplitudeClient', function () { }); }); - it('should not create any cookies if disabledCookies = true', function () { + it('should not use any cookies with simple host if disabledCookies = true', function () { deleteAllCookies(); clock.tick(20); @@ -2747,15 +2747,46 @@ describe('AmplitudeClient', function () { assert.equal(cookieArray.length, 0); var deviceId = 'test_device_id'; - var amplitude2 = new AmplitudeClient(); + var amplitude = new AmplitudeClient(); - amplitude2.init(apiKey, null, { + amplitude.init(apiKey, null, { + deviceId: deviceId, + disableCookies: true, + }); + + cookieArray = getAllCookies(); + assert.equal(cookieArray.length, 0); + }); + + it('should not use any cookies with multiple domains in host if disabledCookies = true', function () { + const stubbedGetLocation = sinon.stub(utils, 'getLocation').returns({ href: 'https://abc.def.xyz/test' }); + const spiedSet = sinon.spy(baseCookie, 'set'); + const spiedGet = sinon.spy(baseCookie, 'get'); + deleteAllCookies(); + clock.tick(20); + + var cookieArray = getAllCookies(); + assert.equal(cookieArray.length, 0); + + var deviceId = 'test_device_id'; + var amplitude = new AmplitudeClient(); + + amplitude.init(apiKey, null, { deviceId: deviceId, disableCookies: true, }); cookieArray = getAllCookies(); assert.equal(cookieArray.length, 0); + + // spied cookie operations should not be fired + assert.equal(spiedSet.called, false); + assert.equal(spiedGet.called, false); + + // restore stub, spy + stubbedGetLocation.restore(); + spiedSet.restore(); + spiedGet.restore(); }); it('should create cookies if disabledCookies = false', function () { @@ -2766,9 +2797,9 @@ describe('AmplitudeClient', function () { assert.equal(cookieArray.length, 0); var deviceId = 'test_device_id'; - var amplitude2 = new AmplitudeClient(); + var amplitude = new AmplitudeClient(); - amplitude2.init(apiKey, null, { + amplitude.init(apiKey, null, { deviceId: deviceId, disableCookies: false, }); diff --git a/test/utils.js b/test/utils.js index 6f9c43e0..483fd963 100644 --- a/test/utils.js +++ b/test/utils.js @@ -1,6 +1,7 @@ import sinon from 'sinon'; import utils from '../src/utils.js'; import constants from '../src/constants.js'; +import GlobalScope from '../src/global-scope'; describe('utils', function () { describe('isEmptyString', function () { @@ -280,4 +281,21 @@ describe('utils', function () { assert.isFalse(utils.validateSessionId(false)); }); }); + + describe('getHost', function () { + it('should return hostname for url', function () { + const url = 'https://amplitude.is.good.com/test'; + assert.equal(utils.getHost(url), 'amplitude.is.good.com'); + }); + + it('should return current hostname if no url is provided', function () { + assert.equal(utils.getHost(), GlobalScope.location.hostname); + }); + }); + + describe('getLocation', function () { + it('should return global location', function () { + assert.equal(utils.getLocation(), GlobalScope.location); + }); + }); });