Skip to content

Commit

Permalink
Serialize new session cookie synchronously to avoid overlapping sessi…
Browse files Browse the repository at this point in the history
…ons (close #1381)
  • Loading branch information
matus-tomlein committed Nov 18, 2024
1 parent e835323 commit 270cad3
Show file tree
Hide file tree
Showing 12 changed files with 154 additions and 18 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@snowplow/browser-tracker-core",
"comment": "Serialize new session cookie synchronously to avoid overlapping sessions (#1381)",
"type": "none"
}
],
"packageName": "@snowplow/browser-tracker-core"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@snowplow/javascript-tracker",
"comment": "Serialize new session cookie synchronously to avoid overlapping sessions (#1381)",
"type": "none"
}
],
"packageName": "@snowplow/javascript-tracker"
}
16 changes: 8 additions & 8 deletions common/config/rush/pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion common/config/rush/repo-state.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// DO NOT MODIFY THIS FILE MANUALLY BUT DO COMMIT IT. It is generated and used by Rush.
{
"pnpmShrinkwrapHash": "6693fc661ad5b6461d8a0fbbecf81c5f61d703bb",
"pnpmShrinkwrapHash": "77e8d084b2ff087ff8f5dfa5df436f42dbaa623c",
"preferredVersionsHash": "bf21a9e8fbc5a3846fb05b4fa0859e0917b2202f"
}
13 changes: 7 additions & 6 deletions libraries/browser-tracker-core/src/tracker/cookie_storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,18 @@ export interface CookieStorage {
* @param secure - Boolean to specify if cookie should be secure
*/
deleteCookie(name: string, path?: string, domainName?: string, sameSite?: string, secure?: boolean): void;

/**
* Write all pending cookies.
*/
flush(): void;
}

export interface AsyncCookieStorage extends CookieStorage {
/**
* Clear the cookie storage cache (does not delete any cookies)
*/
clearCache(): void;

/**
* Write all pending cookies.
*/
flush(): void;
}

interface Cookie {
Expand Down Expand Up @@ -203,5 +203,6 @@ export const syncCookieStorage: CookieStorage = {
cookie(name, value, ttl, path, domain, samesite, secure);
return document.cookie.indexOf(`${name}=`) !== -1;
},
deleteCookie
deleteCookie,
flush: () => {},
};
12 changes: 12 additions & 0 deletions libraries/browser-tracker-core/src/tracker/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,11 @@ export function Tracker(
// Update currentVisitTs
updateNowTsInIdCookie(idCookie);
setDomainUserIdCookie(idCookie);

if (!eventIndexFromIdCookie(idCookie)) {
// Synchronously update the cookies to persist the new session ASAP
cookieStorage.flush();
}
}
}

