fix(tracing): Baggage parsing fails when input is not of type string (#5276)

This patch attempts to fix a bug reported in #5250 where the input parameter for `parseBaggageString` was not a string and hence the `string.split` function call would throw an error. We now check for the type of the input and make the function

* return an empty baggage object if the input is neither a string, nor an array. In this case we log a warning to get more information on what is actually passed in.
* parse the baggage string if the input is a string
* join and then split the array if the input is an array

Additionally, the parser now ignores empty or blank baggage entries (e.g. `foo=bar,   ,,alice=bob`).

Added tests for parsing the new data types

Author: Lms24

packages/node/src/integrations/http.ts

@@ -123,7 +123,7 @@ function _createWrappedRequestMethodFactory(
               `[Tracing] Adding sentry-trace header ${sentryTraceHeader} to outgoing request to ${requestUrl}: `,
             );
 
-          const headerBaggageString = requestOptions.headers && (requestOptions.headers.baggage as string);
+          const headerBaggageString = requestOptions.headers && requestOptions.headers.baggage;
 
           requestOptions.headers = {
             ...requestOptions.headers,

packages/types/src/index.ts

@@ -36,7 +36,7 @@ export type {} from './globals';
 export type { Hub } from './hub';
 export type { Integration, IntegrationClass } from './integration';
 export type { Mechanism } from './mechanism';
-export type { ExtractedNodeRequestData, Primitive, WorkerLocation } from './misc';
+export type { ExtractedNodeRequestData, HttpHeaderValue, Primitive, WorkerLocation } from './misc';
 export type { ClientOptions, Options } from './options';
 export type { Package } from './package';
 export type { PolymorphicEvent } from './polymorphics';

packages/types/src/misc.ts

@@ -63,3 +63,5 @@ export interface WorkerLocation {
 }
 
 export type Primitive = number | string | boolean | bigint | symbol | null | undefined;
+
+export type HttpHeaderValue = string | string[] | number | null;

packages/utils/src/baggage.ts

@@ -1,5 +1,7 @@
 import { Baggage, BaggageObj, TraceparentData } from '@sentry/types';
+import { HttpHeaderValue } from '@sentry/types';
 
+import { isString } from './is';
 import { logger } from './logger';
 
 export const BAGGAGE_HEADER_NAME = 'baggage';
@@ -89,9 +91,28 @@ export function serializeBaggage(baggage: Baggage): string {
   }, baggage[1]);
 }
 
