From aa8488bc941876e01f5c2a6d653c224bee5295e1 Mon Sep 17 00:00:00 2001 From: Marco Saia Date: Wed, 10 Dec 2025 11:19:35 +0100 Subject: [PATCH 1/2] Correct Baggage Header Handling * Handle Baggage Headers properly in DatadogTracingContext * Do not encode or drop user-defined baggage header entries * Do not double-encode entries * Remove duplicate insert of RUM Session ID --- packages/core/src/rum/__tests__/DdRum.test.ts | 95 ++++++++++++++- packages/core/src/rum/helper.ts | 46 ++++++-- .../DatadogTracingContext.ts | 25 +++- .../distributedTracingHeaders.ts | 7 -- .../__tests__/baggageHeaderUtils.test.ts | 96 +++++++++++++-- .../XHRProxy/baggageHeaderUtils.ts | 110 ++++++++++++++++-- 6 files changed, 335 insertions(+), 44 deletions(-) diff --git a/packages/core/src/rum/__tests__/DdRum.test.ts b/packages/core/src/rum/__tests__/DdRum.test.ts index e1c699fe1..0f3dd80b7 100644 --- a/packages/core/src/rum/__tests__/DdRum.test.ts +++ b/packages/core/src/rum/__tests__/DdRum.test.ts @@ -17,7 +17,14 @@ import { DdRum } from '../DdRum'; import type { ActionEventMapper } from '../eventMappers/actionEventMapper'; import type { ErrorEventMapper } from '../eventMappers/errorEventMapper'; import type { ResourceEventMapper } from '../eventMappers/resourceEventMapper'; -import { setCachedSessionId } from '../helper'; +import { + clearCachedAccountId, + clearCachedSessionId, + clearCachedUserId, + setCachedAccountId, + setCachedSessionId, + setCachedUserId +} from '../helper'; import { DatadogTracingContext } from '../instrumentation/resourceTracking/distributedTracing/DatadogTracingContext'; import { DatadogTracingIdentifier } from '../instrumentation/resourceTracking/distributedTracing/DatadogTracingIdentifier'; import { TracingIdFormat } from '../instrumentation/resourceTracking/distributedTracing/TracingIdentifier'; @@ -48,7 +55,9 @@ describe('DdRum', () => { beforeEach(() => { jest.clearAllMocks(); BufferSingleton.onInitialization(); - setCachedSessionId(undefined as any); + clearCachedSessionId(); + clearCachedUserId(); + clearCachedAccountId(); }); describe('Context validation', () => { @@ -1009,6 +1018,88 @@ describe('DdRum', () => { } }); + it('tracing context contains User ID in baggage when User ID is cached', () => { + for (let i = 0; i < 100; i++) { + const randomUserId = `test-${Math.random()}`; + + setCachedUserId(randomUserId); + const tracingContext = DdRum.getTracingContextForPropagators( + [ + PropagatorType.DATADOG, + PropagatorType.TRACECONTEXT, + PropagatorType.B3MULTI, + PropagatorType.B3 + ], + 100 + ); + + const requestHeaders = tracingContext.getHeadersForRequest(); + expect(requestHeaders).toHaveProperty('baggage'); + expect(requestHeaders['baggage']).toBe( + `user.id=${randomUserId}` + ); + + const resourceContext = tracingContext.getRumResourceContext(); + expect(resourceContext['baggage']).toBeUndefined(); + } + }); + + it('tracing context contains Account ID in baggage when Account ID is cached', () => { + for (let i = 0; i < 100; i++) { + const randomAccountId = `test-${Math.random()}`; + + setCachedAccountId(randomAccountId); + const tracingContext = DdRum.getTracingContextForPropagators( + [ + PropagatorType.DATADOG, + PropagatorType.TRACECONTEXT, + PropagatorType.B3MULTI, + PropagatorType.B3 + ], + 100 + ); + + const requestHeaders = tracingContext.getHeadersForRequest(); + expect(requestHeaders).toHaveProperty('baggage'); + expect(requestHeaders['baggage']).toBe( + `account.id=${randomAccountId}` + ); + + const resourceContext = tracingContext.getRumResourceContext(); + expect(resourceContext['baggage']).toBeUndefined(); + } + }); + + it('tracing context contains User ID, Account ID and Session ID in baggage when all session info is cached', () => { + for (let i = 0; i < 100; i++) { + const randomSessionId = `session-${Math.random()}`; + const randomUserId = `user-${Math.random()}`; + const randomAccountId = `account-${Math.random()}`; + + setCachedSessionId(randomSessionId); + setCachedUserId(randomUserId); + setCachedAccountId(randomAccountId); + const tracingContext = DdRum.getTracingContextForPropagators( + [ + PropagatorType.DATADOG, + PropagatorType.TRACECONTEXT, + PropagatorType.B3MULTI, + PropagatorType.B3 + ], + 100 + ); + + const requestHeaders = tracingContext.getHeadersForRequest(); + expect(requestHeaders).toHaveProperty('baggage'); + expect(requestHeaders['baggage']).toBe( + `session.id=${randomSessionId},user.id=${randomUserId},account.id=${randomAccountId}` + ); + + const resourceContext = tracingContext.getRumResourceContext(); + expect(resourceContext['baggage']).toBeUndefined(); + } + }); + it('injects headers and context correctly with all propagators and sampling rate (50% 0, 50% 100)', () => { for (let i = 0; i < 100; i++) { const tracingSamplingRate = diff --git a/packages/core/src/rum/helper.ts b/packages/core/src/rum/helper.ts index a153b79b5..7ea24fd03 100644 --- a/packages/core/src/rum/helper.ts +++ b/packages/core/src/rum/helper.ts @@ -1,36 +1,62 @@ +import { getGlobalInstance } from '../utils/singletonUtils'; + /* * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. * This product includes software developed at Datadog (https://www.datadoghq.com/). * Copyright 2016-Present Datadog, Inc. */ -let _cachedSessionId: string | undefined; -let _cachedUserId: string | undefined; -let _cachedAccountId: string | undefined; +const SESSION_INFO_MODULE = 'com.datadog.reactnative.sdk.session_info'; + +class _SessionInfo { + sessionId: string | undefined = undefined; + userId: string | undefined = undefined; + accountId: string | undefined = undefined; + + _reset() { + this.sessionId = undefined; + this.userId = undefined; + this.accountId = undefined; + } +} + +const SessionInfo = getGlobalInstance( + SESSION_INFO_MODULE, + () => new _SessionInfo() +); +// Helper functions to interact with the SessionInfo singleton export const getCachedSessionId = () => { - return _cachedSessionId; + return SessionInfo.sessionId; }; export const setCachedSessionId = (sessionId: string) => { - _cachedSessionId = sessionId; + SessionInfo.sessionId = sessionId; }; export const clearCachedSessionId = () => { - _cachedSessionId = undefined; + SessionInfo.sessionId = undefined; }; export const getCachedUserId = () => { - return _cachedUserId; + return SessionInfo.userId; }; export const setCachedUserId = (userId: string) => { - _cachedUserId = userId; + SessionInfo.userId = userId; +}; + +export const clearCachedUserId = () => { + SessionInfo.userId = undefined; }; export const getCachedAccountId = () => { - return _cachedAccountId; + return SessionInfo.accountId; }; export const setCachedAccountId = (accountId: string) => { - _cachedAccountId = accountId; + SessionInfo.accountId = accountId; +}; + +export const clearCachedAccountId = () => { + SessionInfo.accountId = undefined; }; diff --git a/packages/core/src/rum/instrumentation/resourceTracking/distributedTracing/DatadogTracingContext.ts b/packages/core/src/rum/instrumentation/resourceTracking/distributedTracing/DatadogTracingContext.ts index 25e5bb134..0f0617408 100644 --- a/packages/core/src/rum/instrumentation/resourceTracking/distributedTracing/DatadogTracingContext.ts +++ b/packages/core/src/rum/instrumentation/resourceTracking/distributedTracing/DatadogTracingContext.ts @@ -4,8 +4,11 @@ * Copyright 2016-Present Datadog, Inc. */ +import { formatBaggageHeader } from '../requestProxy/XHRProxy/baggageHeaderUtils'; + import { DatadogTracingIdentifier } from './DatadogTracingIdentifier'; import type { SpanId, TraceId } from './TracingIdentifier'; +import { BAGGAGE_HEADER_KEY } from './distributedTracingHeaders'; /** * An object that contains the tracing attributes as headers for network requests and attributes @@ -23,7 +26,7 @@ export class DatadogTracingContext { traceId: TraceId | undefined | void, spanId: SpanId | undefined | void ) { - this.headersForRequest = requestHeaders; + this.headersForRequest = this.processHeaders(requestHeaders); this.rumResourceContext = resourceContext; this.traceId = traceId ? new DatadogTracingIdentifier(traceId) @@ -78,4 +81,24 @@ export class DatadogTracingContext { inject(key, this.rumResourceContext[key]); }); } + + private processHeaders( + requestHeaders: { header: string; value: string }[] + ) { + const baggageHeaderEntries = requestHeaders + .filter( + ({ header, value }) => header === BAGGAGE_HEADER_KEY && value + ) + .map(({ value }) => value); + const baggageHeader = formatBaggageHeader( + new Set(baggageHeaderEntries) + ); + if (!baggageHeader) { + return requestHeaders; + } + + return requestHeaders + .filter(({ header }) => header !== BAGGAGE_HEADER_KEY) + .concat({ header: BAGGAGE_HEADER_KEY, value: baggageHeader }); + } } diff --git a/packages/core/src/rum/instrumentation/resourceTracking/distributedTracing/distributedTracingHeaders.ts b/packages/core/src/rum/instrumentation/resourceTracking/distributedTracing/distributedTracingHeaders.ts index 8ad9137c7..033775a17 100644 --- a/packages/core/src/rum/instrumentation/resourceTracking/distributedTracing/distributedTracingHeaders.ts +++ b/packages/core/src/rum/instrumentation/resourceTracking/distributedTracing/distributedTracingHeaders.ts @@ -167,13 +167,6 @@ export const getTracingHeadersFromAttributes = ( } } - if (hasDatadogOrW3CPropagator && tracingAttributes.rumSessionId) { - headers.push({ - header: BAGGAGE_HEADER_KEY, - value: `${DD_RUM_SESSION_ID_TAG}=${tracingAttributes.rumSessionId}` - }); - } - return headers; }; diff --git a/packages/core/src/rum/instrumentation/resourceTracking/requestProxy/XHRProxy/__tests__/baggageHeaderUtils.test.ts b/packages/core/src/rum/instrumentation/resourceTracking/requestProxy/XHRProxy/__tests__/baggageHeaderUtils.test.ts index eee59838f..bf2fa5f2b 100644 --- a/packages/core/src/rum/instrumentation/resourceTracking/requestProxy/XHRProxy/__tests__/baggageHeaderUtils.test.ts +++ b/packages/core/src/rum/instrumentation/resourceTracking/requestProxy/XHRProxy/__tests__/baggageHeaderUtils.test.ts @@ -20,10 +20,32 @@ describe('formatBaggageHeader', () => { expect(logSpy).not.toHaveBeenCalled(); }); - it('should percent-encode spaces and non-ASCII characters in values', () => { + it('should not encode non-datadog-specific property values and log a warning', () => { const entries = new Set(['user=Amélie', 'region=us east']); const result = formatBaggageHeader(entries); - expect(result).toBe('user=Am%C3%A9lie,region=us%20east'); + expect(result).toBe('user=Amélie,region=us east'); + expect(logSpy).toHaveBeenCalledWith( + expect.stringContaining('invalid baggage header value detected'), + SdkVerbosity.WARN + ); + }); + + it('should only encode datadog-specific property values', () => { + const entries = new Set([ + 'user=Amélie', + 'session.id=example session id', + 'user.id=example user id', + 'account.id=example account id', + 'region=us east' + ]); + const result = formatBaggageHeader(entries); + expect(result).toBe( + 'user=Amélie,session.id=example%20session%20id,user.id=example%20user%20id,account.id=example%20account%20id,region=us east' + ); + expect(logSpy).toHaveBeenCalledWith( + expect.stringContaining('invalid baggage header value detected'), + SdkVerbosity.WARN + ); }); it('should support properties with and without values', () => { @@ -38,32 +60,32 @@ describe('formatBaggageHeader', () => { expect(result).toBe('foo=bar;p1=one;p2'); }); - it('should skip invalid entries without crashing', () => { + it('should warn of invalid entries without crashing', () => { const entries = new Set(['valid=ok', 'invalidEntry']); const result = formatBaggageHeader(entries); expect(result).toBe('valid=ok'); expect(logSpy).toHaveBeenCalledWith( expect.stringContaining('Dropped invalid baggage header entry'), - SdkVerbosity.WARN + SdkVerbosity.ERROR ); }); - it('should skip entries with invalid key (non-token)', () => { + it('should warn of entries with invalid key (non-token)', () => { const entries = new Set(['in valid=value', 'user=ok']); const result = formatBaggageHeader(entries); - expect(result).toBe('user=ok'); + expect(result).toBe('in valid=value,user=ok'); expect(logSpy).toHaveBeenCalledWith( - expect.stringContaining('key not compliant'), + expect.stringContaining('invalid baggage header keys detected'), SdkVerbosity.WARN ); }); - it('should skip invalid properties (bad property key)', () => { + it('should warn of invalid properties (bad property key)', () => { const entries = new Set(['user=ok;invalid key=value;good=yes']); const result = formatBaggageHeader(entries); - expect(result).toBe('user=ok;good=yes'); + expect(result).toBe('user=ok;invalid key=value;good=yes'); expect(logSpy).toHaveBeenCalledWith( - expect.stringContaining('property key not compliant'), + expect.stringContaining('invalid baggage header key-value'), SdkVerbosity.WARN ); }); @@ -106,12 +128,62 @@ describe('formatBaggageHeader', () => { it('should trim keys and values', () => { const entries = new Set([ - 'traceId=abc123;sampled=true;debug', + 'traceId=abc123; sampled=true; debug', 'test1 = this is a test' ]); const result = formatBaggageHeader(entries); expect(result).toBe( - 'traceId=abc123;sampled=true;debug,test1=this%20is%20a%20test' + 'traceId=abc123;sampled=true;debug,test1=this is a test' + ); + }); + + it('should not double encode non-datadog-specific percent-encoded values', () => { + const entries = new Set([ + 'user=foo%20bar', + 'name=Am%C3%A9lie', + 'path=%2Fapi%2Fv1%2Fusers' + ]); + + const result = formatBaggageHeader(entries); + + expect(result).toBe( + 'user=foo%20bar,name=Am%C3%A9lie,path=%2Fapi%2Fv1%2Fusers' + ); + }); + + it('should not double encode non-datadog-specific percent-encoded property values', () => { + const entries = new Set([ + 'traceId=abc123;user=Am%C3%A9lie;note=hello%20world' + ]); + + const result = formatBaggageHeader(entries); + + expect(result).toBe( + 'traceId=abc123;user=Am%C3%A9lie;note=hello%20world' + ); + }); + + it('should re-encode mixed encoded/decoded datadog-specific values only once', () => { + const entries = new Set([ + // should not be encoded because "user" is not datadog-specific + 'user=hello%20world test', + // partially encoded: "%25" + literal space + 'session.id=example%20session id', + // contains a literal "%", not a valid escape sequence + 'user.id=example%user id', + // not encoded + 'account.id=example account id' + ]); + + const result = formatBaggageHeader(entries); + + expect(result).toBe( + 'user=hello%20world test,session.id=example%20session%20id,user.id=example%user%20id,account.id=example%20account%20id' + ); + + expect(logSpy).toHaveBeenCalledWith( + expect.stringContaining('invalid baggage header value detected'), + SdkVerbosity.WARN ); }); }); diff --git a/packages/core/src/rum/instrumentation/resourceTracking/requestProxy/XHRProxy/baggageHeaderUtils.ts b/packages/core/src/rum/instrumentation/resourceTracking/requestProxy/XHRProxy/baggageHeaderUtils.ts index 7094ec4e8..5861e04b2 100644 --- a/packages/core/src/rum/instrumentation/resourceTracking/requestProxy/XHRProxy/baggageHeaderUtils.ts +++ b/packages/core/src/rum/instrumentation/resourceTracking/requestProxy/XHRProxy/baggageHeaderUtils.ts @@ -6,6 +6,11 @@ import { InternalLog } from '../../../../../InternalLog'; import { SdkVerbosity } from '../../../../../SdkVerbosity'; +import { + DD_RUM_ACCOUNT_ID_TAG, + DD_RUM_SESSION_ID_TAG, + DD_RUM_USER_ID_TAG +} from '../../distributedTracing/distributedTracingHeaders'; // The resulting baggage-string should contain 64 list-members or less (https://www.w3.org/TR/baggage/#limits) const MAX_MEMBERS = 64; @@ -32,7 +37,7 @@ export function formatBaggageHeader(entries: Set): string | null { if (!rawEntry.includes('=')) { InternalLog.log( 'XHRProxy: Dropped invalid baggage header entry - expected format "key=value".', - SdkVerbosity.WARN + SdkVerbosity.ERROR ); continue; } @@ -43,23 +48,30 @@ export function formatBaggageHeader(entries: Set): string | null { if (idx <= 0) { InternalLog.log( "XHRProxy: Dropped invalid baggage header entry - no '=' or empty key", - SdkVerbosity.WARN + SdkVerbosity.ERROR ); continue; } const rawKey = mainPart.slice(0, idx).trim(); const rawValue = mainPart.slice(idx + 1).trim(); + const isDatadogKey = isDatadogPropertyKey(rawKey); - if (!TOKEN_REGEX.test(rawKey)) { + if (!isDatadogKey && !TOKEN_REGEX.test(rawKey)) { InternalLog.log( - 'XHRProxy: Dropped invalid baggage header entry - key not compliant to RFC 7230 token grammar', + 'XHRProxy: invalid baggage header keys detected (not RFC 7230-compliant); functionality may be affected.', SdkVerbosity.WARN ); - continue; } - const encodedValue = encodeValue(rawValue); + // Only encode datadog-specific properties + const encodedValue = isDatadogKey ? encodeValue(rawValue) : rawValue; + if (!isDatadogKey && !isCompliantBaggageValue(rawValue)) { + InternalLog.log( + 'XHRProxy: invalid baggage header value detected (not RFC-compliant); functionality may be affected.', + SdkVerbosity.WARN + ); + } // Handle properties const properties: string[] = []; @@ -75,10 +87,9 @@ export function formatBaggageHeader(entries: Set): string | null { const propKey = trimmed.trim(); if (!TOKEN_REGEX.test(propKey)) { InternalLog.log( - 'XHRProxy: Dropped invalid baggage header entry - property key not compliant to RFC 7230 token grammar', + 'XHRProxy: invalid baggage header property key detected (not RFC 7230-compliant); functionality may be affected.', SdkVerbosity.WARN ); - continue; } properties.push(propKey); } else { @@ -87,10 +98,9 @@ export function formatBaggageHeader(entries: Set): string | null { const propVal = trimmed.slice(eqIdx + 1).trim(); if (!TOKEN_REGEX.test(propKey)) { InternalLog.log( - 'XHRProxy: Dropped invalid baggage header entry - key-value property key not compliant to RFC 7230 token grammar', + 'XHRProxy: invalid baggage header key-value property key detected (not RFC 7230-compliant); functionality may be affected.', SdkVerbosity.WARN ); - continue; } properties.push(`${propKey}=${encodeValue(propVal)}`); } @@ -149,13 +159,29 @@ function getBaggageHeaderSafeChars(): Set { return safeChars; } -/* +/** + * Checks if the given key is a Datadog-specific baggage property key. + * @param key the baggage property key. + * @returns true if the key is Datadog-specific, false otherwise. + */ +function isDatadogPropertyKey(key: string): boolean { + return ( + key === DD_RUM_SESSION_ID_TAG || + key === DD_RUM_USER_ID_TAG || + key === DD_RUM_ACCOUNT_ID_TAG + ); +} + +/** * Percent-encode all characters outside baggage-octet range. + * @param raw The raw baggage value. + * @returns the encoded baggage value. */ function encodeValue(raw: string): string { + const decoded = tryDecodeValue(raw); const safeChars = getBaggageHeaderSafeChars(); let result = ''; - for (const ch of Array.from(raw)) { + for (const ch of Array.from(decoded)) { if (safeChars.has(ch)) { result += ch; } else { @@ -170,3 +196,63 @@ function encodeValue(raw: string): string { } return result; } + +/** + * Try to decode the given baggage value, to avoid double-encoding. + * @param rawValue The raw baggage value. + * @returns the decoded value, or the original raw value if decoding fails. + */ +function tryDecodeValue(rawValue: string): string { + try { + // Try interpreting it as already-encoded + return decodeURIComponent(rawValue); + } catch { + return rawValue; + } +} + +/** + * Check if a character is a hexadecimal digit. + * @param ch The character to check. + * @returns true if the character is a hexadecimal digit, false otherwise. + */ +function isHexDigit(ch: string): boolean { + return ( + (ch >= '0' && ch <= '9') || + (ch >= 'A' && ch <= 'F') || + (ch >= 'a' && ch <= 'f') + ); +} + +/** + * Check if a raw baggage value is RFC-compliant: + * - characters are in baggage-octet, OR + * - part of a valid %HH percent-encoded sequence. + */ +function isCompliantBaggageValue(raw: string): boolean { + const safeChars = getBaggageHeaderSafeChars(); + + for (let i = 0; i < raw.length; i++) { + const ch = raw[i]; + + // Directly allowed character + if (safeChars.has(ch)) { + continue; + } + + // Percent-encoded triplet: %HH + if (ch === '%' && i + 2 < raw.length) { + const h1 = raw[i + 1]; + const h2 = raw[i + 2]; + if (isHexDigit(h1) && isHexDigit(h2)) { + i += 2; // skip the two hex digits + continue; + } + } + + // Not safe and not a valid %HH + return false; + } + + return true; +} From e47e636aa98fe5dbcc3d0bcfbd140bd55fbe3a33 Mon Sep 17 00:00:00 2001 From: Marco Saia Date: Mon, 15 Dec 2025 10:47:23 +0100 Subject: [PATCH 2/2] Refactored baggage header parsing for readability --- .../XHRProxy/baggageHeaderUtils.ts | 32 +++++++++++-------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/packages/core/src/rum/instrumentation/resourceTracking/requestProxy/XHRProxy/baggageHeaderUtils.ts b/packages/core/src/rum/instrumentation/resourceTracking/requestProxy/XHRProxy/baggageHeaderUtils.ts index 5861e04b2..305bfa896 100644 --- a/packages/core/src/rum/instrumentation/resourceTracking/requestProxy/XHRProxy/baggageHeaderUtils.ts +++ b/packages/core/src/rum/instrumentation/resourceTracking/requestProxy/XHRProxy/baggageHeaderUtils.ts @@ -56,21 +56,27 @@ export function formatBaggageHeader(entries: Set): string | null { const rawKey = mainPart.slice(0, idx).trim(); const rawValue = mainPart.slice(idx + 1).trim(); const isDatadogKey = isDatadogPropertyKey(rawKey); + let encodedValue: string; - if (!isDatadogKey && !TOKEN_REGEX.test(rawKey)) { - InternalLog.log( - 'XHRProxy: invalid baggage header keys detected (not RFC 7230-compliant); functionality may be affected.', - SdkVerbosity.WARN - ); - } + if (isDatadogKey) { + // Only encode datadog-specific properties + encodedValue = isDatadogKey ? encodeValue(rawValue) : rawValue; + } else { + if (!TOKEN_REGEX.test(rawKey)) { + InternalLog.log( + 'XHRProxy: invalid baggage header keys detected (not RFC 7230-compliant); functionality may be affected.', + SdkVerbosity.WARN + ); + } + if (!isCompliantBaggageValue(rawValue)) { + InternalLog.log( + 'XHRProxy: invalid baggage header value detected (not RFC-compliant); functionality may be affected.', + SdkVerbosity.WARN + ); + } - // Only encode datadog-specific properties - const encodedValue = isDatadogKey ? encodeValue(rawValue) : rawValue; - if (!isDatadogKey && !isCompliantBaggageValue(rawValue)) { - InternalLog.log( - 'XHRProxy: invalid baggage header value detected (not RFC-compliant); functionality may be affected.', - SdkVerbosity.WARN - ); + // non-datadog properties are not encoded + encodedValue = rawValue; } // Handle properties