From 1e0cb046f029085b09d0f3193a325f4adeaec3f4 Mon Sep 17 00:00:00 2001
From: Francesco Novy <francesco.novy@sentry.io>
Date: Wed, 27 Nov 2024 09:57:19 +0100
Subject: [PATCH] ref(core): Do not check baggage validity (#14479)

This PR drops the validation for baggage content. We didn't do this for
browser previously, only for node, but it adds bundle size and does not
appear too important.

I left the trace header validation in for now, we may also drop this but
it is smaller and I guess also more important to us...?
---
 packages/core/src/utils/traceData.ts          | 29 +-------
 .../core/test/lib/utils/traceData.test.ts     | 73 -------------------
 2 files changed, 1 insertion(+), 101 deletions(-)

diff --git a/packages/core/src/utils/traceData.ts b/packages/core/src/utils/traceData.ts
index bd93948d7953..4892e558661a 100644
--- a/packages/core/src/utils/traceData.ts
+++ b/packages/core/src/utils/traceData.ts
@@ -44,39 +44,12 @@ export function getTraceData(options: { span?: Span } = {}): SerializedTraceData
     return {};
   }
 
-  const validBaggage = isValidBaggageString(baggage);
-  if (!validBaggage) {
-    logger.warn('Invalid baggage data. Not returning "baggage" value');
-  }
-
   return {
     'sentry-trace': sentryTrace,
-    ...(validBaggage && { baggage }),
+    baggage,
   };
 }
 
-/**
- * Tests string against baggage spec as defined in:
- *
- * - W3C Baggage grammar: https://www.w3.org/TR/baggage/#definition
- * - RFC7230 token definition: https://datatracker.ietf.org/doc/html/rfc7230#section-3.2.6
- *
- * exported for testing
- */
-export function isValidBaggageString(baggage?: string): boolean {
-  if (!baggage || !baggage.length) {
-    return false;
-  }
-  const keyRegex = "[-!#$%&'*+.^_`|~A-Za-z0-9]+";
-  const valueRegex = '[!#-+-./0-9:<=>?@A-Z\\[\\]a-z{-}]+';
-  const spaces = '\\s*';
-  // eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor -- RegExp for readability, no user input
-  const baggageRegex = new RegExp(
-    `^${keyRegex}${spaces}=${spaces}${valueRegex}(${spaces},${spaces}${keyRegex}${spaces}=${spaces}${valueRegex})*$`,
-  );
-  return baggageRegex.test(baggage);
-}
-
 /**
  * Get a sentry-trace header value for the given scope.
  */
diff --git a/packages/core/test/lib/utils/traceData.test.ts b/packages/core/test/lib/utils/traceData.test.ts
index c6ef21101830..d73ab091d8eb 100644
--- a/packages/core/test/lib/utils/traceData.test.ts
+++ b/packages/core/test/lib/utils/traceData.test.ts
@@ -13,7 +13,6 @@ import {
 import { getAsyncContextStrategy } from '../../../src/asyncContext';
 import { freezeDscOnSpan } from '../../../src/tracing/dynamicSamplingContext';
 
-import { isValidBaggageString } from '../../../src/utils/traceData';
 import type { TestClientOptions } from '../../mocks/client';
 import { TestClient, getDefaultTestClientOptions } from '../../mocks/client';
 
@@ -281,75 +280,3 @@ describe('getTraceData', () => {
     expect(traceData).toEqual({});
   });
 });
-
-describe('isValidBaggageString', () => {
-  it.each([
-    'sentry-environment=production',
-    'sentry-environment=staging,sentry-public_key=key,sentry-trace_id=abc',
-    // @ is allowed in values
-    'sentry-release=project@1.0.0',
-    // spaces are allowed around the delimiters
-    'sentry-environment=staging ,   sentry-public_key=key  ,sentry-release=myproject@1.0.0',
-    'sentry-environment=staging ,   thirdparty=value  ,sentry-release=myproject@1.0.0',
-    // these characters are explicitly allowed for keys in the baggage spec:
-    "!#$%&'*+-.^_`|~1234567890abcxyzABCXYZ=true",
-    // special characters in values are fine (except for ",;\ - see other test)
-    'key=(value)',
-    'key=[{(value)}]',
-    'key=some$value',
-    'key=more#value',
-    'key=max&value',
-    'key=max:value',
-    'key=x=value',
-  ])('returns true if the baggage string is valid (%s)', baggageString => {
-    expect(isValidBaggageString(baggageString)).toBe(true);
-  });
-
-  it.each([
-    // baggage spec doesn't permit leading spaces
-    ' sentry-environment=production,sentry-publickey=key,sentry-trace_id=abc',
-    // no spaces in keys or values
-    'sentry-public key=key',
-    'sentry-publickey=my key',
-    // no delimiters ("(),/:;<=>?@[\]{}") in keys
-    'asdf(x=value',
-    'asdf)x=value',
-    'asdf,x=value',
-    'asdf/x=value',
-    'asdf:x=value',
-    'asdf;x=value',
-    'asdf<x=value',
-    'asdf>x=value',
-    'asdf?x=value',
-    'asdf@x=value',
-    'asdf[x=value',
-    'asdf]x=value',
-    'asdf\\x=value',
-    'asdf{x=value',
-    'asdf}x=value',
-    // no ,;\" in values
-    'key=va,lue',
-    'key=va;lue',
-    'key=va\\lue',
-    'key=va"lue"',
-    // baggage headers can have properties but we currently don't support them
-    'sentry-environment=production;prop1=foo;prop2=bar,nextkey=value',
-    // no fishy stuff
-    'absolutely not a valid baggage string',
-    'val"/><script>alert("xss")</script>',
-    'something"/>',
-    '<script>alert("xss")</script>',
-    '/>',
-    '" onblur="alert("xss")',
-  ])('returns false if the baggage string is invalid (%s)', baggageString => {
-    expect(isValidBaggageString(baggageString)).toBe(false);
-  });
-
-  it('returns false if the baggage string is empty', () => {
-    expect(isValidBaggageString('')).toBe(false);
-  });
-
-  it('returns false if the baggage string is empty', () => {
-    expect(isValidBaggageString(undefined)).toBe(false);
-  });
-});