ref(core): Consolidate cookie parsing into one parser

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`.

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

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,30 +244,36 @@ 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', () => {
       const { xhrHandler, captureEventSpy } = setup();
 
       triggerXhr(xhrHandler, {
         requestHeaders: { Authorization: 'Bearer x' },
-        setCookie: 'session=abc123; theme=dark',
+        setCookie: 'session=abc123; Path=/',
         allResponseHeaders: 'content-type: text/html',
       });
 
       expect(captureEventSpy).toHaveBeenCalledTimes(1);
       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]' });
     });
   });
 });

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 || {};
   }
 

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<string, string> {
-  const obj: Record<string, string> = {};
-  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<string, string> {
+  const record: Record<string, string> = {};
 
-    // 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;
 }

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, string> | string {
+export function filterCookies(
+  cookieString: string,
+  behavior: CollectBehavior,
+  headerName: 'cookie' | 'set-cookie',
+): Record<string, string> {
   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);
 }

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. */