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

Serialize new session cookie synchronously to avoid overlapping sessions (close #1381) #1382

Merged
merged 1 commit into from
Nov 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
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
24 changes: 24 additions & 0 deletions trackers/javascript-tracker/test/integration/cookies.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
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());
log = log.filter((ev) => (ev as any).event.app_id === 'cookies-iframe-' + testIdentifier);
});

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,45 @@
<!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');

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: testIdentifier,
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,21 @@
<!DOCTYPE html>
<html>

<head>
<title>Cookies test page</title>
</head>

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

</html>
Loading