diff --git a/CHANGELOG.md b/CHANGELOG.md index 105cad12d3cd..3556a0de5431 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott -Work in this release was contributed by @psh4607, @thijsw, @trinitiwowka, @nehaprasad-dev, @JealousGx, @Jxxunnn, @eddie333016, @davidmurdoch, @yashschandra, @atharv-sys32, @AG0708, @birkskyum, @mkly, @mcbbugu, @suhailopensource, @zkasuran, @mohd-akram, @RealBhupesh, @halillusion, @psang39, @hafzism, @JosephDoUrden, @Tyagiquamar, @Andarist, @msnelling, @oesnuj, @chiliec, @ihsraham, and @matthewbjones. Thank you for your contributions! +Work in this release was contributed by @psh4607, @thijsw, @trinitiwowka, @nehaprasad-dev, @JealousGx, @Jxxunnn, @eddie333016, @davidmurdoch, @yashschandra, @atharv-sys32, @AG0708, @birkskyum, @mkly, @mcbbugu, @suhailopensource, @zkasuran, @mohd-akram, @RealBhupesh, @halillusion, @psang39, @hafzism, @JosephDoUrden, @Tyagiquamar, @Andarist, @msnelling, @oesnuj, @chiliec, @ihsraham, @Dextheking1, and @matthewbjones. Thank you for your contributions! - ref(browser)!: LCP and CLS spans no longer set `browser.web_vital.lcp.report_event` and `browser.web_vital.cls.report_event`. With per-navigation web vitals (the default) the attribute was already never set; it is now also gone when `softNavigations` and `bfcacheNavigations` are turned off. When the values are finalized is unchanged. - feat(browser)!: `browser.navigation.type` on web vital and bfcache navigation spans now carries the navigation type exactly as web-vitals reports it. `bfcache` is now `back-forward-cache`, and a back/forward navigation that missed the bfcache (`back-forward`) or a discarded-tab restore (`restore`) is no longer folded into `navigate`. Update any dashboards or alerts filtering on `bfcache`. diff --git a/packages/browser/src/integrations/httpclient.ts b/packages/browser/src/integrations/httpclient.ts index a47725b3d37d..723aad26bd4d 100644 --- a/packages/browser/src/integrations/httpclient.ts +++ b/packages/browser/src/integrations/httpclient.ts @@ -92,13 +92,11 @@ function _fetchResponseHandler( if (dc.cookies !== false) { const reqCookieStr = request.headers.get('Cookie') || undefined; if (reqCookieStr) { - const filtered = _INTERNAL_filterCookies(reqCookieStr, dc.cookies); - requestCookies = typeof filtered === 'string' ? { cookie: filtered } : filtered; + requestCookies = _INTERNAL_filterCookies(reqCookieStr, dc.cookies, 'cookie'); } const resCookieStr = response.headers.get('Set-Cookie') || undefined; if (resCookieStr) { - const filtered = _INTERNAL_filterCookies(resCookieStr, dc.cookies); - responseCookies = typeof filtered === 'string' ? { 'set-cookie': filtered } : filtered; + responseCookies = _INTERNAL_filterCookies(resCookieStr, dc.cookies, 'set-cookie'); } } @@ -141,8 +139,7 @@ function _xhrResponseHandler( try { const cookieString = xhr.getResponseHeader('Set-Cookie') || xhr.getResponseHeader('set-cookie') || undefined; if (cookieString) { - const filtered = _INTERNAL_filterCookies(cookieString, dc.cookies); - responseCookies = typeof filtered === 'string' ? { 'set-cookie': filtered } : filtered; + responseCookies = _INTERNAL_filterCookies(cookieString, dc.cookies, 'set-cookie'); } } catch { // ignore it if parsing fails diff --git a/packages/browser/test/integrations/httpclient.test.ts b/packages/browser/test/integrations/httpclient.test.ts index 651e51b24d86..cb5a928eefcb 100644 --- a/packages/browser/test/integrations/httpclient.test.ts +++ b/packages/browser/test/integrations/httpclient.test.ts @@ -146,7 +146,7 @@ describe('httpClientIntegration', () => { triggerFetch(fetchHandler, { requestHeaders: { Authorization: 'Bearer x', Accept: 'application/json', Cookie: 'theme=dark; session=secret' }, - responseHeaders: { 'Content-Type': 'text/html', 'Set-Cookie': 'locale=en; session=secret' }, + responseHeaders: { 'Content-Type': 'text/html', 'Set-Cookie': 'session=secret; Path=/; HttpOnly' }, }); expect(captureEventSpy).toHaveBeenCalledTimes(1); @@ -158,7 +158,7 @@ describe('httpClientIntegration', () => { }); expect(event.request?.cookies).toEqual({ theme: 'dark', session: '[Filtered]' }); expect(event.contexts?.response?.headers).toEqual({ 'content-type': 'text/html', 'set-cookie': '[Filtered]' }); - expect(event.contexts?.response?.cookies).toEqual({ locale: 'en', session: '[Filtered]' }); + expect(event.contexts?.response?.cookies).toEqual({ session: '[Filtered]' }); }); it('filters PII headers when an explicit deny list is configured', () => { @@ -244,14 +244,20 @@ describe('httpClientIntegration', () => { const { xhrHandler, captureEventSpy } = setup(); triggerXhr(xhrHandler, { - setCookie: 'session=abc123; theme=dark; connect.sid=secret', + setCookie: 'connect.sid=s3cr3t; Path=/; HttpOnly', }); - expect(getEvent(captureEventSpy).contexts?.response?.cookies).toEqual({ - session: '[Filtered]', - theme: 'dark', - 'connect.sid': '[Filtered]', + expect(getEvent(captureEventSpy).contexts?.response?.cookies).toEqual({ 'connect.sid': '[Filtered]' }); + }); + + it('does not report Set-Cookie attributes as response cookies', () => { + const { xhrHandler, captureEventSpy } = setup(); + + triggerXhr(xhrHandler, { + setCookie: 'theme=dark; Max-Age=3600; Path=/; Domain=example.com', }); + + expect(getEvent(captureEventSpy).contexts?.response?.cookies).toEqual({ theme: 'dark' }); }); it('collects response headers and filters response cookies by default', () => { @@ -259,7 +265,7 @@ describe('httpClientIntegration', () => { triggerXhr(xhrHandler, { requestHeaders: { Authorization: 'Bearer x' }, - setCookie: 'session=abc123; theme=dark', + setCookie: 'session=abc123; Path=/', allResponseHeaders: 'content-type: text/html', }); @@ -267,7 +273,7 @@ describe('httpClientIntegration', () => { const event = getEvent(captureEventSpy); expect(event.request?.headers).toEqual({ Authorization: '[Filtered]' }); expect(event.contexts?.response?.headers).toEqual({ 'content-type': 'text/html' }); - expect(event.contexts?.response?.cookies).toEqual({ session: '[Filtered]', theme: 'dark' }); + expect(event.contexts?.response?.cookies).toEqual({ session: '[Filtered]' }); }); }); }); diff --git a/packages/core/src/integrations/requestdata.ts b/packages/core/src/integrations/requestdata.ts index eda1603c809d..7f129857857d 100644 --- a/packages/core/src/integrations/requestdata.ts +++ b/packages/core/src/integrations/requestdata.ts @@ -7,12 +7,12 @@ import type { Event } from '../types/event'; import type { IntegrationFn } from '../types/integration'; import type { QueryParams, RequestEventData } from '../types/request'; import type { StreamedSpanJSON } from '../types/span'; -import { parseCookie } from '../utils/cookie'; +import { cookiePairsToRecord, parseCookieHeader } from '../utils/cookie'; import { SENSITIVE_COOKIE_NAME_SNIPPETS } from '../utils/data-collection/filtering-snippets'; import { filterKeyValueData } from '../utils/data-collection/filterKeyValueData'; import { filterQueryParams } from '../utils/data-collection/filterQueryParams'; import { filterUrlQuery } from '../utils/data-collection/filterUrlQuery'; -import { httpHeadersToSpanAttributes } from '../utils/request'; +import { filterCookiePairs, httpHeadersToSpanAttributes } from '../utils/request'; import { getUrlQuery } from '../utils/url'; import { getClientIPAddress, ipHeaderNames } from '../vendor/getIpAddress'; import { safeSetSpanJSONAttributes } from '../tracing/spans/captureSpan'; @@ -185,12 +185,20 @@ function addNormalizedRequestDataToSpan( // Process cookies before headers so normalizedRequest.cookies takes precedence // over the raw cookie header (matching the processEvent path). - if (requestData.cookies && Object.keys(requestData.cookies).length > 0) { - const cookieString = Object.entries(requestData.cookies) - .map(([name, value]) => `${name}=${value}`) - .join('; '); - const cookieAttributes = httpHeadersToSpanAttributes({ cookie: cookieString }, dataCollection, 'request'); - safeSetSpanJSONAttributes(span, cookieAttributes); + if (include.cookies) { + // Cookies are not serialized to a string and re-parsed: a decoded value could contain ";" and + // split into a second, differently named cookie that escapes the denylist. + const cookieHeader = normalizedRequest.headers?.cookie; + const cookiePairs = normalizedRequest.cookies + ? Object.entries(normalizedRequest.cookies) + : cookieHeader + ? parseCookieHeader(cookieHeader, 'cookie') + : []; + if (cookiePairs.length > 0) { + safeSetSpanJSONAttributes(span, { + 'http.request.header.cookie': filterCookiePairs(cookiePairs, dataCollection.cookies), + }); + } } if (requestData.headers) { @@ -245,7 +253,9 @@ function extractNormalizedRequestData( } if (include.cookies) { - const cookies = normalizedRequest.cookies || (headers?.cookie ? parseCookie(headers.cookie) : undefined); + const cookies = + normalizedRequest.cookies || + (headers?.cookie ? cookiePairsToRecord(parseCookieHeader(headers.cookie, 'cookie')) : undefined); requestData.cookies = cookies || {}; } diff --git a/packages/core/src/utils/cookie.ts b/packages/core/src/utils/cookie.ts index 218342ae36d3..fb4a8cb7e928 100644 --- a/packages/core/src/utils/cookie.ts +++ b/packages/core/src/utils/cookie.ts @@ -1,5 +1,5 @@ /** - * This code was originally copied from the 'cookie` module at v0.5.0 and was simplified for our use case. + * The value decoding in `cookiePairsToRecord` was originally copied from the 'cookie` module at v0.5.0. * https://github.com/jshttp/cookie/blob/a0c84147aab6266bdb3996cf4062e93907c0b0fc/index.js * It had the following license: * @@ -28,51 +28,68 @@ * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ -/** - * Parses a cookie string - */ -export function parseCookie(str: string): Record { - const obj: Record = {}; - let index = 0; +import { FILTERED_VALUE } from './data-collection/filtering-snippets'; - while (index < str.length) { - const eqIdx = str.indexOf('=', index); +/** A cookie's name and raw value. A nameless cookie (RFC 6265bis) has the name `''`. */ +export type CookiePair = [name: string, value: string]; - // no more cookie pairs - if (eqIdx === -1) { - break; +/** + * Splits a `Cookie` / `Set-Cookie` header into its ordered name-value pairs. Values are trimmed, but not + * decoded or unquoted. + * + * A segment without an `=` is a nameless cookie, so the bare token is its value (RFC 6265bis). + */ +export function parseCookieHeader(value: string | string[], headerName: 'cookie' | 'set-cookie'): CookiePair[] { + // Set-Cookie: one cookie per header, followed by attributes ("name=value; HttpOnly; Secure") + // Cookie: multiple cookies separated by ";" (the space after ";" is not guaranteed on the wire) + const segments = (Array.isArray(value) ? value : [value]).flatMap(headerValue => { + if (typeof headerValue !== 'string') { + return []; } + return headerName === 'set-cookie' ? [headerValue.split(';')[0]!] : headerValue.split(';'); + }); - let endIdx = str.indexOf(';', index); - - if (endIdx === -1) { - endIdx = str.length; - } else if (endIdx < eqIdx) { - // backtrack on prior semicolon - index = str.lastIndexOf(';', eqIdx - 1) + 1; - continue; - } + return ( + segments + .map(segment => segment.trim()) + // ";;" and trailing ";" leave empty segments. "=" has neither name nor value, so RFC 6265bis ignores it. + .filter(segment => segment !== '' && segment !== '=') + .map((segment): CookiePair => { + // Only first "=" separates name from value: "jwt=eyJhbGc=" has value "eyJhbGc=" + const equalSignIndex = segment.indexOf('='); + return equalSignIndex === -1 + ? // No "=": nameless cookie, the whole segment is the value + ['', segment] + : // Trim both parts, so that "theme = dark" is named "theme", not "theme " + [segment.slice(0, equalSignIndex).trim(), segment.slice(equalSignIndex + 1).trim()]; + }) + ); +} - const key = str.slice(index, eqIdx).trim(); +/** + * Converts cookie pairs to a record with decoded values. The first cookie of a name wins. + * + * A nameless cookie's token is its value, and no name-based denylist can match it. So it is stored + * under the name `''` and its value is always filtered. + */ +export function cookiePairsToRecord(pairs: CookiePair[]): Record { + const record: Record = {}; - // only assign once - if (undefined === obj[key]) { - let val = str.slice(eqIdx + 1, endIdx).trim(); + for (const [name, value] of pairs) { + if (record[name] === undefined) { + record[name] = name === '' ? FILTERED_VALUE : decodeCookieValue(value); + } + } - // quoted values - if (val.charCodeAt(0) === 0x22) { - val = val.slice(1, -1); - } + return record; +} - try { - obj[key] = val.indexOf('%') !== -1 ? decodeURIComponent(val) : val; - } catch { - obj[key] = val; - } - } +function decodeCookieValue(value: string): string { + const unquoted = value.length > 1 && value.startsWith('"') && value.endsWith('"') ? value.slice(1, -1) : value; - index = endIdx + 1; + try { + return unquoted.indexOf('%') !== -1 ? decodeURIComponent(unquoted) : unquoted; + } catch { + return unquoted; } - - return obj; } diff --git a/packages/core/src/utils/data-collection/filterCookies.ts b/packages/core/src/utils/data-collection/filterCookies.ts index 0fc373f4bfce..8a8f70e7338b 100644 --- a/packages/core/src/utils/data-collection/filterCookies.ts +++ b/packages/core/src/utils/data-collection/filterCookies.ts @@ -1,31 +1,25 @@ import type { CollectBehavior } from '../../types/datacollection'; -import { parseCookie } from '../cookie'; -import { FILTERED_VALUE as FILTERED, SENSITIVE_COOKIE_NAME_SNIPPETS } from './filtering-snippets'; +import { cookiePairsToRecord, parseCookieHeader } from '../cookie'; +import { SENSITIVE_COOKIE_NAME_SNIPPETS } from './filtering-snippets'; import { filterKeyValueData } from './filterKeyValueData'; /** - * Filters a cookie string according to a `CollectBehavior`. + * Filters a `Cookie` / `Set-Cookie` header string according to a `CollectBehavior`. * - * When individual cookies can be parsed, each key-value pair is filtered - * independently. When parsing fails, the entire string is replaced with `[Filtered]`. - * A nameless segment inside an otherwise parseable string (`"opaque-blob; theme=dark"`) is - * dropped, since a record key cannot carry a `[Filtered]` marker without leaking the token. + * Each named cookie is filtered independently. A nameless cookie (`"opaque-blob"`, `"=opaque-blob"`) + * is reported as `{ '': '[Filtered]' }`, since its token is the value. + * + * @param headerName - `'set-cookie'` keeps only the cookie pair and ignores the attributes (`Path`, `Max-Age`, ...) */ -export function filterCookies(cookieString: string, behavior: CollectBehavior): Record | string { +export function filterCookies( + cookieString: string, + behavior: CollectBehavior, + headerName: 'cookie' | 'set-cookie', +): Record { if (behavior === false) { return {}; } - try { - const parsed = parseCookie(cookieString); - - // A non-empty string we cannot parse may still hold a session token, so it counts as sensitive. - if (Object.keys(parsed).length === 0) { - return cookieString ? FILTERED : {}; - } - - return filterKeyValueData(parsed, behavior, SENSITIVE_COOKIE_NAME_SNIPPETS); - } catch { - return FILTERED; - } + const cookies = cookiePairsToRecord(parseCookieHeader(cookieString, headerName)); + return filterKeyValueData(cookies, behavior, SENSITIVE_COOKIE_NAME_SNIPPETS); } diff --git a/packages/core/src/utils/request.ts b/packages/core/src/utils/request.ts index 7c6ae29a3304..7c92511185ab 100644 --- a/packages/core/src/utils/request.ts +++ b/packages/core/src/utils/request.ts @@ -1,10 +1,12 @@ /* eslint-disable max-lines-per-function */ import { DEBUG_BUILD } from '../debug-build'; import type { Scope } from '../scope'; -import type { ResolvedDataCollection } from '../types/datacollection'; +import type { CollectBehavior, ResolvedDataCollection } from '../types/datacollection'; import type { PolymorphicRequest } from '../types/polymorphics'; import type { RequestEventData } from '../types/request'; import type { WebFetchHeaders, WebFetchRequest } from '../types/webfetchapi'; +import type { CookiePair } from './cookie'; +import { parseCookieHeader } from './cookie'; import { debug } from './debug-logger'; import { FILTERED_VALUE, SENSITIVE_COOKIE_NAME_SNIPPETS } from './data-collection/filtering-snippets'; import { shouldFilterDataKey } from './data-collection/filterKeyValueData'; @@ -303,18 +305,10 @@ export function httpHeadersToSpanAttributes( continue; } - const cookies = parseCookieHeader(value, lowerKey === 'set-cookie'); + const cookies = parseCookieHeader(value, lowerKey); + // A cookie header without a single pair may still hold a token, so it counts as sensitive. spanAttributes[`${prefix}${lowerKey}`] = cookies.length - ? cookies.map(([cookieKey, cookieValue]) => { - // A nameless cookie's bare token is its value; no denylist could match it, so it is - // always filtered. - if (cookieKey === '') { - return FILTERED_VALUE; - } - return shouldFilterDataKey(cookieKey, cookieBehavior, SENSITIVE_COOKIE_NAME_SNIPPETS) - ? `${cookieKey}=${FILTERED_VALUE}` - : `${cookieKey}=${cookieValue}`; - }) + ? filterCookiePairs(cookies, cookieBehavior) : [FILTERED_VALUE]; } else { if (headerBehavior === false) { @@ -343,31 +337,17 @@ export function httpHeadersToSpanAttributes( return spanAttributes; } -/** - * Splits a `Cookie` / `Set-Cookie` header into its name-value pairs. - * - * A segment without an `=` is a nameless cookie, so the bare token is its value (RFC 6265bis): - * it is returned as a pair with an empty name. - */ -function parseCookieHeader(value: string | string[], isSetCookie: boolean): [string, string][] { - // Set-Cookie: one cookie per value, with attributes ("name=value; HttpOnly; Secure") - // Cookie: multiple cookies separated by ";" (the space after ";" is not guaranteed on the wire) - const cookies = (Array.isArray(value) ? value : [value]).flatMap(headerValue => { - if (typeof headerValue !== 'string' || headerValue === '') { - return []; +/** Formats cookie pairs as `name=value` span attribute values, with sensitive values replaced. */ +export function filterCookiePairs(cookies: CookiePair[], cookieBehavior: CollectBehavior): string[] { + return cookies.map(([cookieKey, cookieValue]) => { + // A nameless cookie's bare token is its value; no denylist could match it, so it is always filtered. + if (cookieKey === '') { + return FILTERED_VALUE; } - return isSetCookie ? [headerValue.split(';')[0]!] : headerValue.split(';'); + return shouldFilterDataKey(cookieKey, cookieBehavior, SENSITIVE_COOKIE_NAME_SNIPPETS) + ? `${cookieKey}=${FILTERED_VALUE}` + : `${cookieKey}=${cookieValue}`; }); - - return cookies - .map(cookie => cookie.trim()) - .filter(cookie => cookie !== '') - .map(cookie => { - const equalSignIndex = cookie.indexOf('='); - return equalSignIndex !== -1 - ? [cookie.substring(0, equalSignIndex), cookie.substring(equalSignIndex + 1)] - : ['', cookie]; - }); } /** Extract the query params from an URL. */ diff --git a/packages/core/test/lib/integrations/requestdata.test.ts b/packages/core/test/lib/integrations/requestdata.test.ts index e5528d6b5816..549e8aa156b5 100644 --- a/packages/core/test/lib/integrations/requestdata.test.ts +++ b/packages/core/test/lib/integrations/requestdata.test.ts @@ -32,7 +32,7 @@ function baseEvent(overrides: Partial = {}): Event { }; } -/** Rich normalized request (Cookie header only — tests `parseCookie` path). */ +/** Rich normalized request (Cookie header only — tests the cookie header parsing path). */ function richNormalizedRequest() { return { method: 'POST', @@ -313,6 +313,23 @@ describe('requestDataIntegration', () => { expect(event.request?.cookies).toEqual({ id: '42' }); }); + it('filters a nameless token in the cookie header', () => { + const integration = requestDataIntegration(); + const event: Event = { + sdkProcessingMetadata: { + normalizedRequest: { + method: 'GET', + url: 'https://example.com/', + headers: { cookie: '=y7Uu0Rk2QpLmXv3; theme=dark' }, + }, + }, + }; + + integration.processEvent?.(event, {}, mockClient({ cookies: true })); + + expect(event.request?.cookies).toEqual({ '': '[Filtered]', theme: 'dark' }); + }); + it('omits headers when include.headers is false and dataCollection enables headers', () => { const integration = requestDataIntegration({ include: { headers: false } }); const event: Event = { @@ -978,6 +995,19 @@ describe('requestDataIntegration processSegmentSpan', () => { }); }); + it('does not split a cookie value that decodes to ";name=value" into a second cookie', () => { + const integration = requestDataIntegration(); + const span = makeSpan(); + + mockIsolationScope({ + headers: { cookie: 'session=%3Btheme%3Ds3cr3t' }, + }); + + integration.processSegmentSpan!(span, mockClient({ userInfo: false })); + + expect(span.attributes['http.request.header.cookie']).toEqual(['session=[Filtered]']); + }); + it('filters sensitive cookies', () => { const integration = requestDataIntegration(); const span = makeSpan(); diff --git a/packages/core/test/lib/utils/cookie.test.ts b/packages/core/test/lib/utils/cookie.test.ts index ccec4a9a26dd..495041e72397 100644 --- a/packages/core/test/lib/utils/cookie.test.ts +++ b/packages/core/test/lib/utils/cookie.test.ts @@ -1,5 +1,5 @@ /** - * This code was originally copied from the 'cookie` module at v0.5.0 and was simplified for our use case. + * The `cookiePairsToRecord` decoding cases were originally copied from the 'cookie` module at v0.5.0. * https://github.com/jshttp/cookie/blob/a0c84147aab6266bdb3996cf4062e93907c0b0fc/test/parse.js * It had the following license: * @@ -29,40 +29,167 @@ */ import { describe, expect, it } from 'vitest'; -import { parseCookie } from '../../../src/utils/cookie'; +import { cookiePairsToRecord, parseCookieHeader } from '../../../src/utils/cookie'; -describe('parseCookie(str)', function () { - it('should parse cookie string to object', function () { - expect(parseCookie('foo=bar')).toEqual({ foo: 'bar' }); - expect(parseCookie('foo=123')).toEqual({ foo: '123' }); +describe('parseCookieHeader', () => { + describe('cookie', () => { + it('returns the pairs in header order and keeps repeated names', () => { + expect(parseCookieHeader('locale=en; theme=dark; locale=de', 'cookie')).toEqual([ + ['locale', 'en'], + ['theme', 'dark'], + ['locale', 'de'], + ]); + }); + + it('splits on ";" without a following space', () => { + expect(parseCookieHeader('theme=dark;__Secure-session=abc123', 'cookie')).toEqual([ + ['theme', 'dark'], + ['__Secure-session', 'abc123'], + ]); + }); + + it('trims whitespace around names and values', () => { + expect(parseCookieHeader(' THEME = dark ; locale = en', 'cookie')).toEqual([ + ['THEME', 'dark'], + ['locale', 'en'], + ]); + }); + + it('splits a pair only at the first "="', () => { + expect(parseCookieHeader('jwt=eyJhbGc=.eyJzdWI=.SflKxw', 'cookie')).toEqual([ + ['jwt', 'eyJhbGc=.eyJzdWI=.SflKxw'], + ]); + }); + + it('keeps values as they are on the wire', () => { + expect(parseCookieHeader('email=jane%40example.com; theme="dark mode"', 'cookie')).toEqual([ + ['email', 'jane%40example.com'], + ['theme', '"dark mode"'], + ]); + }); + + it('keeps an empty value', () => { + expect(parseCookieHeader('cart=; theme= ', 'cookie')).toEqual([ + ['cart', ''], + ['theme', ''], + ]); + }); + + it.each([ + ['a segment without "="', 'y7Uu0Rk2QpLmXv3; theme=dark'], + ['a segment that starts with "="', '=y7Uu0Rk2QpLmXv3; theme=dark'], + ])('returns %s as a nameless cookie', (_, header) => { + expect(parseCookieHeader(header, 'cookie')).toEqual([ + ['', 'y7Uu0Rk2QpLmXv3'], + ['theme', 'dark'], + ]); + }); + + it.each(['', ' ', ';;;', ' ; ; ', '=', ' = ; ='])('returns no pairs for %j', header => { + expect(parseCookieHeader(header, 'cookie')).toEqual([]); + }); + + it('does not split a value on ","', () => { + expect(parseCookieHeader('recent=shoes,socks; theme=dark', 'cookie')).toEqual([ + ['recent', 'shoes,socks'], + ['theme', 'dark'], + ]); + }); + + it('reads Set-Cookie attribute names as cookie names', () => { + expect(parseCookieHeader('Path=/; Max-Age=3600', 'cookie')).toEqual([ + ['Path', '/'], + ['Max-Age', '3600'], + ]); + }); }); - it('should ignore OWS', function () { - expect(parseCookie('FOO = bar; baz = raz')).toEqual({ FOO: 'bar', baz: 'raz' }); + describe('set-cookie', () => { + it.each([ + 'sid=s3cr3t; Max-Age=3600; Path=/', + 'sid=s3cr3t; Expires=Wed, 21 Oct 2026 07:28:00 GMT; Domain=example.com', + 'sid=s3cr3t; HttpOnly; Secure; SameSite=Lax', + 'sid=s3cr3t;Secure', + ])('drops the attributes of %j', header => { + expect(parseCookieHeader(header, 'set-cookie')).toEqual([['sid', 's3cr3t']]); + }); + + it('returns a nameless cookie when the cookie segment has no "="', () => { + expect(parseCookieHeader('y7Uu0Rk2QpLmXv3; HttpOnly', 'set-cookie')).toEqual([['', 'y7Uu0Rk2QpLmXv3']]); + }); + + it('returns no pairs when the cookie segment is empty', () => { + expect(parseCookieHeader('; HttpOnly', 'set-cookie')).toEqual([]); + }); + + it('returns one pair per header value', () => { + expect(parseCookieHeader(['theme=dark; HttpOnly', 'sid=s3cr3t; Secure'], 'set-cookie')).toEqual([ + ['theme', 'dark'], + ['sid', 's3cr3t'], + ]); + }); + }); + + describe('array values', () => { + it('concatenates the cookies of multiple Cookie header values', () => { + expect(parseCookieHeader(['theme=dark; locale=en', 'sid=s3cr3t'], 'cookie')).toEqual([ + ['theme', 'dark'], + ['locale', 'en'], + ['sid', 's3cr3t'], + ]); + }); + + it('returns no pairs for an empty array', () => { + expect(parseCookieHeader([], 'cookie')).toEqual([]); + }); + + it('skips values that are not strings', () => { + const values = ['theme=dark', undefined, 42] as unknown as string[]; + + expect(parseCookieHeader(values, 'cookie')).toEqual([['theme', 'dark']]); + }); }); +}); - it('should parse cookie with empty value', function () { - expect(parseCookie('foo= ; bar=')).toEqual({ foo: '', bar: '' }); +describe('cookiePairsToRecord', () => { + it('returns an empty record for no pairs', () => { + expect(cookiePairsToRecord([])).toEqual({}); }); - it('should URL-decode values', function () { - expect(parseCookie('foo="bar=123456789&name=Magic+Mouse"')).toEqual({ foo: 'bar=123456789&name=Magic+Mouse' }); + it('keeps the first value of a repeated name, even when it is empty', () => { + expect( + cookiePairsToRecord([ + ['locale', ''], + ['theme', 'dark'], + ['locale', 'de'], + ]), + ).toEqual({ locale: '', theme: 'dark' }); + }); + + it('filters the value of a nameless cookie', () => { + expect( + cookiePairsToRecord([ + ['', 'y7Uu0Rk2QpLmXv3'], + ['theme', 'dark'], + ]), + ).toEqual({ '': '[Filtered]', theme: 'dark' }); + }); - expect(parseCookie('email=%20%22%2c%3b%2f')).toEqual({ email: ' ",;/' }); + it('URL-decodes values', () => { + expect(cookiePairsToRecord([['email', '%20%22%2c%3b%2f']])).toEqual({ email: ' ",;/' }); }); - it('should return original value on escape error', function () { - expect(parseCookie('foo=%1;bar=bar')).toEqual({ foo: '%1', bar: 'bar' }); + it('keeps a value that is not valid URL encoding', () => { + expect(cookiePairsToRecord([['discount', '50%']])).toEqual({ discount: '50%' }); }); - it('should ignore cookies without value', function () { - expect(parseCookie('foo=bar;fizz ; buzz')).toEqual({ foo: 'bar' }); - expect(parseCookie(' fizz; foo= bar')).toEqual({ foo: 'bar' }); + it('strips the quotes of a quoted value', () => { + expect(cookiePairsToRecord([['cart', '"sku=123456789&name=Magic+Mouse"']])).toEqual({ + cart: 'sku=123456789&name=Magic+Mouse', + }); }); - it('should ignore duplicate cookies', function () { - expect(parseCookie('foo=%1;bar=bar;foo=boo')).toEqual({ foo: '%1', bar: 'bar' }); - expect(parseCookie('foo=false;bar=bar;foo=tre')).toEqual({ foo: 'false', bar: 'bar' }); - expect(parseCookie('foo=;bar=bar;foo=boo')).toEqual({ foo: '', bar: 'bar' }); + it.each(['"unterminated', 'unstarted"', '"'])('keeps %j, which is not a quoted value', value => { + expect(cookiePairsToRecord([['note', value]])).toEqual({ note: value }); }); }); diff --git a/packages/core/test/lib/utils/data-collection/filterCookies.test.ts b/packages/core/test/lib/utils/data-collection/filterCookies.test.ts index 4f8ed3d57fba..f686e0b46e4f 100644 --- a/packages/core/test/lib/utils/data-collection/filterCookies.test.ts +++ b/packages/core/test/lib/utils/data-collection/filterCookies.test.ts @@ -4,13 +4,13 @@ import { filterCookies } from '../../../../src/utils/data-collection/filterCooki describe('filterCookies', () => { describe('off mode (false)', () => { it('returns empty record', () => { - expect(filterCookies('theme=dark; user_session=abc123', false)).toEqual({}); + expect(filterCookies('theme=dark; user_session=abc123', false, 'cookie')).toEqual({}); }); }); describe('denyList mode (true)', () => { it('filters sensitive cookie names and preserves safe ones', () => { - const result = filterCookies('theme=dark; user_session=abc123; locale=en', true); + const result = filterCookies('theme=dark; user_session=abc123; locale=en', true, 'cookie'); expect(result).toEqual({ theme: 'dark', @@ -20,7 +20,7 @@ describe('filterCookies', () => { }); it('filters auth-related cookies', () => { - const result = filterCookies('auth_token=xyz; color=blue', true); + const result = filterCookies('auth_token=xyz; color=blue', true, 'cookie'); expect(result).toEqual({ auth_token: '[Filtered]', // matches "auth" and "token" @@ -29,7 +29,11 @@ describe('filterCookies', () => { }); it('filters cookie-specific sensitive names', () => { - const result = filterCookies('theme=dark; connect.sid=abc; remember_me=xyz; __secure-token=secret', true); + const result = filterCookies( + 'theme=dark; connect.sid=abc; remember_me=xyz; __secure-token=secret', + true, + 'cookie', + ); expect(result).toEqual({ theme: 'dark', @@ -42,7 +46,7 @@ describe('filterCookies', () => { describe('denyList mode ({ deny: [...] })', () => { it('applies extra deny terms on top of built-in denylist', () => { - const result = filterCookies('theme=dark; tracking_id=abc', { deny: ['tracking'] }); + const result = filterCookies('theme=dark; tracking_id=abc', { deny: ['tracking'] }, 'cookie'); expect(result).toEqual({ theme: 'dark', @@ -53,9 +57,13 @@ describe('filterCookies', () => { describe('allowList mode ({ allow: [...] })', () => { it('only allows specified cookie names to pass through', () => { - const result = filterCookies('theme=dark; user_session=abc; locale=en', { - allow: ['theme', 'locale'], - }); + const result = filterCookies( + 'theme=dark; user_session=abc; locale=en', + { + allow: ['theme', 'locale'], + }, + 'cookie', + ); expect(result).toEqual({ theme: 'dark', @@ -65,7 +73,7 @@ describe('filterCookies', () => { }); it('sensitive denylist overrides allowlist', () => { - const result = filterCookies('auth_token=secret', { allow: ['auth_token'] }); + const result = filterCookies('auth_token=secret', { allow: ['auth_token'] }, 'cookie'); expect(result).toEqual({ auth_token: '[Filtered]', // "auth" and "token" match sensitive denylist @@ -75,33 +83,57 @@ describe('filterCookies', () => { describe('empty and unparseable input', () => { it('returns empty record for empty string', () => { - expect(filterCookies('', true)).toEqual({}); + expect(filterCookies('', true, 'cookie')).toEqual({}); }); - it('filters the whole string when no key-value pairs can be extracted', () => { - expect(filterCookies(';;;', true)).toBe('[Filtered]'); - expect(filterCookies('opaque-session-blob', true)).toBe('[Filtered]'); + it('returns an empty record when the string holds no cookie', () => { + expect(filterCookies(';;;', true, 'cookie')).toEqual({}); }); }); - // Intended behavior for the cookie parsing consolidation follow-up: `Set-Cookie` attributes are - // metadata, not cookies, so they must not show up as key-value pairs. Marked `fails` until the - // shared parser handles them. - describe('Set-Cookie attribute handling (known gaps)', () => { - it.fails('does not report Set-Cookie attributes as cookie pairs', () => { - expect(filterCookies('sid=1; Max-Age=3600; Path=/', true)).toEqual({ sid: '[Filtered]' }); + describe('nameless cookies', () => { + it.each(['y7Uu0Rk2QpLmXv3; theme=dark', '=y7Uu0Rk2QpLmXv3; theme=dark', 'theme=dark; y7Uu0Rk2QpLmXv3'])( + 'filters the nameless token in %j and keeps the named cookie', + cookieString => { + expect(filterCookies(cookieString, true, 'cookie')).toEqual({ '': '[Filtered]', theme: 'dark' }); + }, + ); + + it.each(['y7Uu0Rk2QpLmXv3', '=y7Uu0Rk2QpLmXv3'])('filters %j when it is the only cookie', cookieString => { + expect(filterCookies(cookieString, true, 'cookie')).toEqual({ '': '[Filtered]' }); }); - it.fails('does not report Expires/Domain attributes as cookie pairs', () => { - expect(filterCookies('theme=dark; Expires=Wed, 21 Oct 2026 07:28:00 GMT; Domain=example.com', true)).toEqual({ + it('filters the nameless token when an allowlist is configured', () => { + expect(filterCookies('y7Uu0Rk2QpLmXv3; theme=dark', { allow: ['theme'] }, 'cookie')).toEqual({ + '': '[Filtered]', theme: 'dark', }); }); }); + describe('Set-Cookie header', () => { + it('does not report Set-Cookie attributes as cookie pairs', () => { + expect(filterCookies('sid=1; Max-Age=3600; Path=/', true, 'set-cookie')).toEqual({ sid: '[Filtered]' }); + }); + + it('does not report Expires/Domain attributes as cookie pairs', () => { + expect( + filterCookies('theme=dark; Expires=Wed, 21 Oct 2026 07:28:00 GMT; Domain=example.com', true, 'set-cookie'), + ).toEqual({ theme: 'dark' }); + }); + + it('filters the token of a nameless cookie', () => { + expect(filterCookies('y7Uu0Rk2QpLmXv3; HttpOnly; Secure', true, 'set-cookie')).toEqual({ '': '[Filtered]' }); + }); + }); + describe('edge cases', () => { + it('reads attribute-like names in a Cookie header as cookies', () => { + expect(filterCookies('theme=dark; Path=/checkout', true, 'cookie')).toEqual({ theme: 'dark', Path: '/checkout' }); + }); + it('handles cookies with = in the value', () => { - const result = filterCookies('data=base64==; theme=light', true); + const result = filterCookies('data=base64==; theme=light', true, 'cookie'); expect(result).toEqual({ data: 'base64==', @@ -110,7 +142,7 @@ describe('filterCookies', () => { }); it('handles quoted cookie values', () => { - const result = filterCookies('theme="dark mode"', true); + const result = filterCookies('theme="dark mode"', true, 'cookie'); expect(result).toEqual({ theme: 'dark mode', diff --git a/packages/core/test/lib/utils/request.test.ts b/packages/core/test/lib/utils/request.test.ts index 58e29ad12ddd..3266eab7a1e2 100644 --- a/packages/core/test/lib/utils/request.test.ts +++ b/packages/core/test/lib/utils/request.test.ts @@ -696,6 +696,16 @@ describe('request utils', () => { }); }); + it('trims whitespace around cookie names and values', () => { + const headers = { Cookie: 'theme = dark; user_session = abc123' }; + + const result = httpHeadersToSpanAttributes(headers, resolveDataCollectionOptions({})); + + expect(result).toEqual({ + 'http.request.header.cookie': ['theme=dark', 'user_session=[Filtered]'], + }); + }); + it('filters common framework and provider session-style cookie names', () => { const headers = { Cookie: