Consolidate the two cookie parsers parseCookieString and getCookieAttachments

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 { parseCookiePairs } 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';
@@ -186,11 +186,13 @@ 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);
+    const cookieAttributes = filterCookiePairs(
+      Object.entries(requestData.cookies).map(([name, value]) => [name, String(value)]),
+      dataCollection.cookies,
+    );
+    if (cookieAttributes.length) {
+      safeSetSpanJSONAttributes(span, { 'http.request.header.cookie': cookieAttributes });
+    }
   }
 
   if (requestData.headers) {
@@ -245,7 +247,7 @@ function extractNormalizedRequestData(
   }
 
   if (include.cookies) {
-    const cookies = normalizedRequest.cookies || (headers?.cookie ? parseCookie(headers.cookie) : undefined);
+    const cookies = normalizedRequest.cookies || (headers?.cookie ? parseCookieRecord(headers.cookie) : undefined);
     requestData.cookies = cookies || {};
   }
 
@@ -260,6 +262,19 @@ function extractNormalizedRequestData(
   return requestData;
 }
 
+function parseCookieRecord(cookieString: string): Record<string, string> {
+  const parsed: Record<string, string> = {};
+
+  for (const [name, value] of parseCookiePairs(cookieString)) {
+    // only assign once
+    if (name !== '' && !(name in parsed)) {
+      parsed[name] = value;
+    }
+  }
+
+  return parsed;
+}
+
 function resolveFilteringBehavior(isIncluded: boolean, behavior: CollectBehavior): CollectBehavior {
   return isIncluded && behavior === false ? true : behavior;
 }

packages/core/src/utils/cookie.ts

@@ -28,51 +28,70 @@
  * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
  */
 
+// `Set-Cookie` attributes are metadata, not cookies. Response cookie strings handed to
+// the `Cookie`-mode parser may still carry them, so they are dropped by name.
+const SET_COOKIE_ATTRIBUTES = new Set([
+  'expires',
+  'max-age',
+  'domain',
+  'path',
+  'secure',
+  'httponly',
+  'samesite',
+  'partitioned',
+]);
+
 /**
- * Parses a cookie string
+ * Parses a `Cookie` or `Set-Cookie` header value into ordered `[name, value]` pairs.
+ *
+ * In `Set-Cookie` mode each header value carries a single cookie, so only the segment
+ * before the first `;` is parsed. Otherwise every `;`-separated segment is a pair, with
+ * known `Set-Cookie` attributes dropped by name.
+ *
+ * Segments without `=` (e.g. a bare token or a flag like `Secure`) are returned as
+ * `['', segment]`; values are unquoted and URL-decoded.
  */
-export function parseCookie(str: string): Record<string, string> {
-  const obj: Record<string, string> = {};
-  let index = 0;
+export function parseCookiePairs(value: string | string[], setCookie = false): [string, string][] {
+  const pairs: [string, string][] = [];
 
-  while (index < str.length) {
-    const eqIdx = str.indexOf('=', index);
-
-    // no more cookie pairs
-    if (eqIdx === -1) {
-      break;
+  for (const headerValue of Array.isArray(value) ? value : [value]) {
+    if (typeof headerValue !== 'string' || headerValue === '') {
+      continue;
     }
 
-    let endIdx = str.indexOf(';', index);
+    // A `Set-Cookie` value carries one cookie, so only its first segment is a pair.
+    // `headerValue` is non-empty here, so the split always yields at least one segment.
+    const segments = setCookie ? headerValue.split(';', 1) : headerValue.split(';');
 
-    if (endIdx === -1) {
-      endIdx = str.length;
-    } else if (endIdx < eqIdx) {
-      // backtrack on prior semicolon
-      index = str.lastIndexOf(';', eqIdx - 1) + 1;
-      continue;
-    }
+    for (let segment of segments) {
+      segment = segment.trim();
 
-    const key = str.slice(index, eqIdx).trim();
+      if (segment === '') {
+        continue;
+      }
 
-    // only assign once
-    if (undefined === obj[key]) {
-      let val = str.slice(eqIdx + 1, endIdx).trim();
+      const eqIdx = segment.indexOf('=');
+      const name = (eqIdx === -1 ? '' : segment.slice(0, eqIdx)).trim();
+      let val = (eqIdx === -1 ? segment : segment.slice(eqIdx + 1)).trim();
 
       // quoted values
       if (val.charCodeAt(0) === 0x22) {
         val = val.slice(1, -1);
       }
 
       try {
-        obj[key] = val.indexOf('%') !== -1 ? decodeURIComponent(val) : val;
+        val = val.indexOf('%') !== -1 ? decodeURIComponent(val) : val;
       } catch {
-        obj[key] = val;
+        // keep the raw value
       }
-    }
 
-    index = endIdx + 1;
+      if (!setCookie && SET_COOKIE_ATTRIBUTES.has(name.toLowerCase())) {
+        continue;
+      }
+
+      pairs.push([name, val]);
+    }
   }
 
-  return obj;
+  return pairs;
 }

packages/core/src/utils/data-collection/filterCookies.ts

@@ -1,5 +1,5 @@
 import type { CollectBehavior } from '../../types/datacollection';
-import { parseCookie } from '../cookie';
+import { parseCookiePairs } from '../cookie';
 import { FILTERED_VALUE as FILTERED, SENSITIVE_COOKIE_NAME_SNIPPETS } from './filtering-snippets';
 import { filterKeyValueData } from './filterKeyValueData';
 
@@ -8,14 +8,23 @@ import { filterKeyValueData } from './filterKeyValueData';
  *
  * 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 is dropped: a record key cannot carry a `[Filtered]` marker
+ * without leaking the bare token.
  */
 export function filterCookies(cookieString: string, behavior: CollectBehavior): Record<string, string> | string {
   if (behavior === false) {
     return {};
   }
 
   try {
-    const parsed = parseCookie(cookieString);
+    const parsed: Record<string, string> = {};
+
+    for (const [name, value] of parseCookiePairs(cookieString)) {
+      // only assign once
+      if (name !== '' && !(name in parsed)) {
+        parsed[name] = value;
+      }
+    }
 
     if (Object.keys(parsed).length === 0) {
       return {};

packages/core/src/utils/request.ts

@@ -1,13 +1,14 @@
 /* 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 { debug } from './debug-logger';
 import { FILTERED_VALUE, SENSITIVE_COOKIE_NAME_SNIPPETS } from './data-collection/filtering-snippets';
 import { shouldFilterDataKey } from './data-collection/filterKeyValueData';
+import { parseCookiePairs } from './cookie';
 import { safeUnref } from './timer';
 import { getUrlQuery } from './url';
 
@@ -303,14 +304,8 @@ export function httpHeadersToSpanAttributes(
           continue;
         }
 
-        const cookies = parseCookieHeader(value, lowerKey === 'set-cookie');
-        spanAttributes[`${prefix}${lowerKey}`] = cookies.length
-          ? cookies.map(([cookieKey, cookieValue]) =>
-              shouldFilterDataKey(cookieKey, cookieBehavior, SENSITIVE_COOKIE_NAME_SNIPPETS)
-                ? `${cookieKey}=${FILTERED_VALUE}`
-                : `${cookieKey}=${cookieValue}`,
-            )
-          : [FILTERED_VALUE];
+        const cookies = parseCookiePairs(value, lowerKey === 'set-cookie');
+        spanAttributes[`${prefix}${lowerKey}`] = filterCookiePairs(cookies, cookieBehavior);
       } else {
         if (headerBehavior === false) {
           continue;
@@ -338,21 +333,30 @@ export function httpHeadersToSpanAttributes(
   return spanAttributes;
 }
 
-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 "; " ("cookie1=value1; cookie2=value2")
-  const cookies = (Array.isArray(value) ? value : [value]).flatMap(headerValue => {
-    if (typeof headerValue !== 'string' || headerValue === '') {
-      return [];
-    }
-    return isSetCookie ? [headerValue.split(';')[0]!] : headerValue.split('; ');
-  });
+/**
+ * Filter already-parsed cookie pairs and serialize them back to `name=value` strings.
+ *
+ * Callers must pass pairs straight through rather than rejoining them into a header
+ * string first: a value containing `;`, `%`, or quotes survives a single parse, but a
+ * round trip through a string splits and double-decodes it.
+ */
+export function filterCookiePairs(pairs: [string, string][], cookieBehavior: CollectBehavior): string[] {
+  if (cookieBehavior === false) {
+    return [];
+  }
+
+  if (!pairs.length) {
+    return [FILTERED_VALUE];
+  }
 
-  return cookies.map(cookie => {
-    const equalSignIndex = cookie.indexOf('=');
-    return equalSignIndex !== -1
-      ? [cookie.substring(0, equalSignIndex), cookie.substring(equalSignIndex + 1)]
-      : [cookie, ''];
+  return pairs.map(([cookieKey, cookieValue]) => {
+    // A nameless segment'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}`;
   });
 }
 

packages/core/test/lib/integrations/requestdata.test.ts

@@ -32,7 +32,7 @@ function baseEvent(overrides: Partial<Event> = {}): Event {
   };
 }
 
-/** Rich normalized request (Cookie header only — tests `parseCookie` path). */
+/** Rich normalized request (Cookie header only — tests `parseCookiePairs` path). */
 function richNormalizedRequest() {
   return {
     method: 'POST',
@@ -963,6 +963,21 @@ describe('requestDataIntegration processSegmentSpan', () => {
     });
   });
 
+  it('does not double-decode cookies already parsed from the request', () => {
+    const integration = requestDataIntegration();
+    const span = makeSpan();
+
+    mockIsolationScope({
+      cookies: { theme: '%20' },
+    });
+
+    integration.processSegmentSpan!(span, mockClient({ userInfo: false }));
+
+    expect(span.attributes).toMatchObject({
+      'http.request.header.cookie': ['theme=%20'],
+    });
+  });
+
   it('falls back to cookie header when normalizedRequest.cookies is not set', () => {
     const integration = requestDataIntegration({ include: { headers: false } });
     const span = makeSpan();