diff --git a/packages/core/src/domain/session/oldCookiesMigration.spec.ts b/packages/core/src/domain/session/oldCookiesMigration.spec.ts deleted file mode 100644 index 1404733549..0000000000 --- a/packages/core/src/domain/session/oldCookiesMigration.spec.ts +++ /dev/null @@ -1,76 +0,0 @@ -import { getCookie, resetInitCookies, setCookie } from '../../browser/cookie' -import { getSessionState } from '../../../test' -import type { Configuration } from '../configuration' -import { - OLD_LOGS_COOKIE_NAME, - OLD_RUM_COOKIE_NAME, - OLD_SESSION_COOKIE_NAME, - tryOldCookiesMigration, -} from './oldCookiesMigration' -import { SESSION_EXPIRATION_DELAY } from './sessionConstants' -import { initCookieStrategy } from './storeStrategies/sessionInCookie' -import type { SessionStoreStrategy } from './storeStrategies/sessionStoreStrategy' -import { SESSION_STORE_KEY } from './storeStrategies/sessionStoreStrategy' -const DEFAULT_INIT_CONFIGURATION = { trackAnonymousUser: true } as Configuration - -describe('old cookies migration', () => { - let sessionStoreStrategy: SessionStoreStrategy - - beforeEach(() => { - sessionStoreStrategy = initCookieStrategy(DEFAULT_INIT_CONFIGURATION, {}) - resetInitCookies() - }) - - afterEach(() => { - resetInitCookies() - }) - - it('should not touch current cookie', () => { - setCookie(SESSION_STORE_KEY, 'id=abcde&rum=0&logs=1&expire=1234567890', SESSION_EXPIRATION_DELAY) - - tryOldCookiesMigration(sessionStoreStrategy) - - expect(getCookie(SESSION_STORE_KEY)).toBe('id=abcde&rum=0&logs=1&expire=1234567890') - }) - - it('should create new cookie from old cookie values', () => { - setCookie(OLD_SESSION_COOKIE_NAME, 'abcde', SESSION_EXPIRATION_DELAY) - setCookie(OLD_LOGS_COOKIE_NAME, '1', SESSION_EXPIRATION_DELAY) - setCookie(OLD_RUM_COOKIE_NAME, '0', SESSION_EXPIRATION_DELAY) - - tryOldCookiesMigration(sessionStoreStrategy) - - expect(getSessionState(SESSION_STORE_KEY).id).toBe('abcde') - expect(getSessionState(SESSION_STORE_KEY).rum).toBe('0') - expect(getSessionState(SESSION_STORE_KEY).logs).toBe('1') - expect(getSessionState(SESSION_STORE_KEY).expire).toMatch(/\d+/) - }) - - it('should create new cookie from a single old cookie', () => { - setCookie(OLD_RUM_COOKIE_NAME, '0', SESSION_EXPIRATION_DELAY) - - tryOldCookiesMigration(sessionStoreStrategy) - expect(getSessionState(SESSION_STORE_KEY).id).not.toBeDefined() - expect(getSessionState(SESSION_STORE_KEY).rum).toBe('0') - expect(getSessionState(SESSION_STORE_KEY).expire).toMatch(/\d+/) - }) - - it('should not create a new cookie if no old cookie is present', () => { - tryOldCookiesMigration(sessionStoreStrategy) - expect(getCookie(SESSION_STORE_KEY)).toBeUndefined() - }) - - it('should behave correctly when performing the migration multiple times', () => { - setCookie(OLD_SESSION_COOKIE_NAME, 'abcde', SESSION_EXPIRATION_DELAY) - setCookie(OLD_LOGS_COOKIE_NAME, '1', SESSION_EXPIRATION_DELAY) - setCookie(OLD_RUM_COOKIE_NAME, '0', SESSION_EXPIRATION_DELAY) - - tryOldCookiesMigration(sessionStoreStrategy) - tryOldCookiesMigration(sessionStoreStrategy) - - expect(getSessionState(SESSION_STORE_KEY).id).toBe('abcde') - expect(getSessionState(SESSION_STORE_KEY).rum).toBe('0') - expect(getSessionState(SESSION_STORE_KEY).logs).toBe('1') - expect(getSessionState(SESSION_STORE_KEY).expire).toMatch(/\d+/) - }) -}) diff --git a/packages/core/src/domain/session/oldCookiesMigration.ts b/packages/core/src/domain/session/oldCookiesMigration.ts deleted file mode 100644 index da41b75f5a..0000000000 --- a/packages/core/src/domain/session/oldCookiesMigration.ts +++ /dev/null @@ -1,42 +0,0 @@ -import { getInitCookie } from '../../browser/cookie' -import type { SessionStoreStrategy } from './storeStrategies/sessionStoreStrategy' -import { SESSION_STORE_KEY } from './storeStrategies/sessionStoreStrategy' -import type { SessionState } from './sessionState' -import { expandSessionState, isSessionStarted } from './sessionState' - -export const OLD_SESSION_COOKIE_NAME = '_dd' -export const OLD_RUM_COOKIE_NAME = '_dd_r' -export const OLD_LOGS_COOKIE_NAME = '_dd_l' - -// duplicate values to avoid dependency issues -export const RUM_SESSION_KEY = 'rum' -export const LOGS_SESSION_KEY = 'logs' - -/** - * This migration should remain in the codebase as long as older versions are available/live - * to allow older sdk versions to be upgraded to newer versions without compatibility issues. - */ -export function tryOldCookiesMigration(cookieStoreStrategy: SessionStoreStrategy) { - const sessionString = getInitCookie(SESSION_STORE_KEY) - if (!sessionString) { - const oldSessionId = getInitCookie(OLD_SESSION_COOKIE_NAME) - const oldRumType = getInitCookie(OLD_RUM_COOKIE_NAME) - const oldLogsType = getInitCookie(OLD_LOGS_COOKIE_NAME) - const session: SessionState = {} - - if (oldSessionId) { - session.id = oldSessionId - } - if (oldLogsType && /^[01]$/.test(oldLogsType)) { - session[LOGS_SESSION_KEY] = oldLogsType - } - if (oldRumType && /^[012]$/.test(oldRumType)) { - session[RUM_SESSION_KEY] = oldRumType - } - - if (isSessionStarted(session)) { - expandSessionState(session) - cookieStoreStrategy.persistSession(session) - } - } -} diff --git a/packages/core/src/domain/session/sessionManager.spec.ts b/packages/core/src/domain/session/sessionManager.spec.ts index 345502f1d6..d709e3b54f 100644 --- a/packages/core/src/domain/session/sessionManager.spec.ts +++ b/packages/core/src/domain/session/sessionManager.spec.ts @@ -104,7 +104,11 @@ describe('startSessionManager', () => { describe('resume from a frozen tab ', () => { it('when session in store, do nothing', () => { - setCookie(SESSION_STORE_KEY, 'id=abcdef&first=tracked', DURATION) + setCookie( + SESSION_STORE_KEY, + `id=abcdef&first=tracked&created=${Date.now()}&expire=${Date.now() + DURATION}`, + DURATION + ) const sessionManager = startSessionManagerWithDefaults() window.dispatchEvent(createNewEvent(DOM_EVENT.RESUME)) @@ -142,8 +146,22 @@ describe('startSessionManager', () => { expectTrackingTypeToBe(sessionManager, FIRST_PRODUCT_KEY, FakeTrackingType.NOT_TRACKED) }) + it('should stamp a creation date on a not-tracked session too', () => { + // The creation date is what caps a session at SESSION_TIME_OUT_DELAY. Without one, the + // visibility timer keeps pushing `expire` forward and a sampled-out session on a page that + // stays open never expires -- so those users never get to re-roll the sampling decision. + startSessionManagerWithDefaults({ computeSessionState: () => NOT_TRACKED_SESSION_STATE }) + + expect(getSessionState(SESSION_STORE_KEY).id).toBeUndefined() + expect(getSessionState(SESSION_STORE_KEY).created).toMatch(/^\d+$/) + }) + it('when tracked should keep existing tracking type and session id', () => { - setCookie(SESSION_STORE_KEY, 'id=abcdef&first=tracked', DURATION) + setCookie( + SESSION_STORE_KEY, + `id=abcdef&first=tracked&created=${Date.now()}&expire=${Date.now() + DURATION}`, + DURATION + ) const sessionManager = startSessionManagerWithDefaults() @@ -152,7 +170,7 @@ describe('startSessionManager', () => { }) it('when not tracked should keep existing tracking type', () => { - setCookie(SESSION_STORE_KEY, 'first=not-tracked', DURATION) + setCookie(SESSION_STORE_KEY, `first=not-tracked&expire=${Date.now() + DURATION}`, DURATION) const sessionManager = startSessionManagerWithDefaults({ computeSessionState: () => NOT_TRACKED_SESSION_STATE }) @@ -174,19 +192,19 @@ describe('startSessionManager', () => { }) it('should be called with an invalid value if the cookie has an invalid value', () => { - setCookie(SESSION_STORE_KEY, 'first=invalid', DURATION) + setCookie(SESSION_STORE_KEY, `first=invalid&expire=${Date.now() + DURATION}`, DURATION) startSessionManagerWithDefaults({ computeSessionState: spy }) expect(spy).toHaveBeenCalledWith('invalid') }) it('should be called with TRACKED', () => { - setCookie(SESSION_STORE_KEY, 'first=tracked', DURATION) + setCookie(SESSION_STORE_KEY, `first=tracked&expire=${Date.now() + DURATION}`, DURATION) startSessionManagerWithDefaults({ computeSessionState: spy }) expect(spy).toHaveBeenCalledWith(FakeTrackingType.TRACKED) }) it('should be called with NOT_TRACKED', () => { - setCookie(SESSION_STORE_KEY, 'first=not-tracked', DURATION) + setCookie(SESSION_STORE_KEY, `first=not-tracked&expire=${Date.now() + DURATION}`, DURATION) startSessionManagerWithDefaults({ computeSessionState: spy }) expect(spy).toHaveBeenCalledWith(FakeTrackingType.NOT_TRACKED) }) @@ -336,7 +354,13 @@ describe('startSessionManager', () => { }) it('should renew an existing timed out session', () => { - setCookie(SESSION_STORE_KEY, `id=abcde&first=tracked&created=${Date.now() - SESSION_TIME_OUT_DELAY}`, DURATION) + // `expire` is still ahead, so the creation cap is what has to end this session -- without it + // the session would expire for the mundane reason of having no deadline at all. + setCookie( + SESSION_STORE_KEY, + `id=abcde&first=tracked&created=${Date.now() - SESSION_TIME_OUT_DELAY}&expire=${Date.now() + SESSION_EXPIRATION_DELAY}`, + DURATION + ) const sessionManager = startSessionManagerWithDefaults() const expireSessionSpy = jasmine.createSpy() @@ -347,13 +371,17 @@ describe('startSessionManager', () => { expect(expireSessionSpy).not.toHaveBeenCalled() // the session has not been active from the start }) - it('should not add created date to an existing session from an older versions', () => { + it('should not adopt a stored session that carries no creation date', () => { + // Written by a version that did not stamp `created`. Such a session cannot be shown to sit + // inside SESSION_TIME_OUT_DELAY, so it is renewed rather than trusted -- adopting it is how + // a session stayed alive for days on a page that was never closed. Every release of this + // SDK stamps `created`, so nothing we ship produces this state. setCookie(SESSION_STORE_KEY, 'id=abcde&first=tracked', DURATION) const sessionManager = startSessionManagerWithDefaults() - expect(sessionManager.findSession()!.id).toBe('abcde') - expect(getSessionState(SESSION_STORE_KEY).created).toBeUndefined() + expect(sessionManager.findSession()!.id).not.toBe('abcde') + expect(getSessionState(SESSION_STORE_KEY).created).toBeDefined() }) }) diff --git a/packages/core/src/domain/session/sessionState.spec.ts b/packages/core/src/domain/session/sessionState.spec.ts index 1fe4fa4c12..525d90936a 100644 --- a/packages/core/src/domain/session/sessionState.spec.ts +++ b/packages/core/src/domain/session/sessionState.spec.ts @@ -1,8 +1,9 @@ import { dateNow } from '../../tools/utils/timeUtils' -import { SESSION_EXPIRATION_DELAY } from './sessionConstants' +import { SESSION_EXPIRATION_DELAY, SESSION_TIME_OUT_DELAY } from './sessionConstants' import type { SessionState } from './sessionState' import { expandSessionState, + getExpireDate, isSessionInExpiredState, toSessionString, toSessionState, @@ -29,22 +30,82 @@ describe('session state utilities', () => { }) describe('isSessionInExpiredState', () => { + const ONE_DAY = 24 * 60 * 60 * 1000 + function dateNowWithOffset(offset = 0) { return String(dateNow() + offset) } it('should correctly identify a session in expired state', () => { expect(isSessionInExpiredState(EXPIRED_SESSION)).toBe(true) - expect(isSessionInExpiredState({ created: dateNowWithOffset(-1000 * 60 * 60 * 4) })).toBe(true) - expect(isSessionInExpiredState({ expire: dateNowWithOffset(-100) })).toBe(true) + expect( + isSessionInExpiredState({ + created: dateNowWithOffset(-SESSION_TIME_OUT_DELAY), + expire: dateNowWithOffset(1000), + }) + ).toBe(true) + expect(isSessionInExpiredState({ created: dateNowWithOffset(-100), expire: dateNowWithOffset(-100) })).toBe(true) + }) + + it('should expire a session that cannot say when it started or when it lapses', () => { + // A missing stamp used to short-circuit the comparison to `true`. See getExpireDate. + expect(isSessionInExpiredState({ first: 'not-tracked' })).toBe(true) + expect(isSessionInExpiredState({ first: 'tracked' })).toBe(true) + expect(isSessionInExpiredState({ id: '123', first: 'tracked', expire: dateNowWithOffset(1000) })).toBe(true) + expect(isSessionInExpiredState({ id: '123', first: 'tracked', created: dateNowWithOffset(-1000) })).toBe(true) + }) + + it('should cap the sliding deadline at SESSION_TIME_OUT_DELAY from creation', () => { + // An `expire` beyond the cap can only come from a clock that was ahead when it was written, + // and would otherwise hold the session open until that error had elapsed for real. + expect( + isSessionInExpiredState({ + created: dateNowWithOffset(-SESSION_TIME_OUT_DELAY), + expire: dateNowWithOffset(ONE_DAY), + }) + ).toBe(true) }) it('should correctly identify a session in live state', () => { expect(isSessionInExpiredState({ created: dateNowWithOffset(-1000), expire: dateNowWithOffset(1000) })).toBe( false ) - expect(isSessionInExpiredState({ first: 'not-tracked' })).toBe(false) - expect(isSessionInExpiredState({ first: 'tracked' })).toBe(false) + }) + + it('should not consider a session that was never started as expired', () => { + expect(isSessionInExpiredState(NOT_STARTED_SESSION)).toBe(false) + }) + }) + + describe('getExpireDate', () => { + function dateNowWithOffset(offset = 0) { + return String(dateNow() + offset) + } + + it('should return undefined without an expire stamp', () => { + expect(getExpireDate({})).toBeUndefined() + expect(getExpireDate({ created: dateNowWithOffset(-1000) })).toBeUndefined() + }) + + it('should return undefined when a session holding an id has no creation date', () => { + expect(getExpireDate({ id: '123', expire: dateNowWithOffset(1000) })).toBeUndefined() + }) + + it('should fall back to expire alone for a not-tracked session, which is never stamped', () => { + const expire = dateNowWithOffset(1000) + expect(getExpireDate({ first: 'not-tracked', expire })).toBe(Number(expire)) + }) + + it('should return the sliding deadline while it is the earlier of the two', () => { + const expire = dateNowWithOffset(1000) + expect(getExpireDate({ created: dateNowWithOffset(-1000), expire })).toBe(Number(expire)) + }) + + it('should return the creation cap once it is the earlier of the two', () => { + const created = dateNowWithOffset(-SESSION_TIME_OUT_DELAY + 1000) + expect(getExpireDate({ created, expire: dateNowWithOffset(SESSION_TIME_OUT_DELAY) })).toBe( + Number(created) + SESSION_TIME_OUT_DELAY + ) }) }) diff --git a/packages/core/src/domain/session/sessionState.ts b/packages/core/src/domain/session/sessionState.ts index 9734e138a3..4166f31b4b 100644 --- a/packages/core/src/domain/session/sessionState.ts +++ b/packages/core/src/domain/session/sessionState.ts @@ -41,18 +41,48 @@ export function isSessionStarted(session: SessionState) { return !isSessionInNotStartedState(session) } +/** + * The moment the session actually stops being usable: whichever comes first, the sliding + * inactivity deadline or the hard cap counted from when the session was created. + * + * Returns undefined when either stamp is missing, which callers treat as "expired". A session + * that cannot say when it started or when it lapses is not given the benefit of the doubt -- + * letting `undefined` short-circuit those comparisons is what allowed sessions to outlive both + * bounds and stay alive for days on a page that was never closed. + * + * Ported from upstream DataDog/browser-sdk 5257b52ea ("fix session lifetime bugs for long-lived + * pages and multi-tab scenarios", #4531); their SessionManager rewrite makes the commit itself + * unmergeable here, so only the rule is carried over. + */ +export function getExpireDate(state: SessionState): number | undefined { + const expireDate = state.expire && Number(state.expire) + if (!expireDate) { + return + } + const createdDate = state.created && Number(state.created) + if (createdDate) { + return Math.min(expireDate, createdDate + SESSION_TIME_OUT_DELAY) + } + // Every session this bundle starts is stamped, so a missing creation date means the state was + // written elsewhere: either by a build that predates the stamp, or by another bundle sharing the + // cookie. A state holding an id is judged strictly -- it cannot be shown to sit inside the cap. + // One without an id is not tracked, carries no identity, and falls back to `expire` alone rather + // than being expired on sight, which would make old and new builds fight over the same cookie. + return state.id === undefined ? expireDate : undefined +} + export function isSessionInExpiredState(session: SessionState) { + if (isSessionInNotStartedState(session)) { + // nothing has been stored yet, so there is no session to consider expired + return false + } return session.isExpired !== undefined || !isActiveSession(session) } // An active session is a session in either `Tracked` or `NotTracked` state function isActiveSession(sessionState: SessionState) { - // created and expire can be undefined for versions which was not storing them - // these checks could be removed when older versions will not be available/live anymore - return ( - (sessionState.created === undefined || dateNow() - Number(sessionState.created) < SESSION_TIME_OUT_DELAY) && - (sessionState.expire === undefined || dateNow() < Number(sessionState.expire)) - ) + const expireDate = getExpireDate(sessionState) + return expireDate ? dateNow() < expireDate : false } export function expandSessionState(session: SessionState) { diff --git a/packages/core/src/domain/session/sessionStore.ts b/packages/core/src/domain/session/sessionStore.ts index 4c0a1a74e5..c926e79410 100644 --- a/packages/core/src/domain/session/sessionStore.ts +++ b/packages/core/src/domain/session/sessionStore.ts @@ -9,6 +9,7 @@ import { selectCookieStrategy, initCookieStrategy } from './storeStrategies/sess import type { SessionStoreStrategyType } from './storeStrategies/sessionStoreStrategy' import { getExpiredSessionState, + expandSessionState, isSessionInExpiredState, isSessionInNotStartedState, isSessionStarted, @@ -181,8 +182,20 @@ export function startSessionStore( delete sessionState.isExpired if (isTracked && !sessionState.id) { sessionState.id = generateUUID() + } + // Stamp every started session, tracked or not. The creation date is what caps a session at + // SESSION_TIME_OUT_DELAY, and a not-tracked session that carried none would be held open + // indefinitely by the visibility timer refreshing `expire` -- the same escape this change + // closes for tracked sessions, left open for the sampled-out half. Without the cap those + // users never get to re-roll the sampling decision. + if (!sessionState.created) { sessionState.created = String(dateNow()) } + // Stamp the deadline before returning. The caller decides whether the session is expired + // straight after this runs and only stamps it afterwards, so a session renewed here would be + // judged with no deadline at all -- which now reads as expired and would stop any session + // from ever establishing. Upstream does the same inside its own expandOrRenew. + expandSessionState(sessionState) } function hasSessionInCache() { diff --git a/packages/core/src/domain/session/sessionStoreOperations.spec.ts b/packages/core/src/domain/session/sessionStoreOperations.spec.ts index 9235fc0bb9..78d76b6efe 100644 --- a/packages/core/src/domain/session/sessionStoreOperations.spec.ts +++ b/packages/core/src/domain/session/sessionStoreOperations.spec.ts @@ -5,6 +5,7 @@ import type { Configuration } from '../configuration' import { initCookieStrategy } from './storeStrategies/sessionInCookie' import { initLocalStorageStrategy } from './storeStrategies/sessionInLocalStorage' import type { SessionState } from './sessionState' +import { SESSION_EXPIRATION_DELAY } from './sessionConstants' import { expandSessionState, toSessionString } from './sessionState' import { processSessionStoreOperations, LOCK_MAX_TRIES, LOCK_RETRY_DELAY } from './sessionStoreOperations' import { SESSION_STORE_KEY } from './storeStrategies/sessionStoreStrategy' @@ -39,8 +40,9 @@ const DEFAULT_INIT_CONFIGURATION = { trackAnonymousUser: true } as Configuration beforeEach(() => { sessionStoreStrategy.expireSession(initialSession) - initialSession = { id: '123', created: String(now) } - otherSession = { id: '456', created: String(now + 100) } + // Both stamps are required for a session to read as live, so the fixtures carry them. + initialSession = { id: '123', created: String(now), expire: String(now + SESSION_EXPIRATION_DELAY) } + otherSession = { id: '456', created: String(now + 100), expire: String(now + SESSION_EXPIRATION_DELAY) } processSpy = jasmine.createSpy('process') afterSpy = jasmine.createSpy('after') storage = mockStorage() diff --git a/packages/core/src/domain/session/storeStrategies/sessionInCookie.ts b/packages/core/src/domain/session/storeStrategies/sessionInCookie.ts index 2c4319e9a3..a6c2736e62 100644 --- a/packages/core/src/domain/session/storeStrategies/sessionInCookie.ts +++ b/packages/core/src/domain/session/storeStrategies/sessionInCookie.ts @@ -2,7 +2,6 @@ import { isChromium } from '../../../tools/utils/browserDetection' import type { CookieOptions } from '../../../browser/cookie' import { getCurrentSite, areCookiesAuthorized, getCookie, setCookie } from '../../../browser/cookie' import type { InitConfiguration, Configuration } from '../../configuration' -import { tryOldCookiesMigration } from '../oldCookiesMigration' import { SESSION_COOKIE_EXPIRATION_DELAY, SESSION_EXPIRATION_DELAY, @@ -38,8 +37,6 @@ export function initCookieStrategy(configuration: Configuration, cookieOptions: ), } - tryOldCookiesMigration(cookieStore) - return cookieStore } diff --git a/packages/logs/src/boot/startLogs.spec.ts b/packages/logs/src/boot/startLogs.spec.ts index 2b112aee69..e48fb5cc4d 100644 --- a/packages/logs/src/boot/startLogs.spec.ts +++ b/packages/logs/src/boot/startLogs.spec.ts @@ -264,7 +264,7 @@ describe('logs', () => { }) it('sends logs without session id when the session expires ', async () => { - setCookie(SESSION_STORE_KEY, 'id=foo&logs=1', ONE_MINUTE) + setCookie(SESSION_STORE_KEY, `id=foo&logs=1&created=${Date.now()}&expire=${Date.now() + ONE_MINUTE}`, ONE_MINUTE) ;({ handleLog, stop: stopLogs } = startLogs( initConfiguration, baseConfiguration, diff --git a/packages/logs/src/domain/logsSessionManager.spec.ts b/packages/logs/src/domain/logsSessionManager.spec.ts index e19ce9176b..54ff4a51d7 100644 --- a/packages/logs/src/domain/logsSessionManager.spec.ts +++ b/packages/logs/src/domain/logsSessionManager.spec.ts @@ -53,7 +53,7 @@ describe('logs session manager', () => { }) it('when tracked should keep existing tracking type and session id', () => { - setCookie(SESSION_STORE_KEY, 'id=abcdef&logs=1', DURATION) + setCookie(SESSION_STORE_KEY, `id=abcdef&logs=1&created=${Date.now()}&expire=${Date.now() + DURATION}`, DURATION) startLogsSessionManagerWithDefaults() @@ -62,7 +62,7 @@ describe('logs session manager', () => { }) it('when not tracked should keep existing tracking type', () => { - setCookie(SESSION_STORE_KEY, 'logs=0', DURATION) + setCookie(SESSION_STORE_KEY, `logs=0&expire=${Date.now() + DURATION}`, DURATION) startLogsSessionManagerWithDefaults() @@ -84,19 +84,19 @@ describe('logs session manager', () => { describe('findTrackedSession', () => { it('should return the current active session', () => { - setCookie(SESSION_STORE_KEY, 'id=abcdef&logs=1', DURATION) + setCookie(SESSION_STORE_KEY, `id=abcdef&logs=1&created=${Date.now()}&expire=${Date.now() + DURATION}`, DURATION) const logsSessionManager = startLogsSessionManagerWithDefaults() expect(logsSessionManager.findTrackedSession()!.id).toBe('abcdef') }) it('should return undefined if the session is not tracked', () => { - setCookie(SESSION_STORE_KEY, 'id=abcdef&logs=0', DURATION) + setCookie(SESSION_STORE_KEY, `id=abcdef&logs=0&created=${Date.now()}&expire=${Date.now() + DURATION}`, DURATION) const logsSessionManager = startLogsSessionManagerWithDefaults() expect(logsSessionManager.findTrackedSession()).toBeUndefined() }) it('should not return the current session if it has expired by default', () => { - setCookie(SESSION_STORE_KEY, 'id=abcdef&logs=1', DURATION) + setCookie(SESSION_STORE_KEY, `id=abcdef&logs=1&created=${Date.now()}&expire=${Date.now() + DURATION}`, DURATION) const logsSessionManager = startLogsSessionManagerWithDefaults() clock.tick(10 * ONE_SECOND) expireCookie() @@ -112,10 +112,10 @@ describe('logs session manager', () => { }) it('should return session corresponding to start time', () => { - setCookie(SESSION_STORE_KEY, 'id=foo&logs=1', DURATION) + setCookie(SESSION_STORE_KEY, `id=foo&logs=1&created=${Date.now()}&expire=${Date.now() + DURATION}`, DURATION) const logsSessionManager = startLogsSessionManagerWithDefaults() clock.tick(10 * ONE_SECOND) - setCookie(SESSION_STORE_KEY, 'id=bar&logs=1', DURATION) + setCookie(SESSION_STORE_KEY, `id=bar&logs=1&created=${Date.now()}&expire=${Date.now() + DURATION}`, DURATION) // simulate a click to renew the session document.dispatchEvent(createNewEvent(DOM_EVENT.CLICK)) clock.tick(STORAGE_POLL_DELAY) diff --git a/packages/rum-core/src/domain/rumSessionManager.spec.ts b/packages/rum-core/src/domain/rumSessionManager.spec.ts index 95fa26abd6..dc82b52f8e 100644 --- a/packages/rum-core/src/domain/rumSessionManager.spec.ts +++ b/packages/rum-core/src/domain/rumSessionManager.spec.ts @@ -87,7 +87,7 @@ describe('rum session manager', () => { }) it('when tracked should keep existing session type and id', () => { - setCookie(SESSION_STORE_KEY, 'id=abcdef&rum=1', DURATION) + setCookie(SESSION_STORE_KEY, `id=abcdef&rum=1&created=${Date.now()}&expire=${Date.now() + DURATION}`, DURATION) startRumSessionManagerWithDefaults() @@ -98,7 +98,7 @@ describe('rum session manager', () => { }) it('when not tracked should keep existing session type', () => { - setCookie(SESSION_STORE_KEY, 'rum=0', DURATION) + setCookie(SESSION_STORE_KEY, `rum=0&expire=${Date.now() + DURATION}`, DURATION) startRumSessionManagerWithDefaults() @@ -108,7 +108,7 @@ describe('rum session manager', () => { }) it('should renew on activity after expiration', () => { - setCookie(SESSION_STORE_KEY, 'id=abcdef&rum=1', DURATION) + setCookie(SESSION_STORE_KEY, `id=abcdef&rum=1&created=${Date.now()}&expire=${Date.now() + DURATION}`, DURATION) startRumSessionManagerWithDefaults({ configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 100 } }) @@ -129,13 +129,13 @@ describe('rum session manager', () => { describe('findSession', () => { it('should return the current session', () => { - setCookie(SESSION_STORE_KEY, 'id=abcdef&rum=1', DURATION) + setCookie(SESSION_STORE_KEY, `id=abcdef&rum=1&created=${Date.now()}&expire=${Date.now() + DURATION}`, DURATION) const rumSessionManager = startRumSessionManagerWithDefaults() expect(rumSessionManager.findTrackedSession()!.id).toBe('abcdef') }) it('should return undefined if the session is not tracked', () => { - setCookie(SESSION_STORE_KEY, 'id=abcdef&rum=0', DURATION) + setCookie(SESSION_STORE_KEY, `id=abcdef&rum=0&created=${Date.now()}&expire=${Date.now() + DURATION}`, DURATION) const rumSessionManager = startRumSessionManagerWithDefaults() expect(rumSessionManager.findTrackedSession()).toBe(undefined) }) @@ -148,7 +148,7 @@ describe('rum session manager', () => { }) it('should return session corresponding to start time', () => { - setCookie(SESSION_STORE_KEY, 'id=abcdef&rum=1', DURATION) + setCookie(SESSION_STORE_KEY, `id=abcdef&rum=1&created=${Date.now()}&expire=${Date.now() + DURATION}`, DURATION) const rumSessionManager = startRumSessionManagerWithDefaults() clock.tick(10 * ONE_SECOND) expireCookie() @@ -158,19 +158,19 @@ describe('rum session manager', () => { }) it('should return session TRACKED_WITH_SESSION_REPLAY', () => { - setCookie(SESSION_STORE_KEY, 'id=abcdef&rum=1', DURATION) + setCookie(SESSION_STORE_KEY, `id=abcdef&rum=1&created=${Date.now()}&expire=${Date.now() + DURATION}`, DURATION) const rumSessionManager = startRumSessionManagerWithDefaults() expect(rumSessionManager.findTrackedSession()!.sessionReplay).toBe(SessionReplayState.SAMPLED) }) it('should return session TRACKED_WITHOUT_SESSION_REPLAY', () => { - setCookie(SESSION_STORE_KEY, 'id=abcdef&rum=2', DURATION) + setCookie(SESSION_STORE_KEY, `id=abcdef&rum=2&created=${Date.now()}&expire=${Date.now() + DURATION}`, DURATION) const rumSessionManager = startRumSessionManagerWithDefaults() expect(rumSessionManager.findTrackedSession()!.sessionReplay).toBe(SessionReplayState.OFF) }) it('should update current entity when replay recording is forced', () => { - setCookie(SESSION_STORE_KEY, 'id=abcdef&rum=2', DURATION) + setCookie(SESSION_STORE_KEY, `id=abcdef&rum=2&created=${Date.now()}&expire=${Date.now() + DURATION}`, DURATION) const rumSessionManager = startRumSessionManagerWithDefaults() rumSessionManager.setForcedReplay() diff --git a/packages/rum-legacy/src/domain/sessionStore.spec.ts b/packages/rum-legacy/src/domain/sessionStore.spec.ts index 3f770365d3..fdeddb47a4 100644 --- a/packages/rum-legacy/src/domain/sessionStore.spec.ts +++ b/packages/rum-legacy/src/domain/sessionStore.spec.ts @@ -210,6 +210,31 @@ describe('session store', () => { expect(second.id).not.toBe(first.id) }) + it('does not adopt a session that carries no creation date', () => { + // Such a cookie has no cap to count from, so every access would push its deadline forward and + // the session would never end. The modern bundle stops trusting these too -- both builds share + // this cookie, so a difference here would let this one resurrect a session the other ended. + document.cookie = `${SESSION_COOKIE_NAME}=id=abcdef&rum=2&expire=${Date.now() + ONE_MINUTE};path=/` + + const session = createSessionStore(100).getOrCreateSession() + + expect(session.id).not.toBe('abcdef') + expect(readRawCookie()).toMatch(/created=\d+/) + }) + + it('stamps a creation date on a sampled-out session it adopts without one', () => { + // An id-less state is a legitimately sampled-out session, so it is adopted rather than redrawn. + // Adopting it without stamping would leave the cap nothing to count from, and every access + // pushes the deadline forward — the sampled-out user on a page that stays open would then + // never get to re-run the draw. + document.cookie = `${SESSION_COOKIE_NAME}=rum=0&expire=${Date.now() + ONE_MINUTE};path=/` + + createSessionStore(100).getOrCreateSession() + + expect(readRawCookie()).toMatch(/rum=0/) + expect(readRawCookie()).toMatch(/created=\d+/) + }) + describe('sampling', () => { it('tracks the session when the sample rate is 100', () => { expect(createSessionStore(100).getOrCreateSession().isTracked).toBe(true) diff --git a/packages/rum-legacy/src/domain/sessionStore.ts b/packages/rum-legacy/src/domain/sessionStore.ts index 4bc4c53b33..60a870a3a7 100644 --- a/packages/rum-legacy/src/domain/sessionStore.ts +++ b/packages/rum-legacy/src/domain/sessionStore.ts @@ -115,6 +115,13 @@ export function createSessionStore(sessionSampleRate: number) { state.id = generateUUID() } + // Stamp every session, tracked or not, so the cap above always has something to count from. + // A session adopted from a build that did not stamp would otherwise keep having its deadline + // pushed forward on every access and never reach the cap at all. + if (!state.created) { + state.created = String(now) + } + state.expire = String(now + SESSION_EXPIRATION_DELAY) inMemoryState = state lastCookieAccess = now @@ -158,10 +165,24 @@ function extractForeignFields(state: SessionState | undefined): SessionState { return kept } +/** + * Mirrors the modern bundle's rule (`core/domain/session/sessionState.ts`): a session lives until + * whichever comes first — its sliding deadline, or the cap counted from when it was created — and a + * state that cannot produce either is expired rather than assumed young. Both builds share `_dd_s`, + * so any difference here would let one build resurrect a session the other had already ended. + */ function isExpired(state: SessionState, now: number): boolean { - const createdAt = Number(state.created) - const expiresAt = Number(state.expire) - return (createdAt && now - createdAt >= SESSION_TIME_OUT_DELAY) || (expiresAt && now >= expiresAt) ? true : false + const expiresAt = state.expire && Number(state.expire) + if (!expiresAt || !isFinite(expiresAt)) { + return true + } + const createdAt = state.created && Number(state.created) + if (createdAt && isFinite(createdAt)) { + return now >= Math.min(expiresAt, createdAt + SESSION_TIME_OUT_DELAY) + } + // No creation date: written by a build that predates the stamp. A state holding an id cannot be + // shown to sit inside the cap; one without an id is not tracked and falls back to the deadline. + return state.id !== undefined || now >= expiresAt } /**