Expand Down Expand Up @@ -920,6 +925,9 @@ export function Tracker(
onSessionUpdateCallback &&
!manualSessionUpdateCalled
) {
// Synchronously update the cookies to persist the new session ASAP
cookieStorage.flush();

onSessionUpdateCallback(clientSession);
manualSessionUpdateCalled = false;
}
Expand Down Expand Up @@ -969,6 +977,10 @@ export function Tracker(
const clientSession = clientSessionFromIdCookie(idCookie, configStateStorageStrategy, configAnonymousTracking);
setDomainUserIdCookie(idCookie);
const sessionIdentifierPersisted = setSessionCookie();

// Synchronously update the cookies to persist the new session ASAP
cookieStorage.flush();

if (sessionIdentifierPersisted && onSessionUpdateCallback) {
manualSessionUpdateCalled = true;
onSessionUpdateCallback(clientSession);
Expand Down
8 changes: 6 additions & 2 deletions libraries/browser-tracker-core/test/helpers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,12 @@ export function createTestSessionIdCookie(params?: CreateTestSessionIdCookie) {
return `_sp_ses.${domainHash}=*; Expires=; Path=/; SameSite=Lax; Secure;`;
}

export function createTracker(configuration?: TrackerConfiguration, sharedState?: SharedState) {
export function createTracker(
configuration?: TrackerConfiguration,
sharedState?: SharedState,
syncCookieWrite: boolean = true
) {
let id = 'sp-' + Math.random();
configuration = { ...configuration, synchronousCookieWrite: true };
configuration = { ...configuration, synchronousCookieWrite: syncCookieWrite };
return addTracker(id, id, '', '', sharedState ?? new SharedState(), configuration);
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,15 @@ describe('Tracker API: ', () => {
jest.clearAllMocks();
});

it('Writes cookies synchronously on session change', () => {
const tracker = createTracker(undefined, undefined, false); // async cookie writes enabled

expect(cookieJar).toContain('_sp_ses');

tracker?.newSession();
expect(cookieJar).toContain(tracker?.getDomainUserInfo().slice(1).join('.'));
});

it('Sets initial domain session index on first session', () => {
const tracker = createTracker();

Expand Down
2 changes: 1 addition & 1 deletion trackers/javascript-tracker/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@
"@wdio/static-server-service": "~8.39.0",
"@wdio/types": "~8.39.0",
"chalk": "4.1.2",
"chromedriver": "~129.0.0",
"chromedriver": "~131.0.0",
"dockerode": "~3.3.1",
"jest": "~27.5.1",
"jest-environment-jsdom": "~27.5.1",
Expand Down
23 changes: 23 additions & 0 deletions trackers/javascript-tracker/test/integration/cookies.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { fetchResults } from '../micro';
import { pageSetup } from './helpers';

describe('Tracker created domain cookies across multiple pages', () => {
let log: Array<unknown> = [];
let testIdentifier = '';

beforeAll(async () => {
testIdentifier = await pageSetup();
await browser.url('/multi_page_cookies');
await browser.pause(5000);

log = await browser.call(async () => await fetchResults());
});

it('all events should have the same session id', () => {
expect(log.length).toBeGreaterThanOrEqual(15);

const sessionIds = log.map((ev) => (ev as any).event.domain_sessionid);
const uniqueSessionIds = Array.from(new Set(sessionIds));
expect(uniqueSessionIds.length).toBe(1);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<!DOCTYPE html>
<html>
<head>
<title>Cookies iframe test page</title>
</head>
<body>
<div id="init"></div>
<div id="cookies"></div>
<script>
(function (p, l, o, w, i, n, g) {
if (!p[i]) {
p.GlobalSnowplowNamespace = p.GlobalSnowplowNamespace || [];
p.GlobalSnowplowNamespace.push(i);
p[i] = function () {
(p[i].q = p[i].q || []).push(arguments);
};
p[i].q = p[i].q || [];
n = l.createElement(o);
g = l.getElementsByTagName(o)[0];
n.async = 1;
n.src = w;
g.parentNode.insertBefore(n, g);
}
})(window, document, 'script', '../snowplow.js', 'snowplow');

const urlParams = new URLSearchParams(window.location.search);

var testIdentifier = document.cookie.split('testIdentifier=')[1].split(';')[0].trim();
var collector_endpoint = document.cookie.split('container=')[1].split(';')[0];

snowplow('newTracker', 'sp0', collector_endpoint, {
appId: 'cookies-iframe-' + testIdentifier,
cookieName: urlParams.get('cookieName'),
cookieSecure: false,
});
snowplow('trackPageView');

snowplow(function () {
document.getElementById('init').innerText = 'true';
});

setTimeout(function () {
document.getElementById('cookies').innerText = document.cookie;
}, 3000); // Wait 3 seconds so sp3 cookies expires
</script>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<!DOCTYPE html>
<html>
<head>
<title>Cookies test page</title>
</head>
<body>
<script>
const cookieId = Math.random().toString(36);

for (let i = 0; i < 15; i++) {
const iframe = document.createElement('iframe');
iframe.src = `/multi_page_cookies/iframe.html?cookieName=_sp_${cookieId}`;
iframe.width = "600";
iframe.height = "400";
iframe.style.margin = "10px";
document.body.appendChild(iframe);
}
</script>
</body>
</html>

0 comments on commit 270cad3

Please sign in to comment.