-/** Parse a baggage header from a string and return a Baggage object */
-export function parseBaggageString(inputBaggageString: string): Baggage {
-  return inputBaggageString.split(',').reduce(
+/** Parse a baggage header from a string or a string array and return a Baggage object */
+export function parseBaggageHeader(inputBaggageValue: HttpHeaderValue): Baggage {
+  // Adding this check here because we got reports of this function failing due to the input value
+  // not being a string. This debug log might help us determine what's going on here.
+  if ((!Array.isArray(inputBaggageValue) && !isString(inputBaggageValue)) || typeof inputBaggageValue === 'number') {
+    __DEBUG_BUILD__ &&
+      logger.warn(
+        '[parseBaggageHeader] Received input value of incompatible type: ',
+        typeof inputBaggageValue,
+        inputBaggageValue,
+      );
+
+    // Gonna early-return an empty baggage object so that we don't fail later on
+    return createBaggage({}, '');
+  }
+
+  const baggageEntries = (isString(inputBaggageValue) ? inputBaggageValue : inputBaggageValue.join(','))
+    .split(',')
+    .map(entry => entry.trim())
+    .filter(entry => entry !== '');
+
+  return baggageEntries.reduce(
     ([baggageObj, baggageString], curr) => {
       const [key, val] = curr.split('=');
       if (SENTRY_BAGGAGE_KEY_PREFIX_REGEX.test(key)) {
@@ -122,16 +143,17 @@ export function parseBaggageString(inputBaggageString: string): Baggage {
  * it would only affect parts of the sentry baggage (@see Baggage interface).
  *
  * @param incomingBaggage the baggage header of the incoming request that might contain sentry entries
- * @param headerBaggageString possibly existing baggage header string added from a third party to request headers
+ * @param thirdPartyBaggageHeader possibly existing baggage header string or string[] added from a third
+ *        party to the request headers
  *
  * @return a merged and serialized baggage string to be propagated with the outgoing request
  */
-export function mergeAndSerializeBaggage(incomingBaggage?: Baggage, headerBaggageString?: string): string {
-  if (!incomingBaggage && !headerBaggageString) {
+export function mergeAndSerializeBaggage(incomingBaggage?: Baggage, thirdPartyBaggageHeader?: HttpHeaderValue): string {
+  if (!incomingBaggage && !thirdPartyBaggageHeader) {
     return '';
   }
 
-  const headerBaggage = (headerBaggageString && parseBaggageString(headerBaggageString)) || undefined;
+  const headerBaggage = (thirdPartyBaggageHeader && parseBaggageHeader(thirdPartyBaggageHeader)) || undefined;
   const thirdPartyHeaderBaggage = headerBaggage && getThirdPartyBaggage(headerBaggage);
 
   const finalBaggage = createBaggage(
@@ -150,14 +172,14 @@ export function mergeAndSerializeBaggage(incomingBaggage?: Baggage, headerBaggag
  *
  * Extracted this logic to a function because it's duplicated in a lot of places.
  *
- * @param rawBaggageString
+ * @param rawBaggageValue
  * @param sentryTraceHeader
  */
 export function parseBaggageSetMutability(
-  rawBaggageString: string | false | undefined | null,
+  rawBaggageValue: HttpHeaderValue | false | undefined,
   sentryTraceHeader: TraceparentData | string | false | undefined | null,
 ): Baggage {
-  const baggage = parseBaggageString(rawBaggageString || '');
+  const baggage = parseBaggageHeader(rawBaggageValue || '');
   if (!isSentryBaggageEmpty(baggage) || (sentryTraceHeader && isSentryBaggageEmpty(baggage))) {
     setBaggageImmutable(baggage);
   }

packages/utils/test/baggage.test.ts

@@ -7,8 +7,8 @@ import {
   isBaggageMutable,
   isSentryBaggageEmpty,
   mergeAndSerializeBaggage,
+  parseBaggageHeader,
   parseBaggageSetMutability,
-  parseBaggageString,
   serializeBaggage,
   setBaggageImmutable,
   setBaggageValue,
@@ -97,9 +97,10 @@ describe('Baggage', () => {
     });
   });
 
-  describe('parseBaggageString', () => {
+  describe('parseBaggageHeader', () => {
     it.each([
       ['parses an empty string', '', createBaggage({})],
+      ['parses a blank string', '     ', createBaggage({})],
       [
         'parses sentry values into baggage',
         'sentry-environment=production,sentry-release=10.0.2',
@@ -113,8 +114,32 @@ describe('Baggage', () => {
           'userId=alice,serverNode=DF%2028,isProduction=false',
         ),
       ],
-    ])('%s', (_: string, baggageString, baggage) => {
-      expect(parseBaggageString(baggageString)).toEqual(baggage);
+      [
+        'parses arbitrary baggage headers from string with empty and blank entries',
+        'userId=alice,    serverNode=DF%2028   , isProduction=false,   ,,sentry-environment=production,,sentry-release=10.0.2',
+        createBaggage(
+          { environment: 'production', release: '10.0.2' },
+          'userId=alice,serverNode=DF%2028,isProduction=false',
+        ),
+      ],
+      [
+        'parses a string array',
+        ['userId=alice', 'sentry-environment=production', 'foo=bar'],
+        createBaggage({ environment: 'production' }, 'userId=alice,foo=bar'),
+      ],
+      [
+        'parses a string array with items containing multiple entries',
+        ['userId=alice,   userName=bob', 'sentry-environment=production,sentry-release=1.0.1', 'foo=bar'],
+        createBaggage({ environment: 'production', release: '1.0.1' }, 'userId=alice,userName=bob,foo=bar'),
+      ],
+      [
+        'parses a string array with empty/blank entries',
+        ['', 'sentry-environment=production,sentry-release=1.0.1', '    ', 'foo=bar'],
+        createBaggage({ environment: 'production', release: '1.0.1' }, 'foo=bar'),
+      ],
+      ['ignorese other input types than string and string[]', 42, createBaggage({}, '')],
+    ])('%s', (_: string, baggageValue, baggage) => {
+      expect(parseBaggageHeader(baggageValue)).toEqual(baggage);
     });
   });