Skip to content

Commit 10cf8c8

Browse files
ajhorstAJ Horst
andauthored
fix(cookies): AMP-35904 dedup cookies (#390)
* fix(cookie): add constants for cookie prop indexes * fix(cookie): dedup cookie on load * fix(cookie): make getLastEventTime more resilient * fix(cookie): fix event time sort * fix(cookie): clarify cookie constant comment * fix(cookie): clean up getAll method * fix(cookie): add unit tests for cookie parsing and sorting * fix(cookie): return empty array from getAll on exception * fix(cookie): remove duplicate test * fix(cookie): refactor getLastEventTime and add warning logs for malformed cookie Co-authored-by: AJ Horst <[email protected]>
1 parent 6c8c961 commit 10cf8c8

File tree

4 files changed

+134
-16
lines changed

4 files changed

+134
-16
lines changed

src/base-cookie.js

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,25 @@ const get = (name) => {
2323
}
2424
};
2525

26+
const getAll = (name) => {
27+
try {
28+
const cookieArray = document.cookie.split(';').map((c) => c.trimStart());
29+
let values = [];
30+
for (let cookie of cookieArray) {
31+
while (cookie.charAt(0) === ' ') {
32+
cookie = cookie.substring(1);
33+
}
34+
if (cookie.indexOf(name) === 0) {
35+
values.push(cookie.substring(name.length));
36+
}
37+
}
38+
39+
return values;
40+
} catch (e) {
41+
return [];
42+
}
43+
};
44+
2645
const set = (name, value, opts) => {
2746
let expires = value !== null ? opts.expirationDays : -1;
2847
if (expires) {
@@ -47,6 +66,32 @@ const set = (name, value, opts) => {
4766
document.cookie = str;
4867
};
4968

69+
const getLastEventTime = (cookie = '') => {
70+
const strValue = cookie.split('.')[Constants.LAST_EVENT_TIME_INDEX];
71+
72+
let parsedValue;
73+
if (strValue) {
74+
parsedValue = parseInt(strValue, 32);
75+
}
76+
77+
if (parsedValue) {
78+
return parsedValue;
79+
} else {
80+
utils.log.warn(`unable to parse malformed cookie: ${cookie}`);
81+
return 0;
82+
}
83+
};
84+
85+
const sortByEventTime = (cookies) => {
86+
return [...cookies].sort((c1, c2) => {
87+
const t1 = getLastEventTime(c1);
88+
const t2 = getLastEventTime(c2);
89+
// sort c1 first if its last event time is more recent
90+
// i.e its event time integer is larger that c2's
91+
return t2 - t1;
92+
});
93+
};
94+
5095
// test that cookies are enabled - navigator.cookiesEnabled yields false positives in IE, need to test directly
5196
const areCookiesEnabled = (opts = {}) => {
5297
const cookieName = Constants.COOKIE_TEST_PREFIX + base64Id();
@@ -68,5 +113,8 @@ const areCookiesEnabled = (opts = {}) => {
68113
export default {
69114
set,
70115
get,
116+
getAll,
117+
getLastEventTime,
118+
sortByEventTime,
71119
areCookiesEnabled,
72120
};

src/constants.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,16 @@ export default {
1818
OPT_OUT: 'amplitude_optOut',
1919
USER_ID: 'amplitude_userId',
2020

21+
// indexes of properties in cookie v2 storage format
22+
DEVICE_ID_INDEX: 0,
23+
USER_ID_INDEX: 1,
24+
OPT_OUT_INDEX: 2,
25+
SESSION_ID_INDEX: 3,
26+
LAST_EVENT_TIME_INDEX: 4,
27+
EVENT_ID_INDEX: 5,
28+
IDENTIFY_ID_INDEX: 6,
29+
SEQUENCE_NUMBER_INDEX: 7,
30+
2131
COOKIE_TEST_PREFIX: 'amp_cookie_test',
2232
COOKIE_PREFIX: 'amp',
2333

src/metadata-storage.js

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -96,20 +96,35 @@ class MetadataStorage {
9696
ampLocalStorage.setItem(this.storageKey, value);
9797
break;
9898
case Constants.STORAGE_COOKIES:
99-
baseCookie.set(this.getCookieStorageKey(), value, {
100-
domain: this.cookieDomain,
101-
secure: this.secure,
102-
sameSite: this.sameSite,
103-
expirationDays: this.expirationDays,
104-
});
99+
this.saveCookie(value);
105100
break;
106101
}
107102
}
108103

104+
saveCookie(value) {
105+
baseCookie.set(this.getCookieStorageKey(), value, {
106+
domain: this.cookieDomain,
107+
secure: this.secure,
108+
sameSite: this.sameSite,
109+
expirationDays: this.expirationDays,
110+
});
111+
}
112+
109113
load() {
110114
let str;
111115
if (this.storage === Constants.STORAGE_COOKIES) {
112-
str = baseCookie.get(this.getCookieStorageKey() + '=');
116+
const cookieKey = this.getCookieStorageKey() + '=';
117+
const allCookies = baseCookie.getAll(cookieKey);
118+
if (allCookies.length === 0 || allCookies.length === 1) {
119+
str = allCookies[0];
120+
} else {
121+
// dedup cookies by deleting them all and restoring
122+
// the one with the most recent event time
123+
const latestCookie = baseCookie.sortByEventTime(allCookies)[0];
124+
allCookies.forEach(() => baseCookie.set(this.getCookieStorageKey(), null, {}));
125+
this.saveCookie(latestCookie);
126+
str = baseCookie.get(cookieKey);
127+
}
113128
}
114129
if (!str) {
115130
str = ampLocalStorage.getItem(this.storageKey);
@@ -129,23 +144,23 @@ class MetadataStorage {
129144
const values = str.split('.');
130145

131146
let userId = null;
132-
if (values[1]) {
147+
if (values[Constants.USER_ID_INDEX]) {
133148
try {
134-
userId = Base64.decode(values[1]);
149+
userId = Base64.decode(values[Constants.USER_ID_INDEX]);
135150
} catch (e) {
136151
userId = null;
137152
}
138153
}
139154

140155
return {
141-
deviceId: values[0],
156+
deviceId: values[Constants.DEVICE_ID_INDEX],
142157
userId,
143-
optOut: values[2] === '1',
144-
sessionId: parseInt(values[3], 32),
145-
lastEventTime: parseInt(values[4], 32),
146-
eventId: parseInt(values[5], 32),
147-
identifyId: parseInt(values[6], 32),
148-
sequenceNumber: parseInt(values[7], 32),
158+
optOut: values[Constants.OPT_OUT_INDEX] === '1',
159+
sessionId: parseInt(values[Constants.SESSION_ID_INDEX], 32),
160+
lastEventTime: parseInt(values[Constants.LAST_EVENT_TIME_INDEX], 32),
161+
eventId: parseInt(values[Constants.EVENT_ID_INDEX], 32),
162+
identifyId: parseInt(values[Constants.IDENTIFY_ID_INDEX], 32),
163+
sequenceNumber: parseInt(values[Constants.SEQUENCE_NUMBER_INDEX], 32),
149164
};
150165
}
151166
}

test/base-cookie.js

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,5 +105,50 @@ describe('cookie', function () {
105105
spyLogWarning.restore();
106106
});
107107
});
108+
109+
describe('getLastEventTime tests', () => {
110+
it('should return 0 if cookie is undefined', () => {
111+
const cookieStr = undefined;
112+
const lastEventTime = cookie.getLastEventTime(cookieStr);
113+
assert.equal(lastEventTime, 0);
114+
});
115+
116+
it('should return 0 if cookie is an empty string', () => {
117+
const cookieStr = '';
118+
const lastEventTime = cookie.getLastEventTime(cookieStr);
119+
assert.equal(lastEventTime, 0);
120+
});
121+
122+
it('should return 0 if cookie is a malformed cookie', () => {
123+
const cookieStr = 'asdfasdfasdfasdf';
124+
const lastEventTime = cookie.getLastEventTime(cookieStr);
125+
assert.equal(lastEventTime, 0);
126+
});
127+
128+
it('should return a number thats base 32 encoded and put into the amplitude cookie format', () => {
129+
const originalTime = 1620698180822;
130+
const cookieStr = `....${originalTime.toString(32)}...`;
131+
const lastEventTime = cookie.getLastEventTime(cookieStr);
132+
assert.equal(lastEventTime, originalTime);
133+
});
134+
});
135+
136+
describe('sortByEventTime tests', () => {
137+
it('should sort cookies by last event time from greatest to least', () => {
138+
const firstTime = 10;
139+
const secondTime = 20;
140+
const thirdTime = 30;
141+
const invalidTime = '';
142+
143+
const cookieArray = [secondTime, invalidTime, thirdTime, firstTime].map((t) => `....${t.toString(32)}...`);
144+
const sortedCookieArray = cookie.sortByEventTime(cookieArray);
145+
146+
assert.notEqual(cookieArray, sortedCookieArray); // returns a shallow copy, not the same array
147+
assert.equal(sortedCookieArray[0], cookieArray[2]); // third time
148+
assert.equal(sortedCookieArray[1], cookieArray[0]); // second time
149+
assert.equal(sortedCookieArray[2], cookieArray[3]); // first time
150+
assert.equal(sortedCookieArray[3], cookieArray[1]); // invalid time
151+
});
152+
});
108153
});
109154
});

0 commit comments

Comments
 (0)