Skip to content

Commit 1e0cb04

Browse files
authored
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...?
1 parent de65590 commit 1e0cb04

File tree

2 files changed

+1
-101
lines changed

2 files changed

+1
-101
lines changed

packages/core/src/utils/traceData.ts

Lines changed: 1 addition & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -44,39 +44,12 @@ export function getTraceData(options: { span?: Span } = {}): SerializedTraceData
4444
return {};
4545
}
4646

47-
const validBaggage = isValidBaggageString(baggage);
48-
if (!validBaggage) {
49-
logger.warn('Invalid baggage data. Not returning "baggage" value');
50-
}
51-
5247
return {
5348
'sentry-trace': sentryTrace,
54-
...(validBaggage && { baggage }),
49+
baggage,
5550
};
5651
}
5752

58-
/**
59-
* Tests string against baggage spec as defined in:
60-
*
61-
* - W3C Baggage grammar: https://www.w3.org/TR/baggage/#definition
62-
* - RFC7230 token definition: https://datatracker.ietf.org/doc/html/rfc7230#section-3.2.6
63-
*
64-
* exported for testing
65-
*/
66-
export function isValidBaggageString(baggage?: string): boolean {
67-
if (!baggage || !baggage.length) {
68-
return false;
69-
}
70-
const keyRegex = "[-!#$%&'*+.^_`|~A-Za-z0-9]+";
71-
const valueRegex = '[!#-+-./0-9:<=>?@A-Z\\[\\]a-z{-}]+';
72-
const spaces = '\\s*';
73-
// eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor -- RegExp for readability, no user input
74-
const baggageRegex = new RegExp(
75-
`^${keyRegex}${spaces}=${spaces}${valueRegex}(${spaces},${spaces}${keyRegex}${spaces}=${spaces}${valueRegex})*$`,
76-
);
77-
return baggageRegex.test(baggage);
78-
}
79-
8053
/**
8154
* Get a sentry-trace header value for the given scope.
8255
*/

packages/core/test/lib/utils/traceData.test.ts

Lines changed: 0 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import {
1313
import { getAsyncContextStrategy } from '../../../src/asyncContext';
1414
import { freezeDscOnSpan } from '../../../src/tracing/dynamicSamplingContext';
1515

16-
import { isValidBaggageString } from '../../../src/utils/traceData';
1716
import type { TestClientOptions } from '../../mocks/client';
1817
import { TestClient, getDefaultTestClientOptions } from '../../mocks/client';
1918

@@ -281,75 +280,3 @@ describe('getTraceData', () => {
281280
expect(traceData).toEqual({});
282281
});
283282
});
284-
285-
describe('isValidBaggageString', () => {
286-
it.each([
287-
'sentry-environment=production',
288-
'sentry-environment=staging,sentry-public_key=key,sentry-trace_id=abc',
289-
// @ is allowed in values
290-
291-
// spaces are allowed around the delimiters
292-
'sentry-environment=staging , sentry-public_key=key ,[email protected]',
293-
'sentry-environment=staging , thirdparty=value ,[email protected]',
294-
// these characters are explicitly allowed for keys in the baggage spec:
295-
"!#$%&'*+-.^_`|~1234567890abcxyzABCXYZ=true",
296-
// special characters in values are fine (except for ",;\ - see other test)
297-
'key=(value)',
298-
'key=[{(value)}]',
299-
'key=some$value',
300-
'key=more#value',
301-
'key=max&value',
302-
'key=max:value',
303-
'key=x=value',
304-
])('returns true if the baggage string is valid (%s)', baggageString => {
305-
expect(isValidBaggageString(baggageString)).toBe(true);
306-
});
307-
308-
it.each([
309-
// baggage spec doesn't permit leading spaces
310-
' sentry-environment=production,sentry-publickey=key,sentry-trace_id=abc',
311-
// no spaces in keys or values
312-
'sentry-public key=key',
313-
'sentry-publickey=my key',
314-
// no delimiters ("(),/:;<=>?@[\]{}") in keys
315-
'asdf(x=value',
316-
'asdf)x=value',
317-
'asdf,x=value',
318-
'asdf/x=value',
319-
'asdf:x=value',
320-
'asdf;x=value',
321-
'asdf<x=value',
322-
'asdf>x=value',
323-
'asdf?x=value',
324-
'asdf@x=value',
325-
'asdf[x=value',
326-
'asdf]x=value',
327-
'asdf\\x=value',
328-
'asdf{x=value',
329-
'asdf}x=value',
330-
// no ,;\" in values
331-
'key=va,lue',
332-
'key=va;lue',
333-
'key=va\\lue',
334-
'key=va"lue"',
335-
// baggage headers can have properties but we currently don't support them
336-
'sentry-environment=production;prop1=foo;prop2=bar,nextkey=value',
337-
// no fishy stuff
338-
'absolutely not a valid baggage string',
339-
'val"/><script>alert("xss")</script>',
340-
'something"/>',
341-
'<script>alert("xss")</script>',
342-
'/>',
343-
'" onblur="alert("xss")',
344-
])('returns false if the baggage string is invalid (%s)', baggageString => {
345-
expect(isValidBaggageString(baggageString)).toBe(false);
346-
});
347-
348-
it('returns false if the baggage string is empty', () => {
349-
expect(isValidBaggageString('')).toBe(false);
350-
});
351-
352-
it('returns false if the baggage string is empty', () => {
353-
expect(isValidBaggageString(undefined)).toBe(false);
354-
});
355-
});

0 commit comments

Comments
 (0)