Skip to content

Commit d992bca

Browse files
committed
Use quoted-string escaping instead of URL-encoding
1 parent 5518dab commit d992bca

File tree

4 files changed

+22
-17
lines changed

4 files changed

+22
-17
lines changed

packages/remix/src/client/serverTimingTracePropagation.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,8 @@ function parseServerTimingTrace(serverTiming: readonly PerformanceServerTiming[]
6161
if (entry.name === 'sentry-trace') {
6262
sentryTrace = entry.description;
6363
} else if (entry.name === 'baggage') {
64-
try {
65-
// Baggage is URL-encoded in Server-Timing header
66-
baggage = decodeURIComponent(entry.description);
67-
} catch {
68-
baggage = entry.description;
69-
}
64+
// Baggage is escaped for quoted-string context (backslash-escaped quotes and backslashes)
65+
baggage = entry.description.replace(/\\"/g, '"').replace(/\\\\/g, '\\');
7066
}
7167
}
7268

packages/remix/src/server/serverTimingTracePropagation.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,10 @@ export function generateSentryServerTimingHeader(options: ServerTimingTraceOptio
8888
metrics.push(`sentry-trace;desc="${sentryTrace}"`);
8989

9090
if (opts.includeBaggage && baggage) {
91-
// URL-encode baggage to handle special characters
92-
metrics.push(`baggage;desc="${encodeURIComponent(baggage)}"`);
91+
// Escape special characters for use inside a quoted-string (RFC 7230)
92+
// We escape backslashes and double quotes to prevent injection
93+
const escapedBaggage = baggage.replace(/\\/g, '\\\\').replace(/"/g, '\\"');
94+
metrics.push(`baggage;desc="${escapedBaggage}"`);
9395
}
9496

9597
return metrics.join(', ');

packages/remix/test/client/serverTimingTracePropagation.test.ts

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,8 @@ describe('serverTimingTracePropagation', () => {
137137
responseStart: 100,
138138
serverTiming: [
139139
{ name: 'sentry-trace', description: sentryTrace },
140-
{ name: 'baggage', description: encodeURIComponent(baggage) },
140+
// Baggage is escaped for quoted-string context (not URL-encoded)
141+
{ name: 'baggage', description: baggage },
141142
],
142143
},
143144
]);
@@ -232,18 +233,20 @@ describe('serverTimingTracePropagation', () => {
232233
expect(result).toBeNull();
233234
});
234235

235-
it('decodes URL-encoded baggage', () => {
236+
it('unescapes backslash-escaped baggage', () => {
236237
const traceId = '12345678901234567890123456789012';
237238
const spanId = '1234567890123456';
238239
const sentryTrace = `${traceId}-${spanId}-1`;
239240
const baggage = 'sentry-environment=production,sentry-release=1.0.0';
241+
// Simulate escaped baggage (backslash-escaped quotes and backslashes)
242+
const escapedBaggage = baggage.replace(/\\/g, '\\\\').replace(/"/g, '\\"');
240243

241244
mockPerformance.getEntriesByType = vi.fn().mockReturnValue([
242245
{
243246
responseStart: 100,
244247
serverTiming: [
245248
{ name: 'sentry-trace', description: sentryTrace },
246-
{ name: 'baggage', description: encodeURIComponent(baggage) },
249+
{ name: 'baggage', description: escapedBaggage },
247250
],
248251
},
249252
]);
@@ -253,25 +256,27 @@ describe('serverTimingTracePropagation', () => {
253256
expect(result?.baggage).toBe(baggage);
254257
});
255258

256-
it('handles malformed URL-encoded baggage gracefully', () => {
259+
it('handles baggage with special characters', () => {
257260
const traceId = '12345678901234567890123456789012';
258261
const spanId = '1234567890123456';
259262
const sentryTrace = `${traceId}-${spanId}-1`;
260-
const malformedBaggage = '%E0%A4%A';
263+
// Baggage with escaped quotes (simulating what server sends)
264+
const escapedBaggage = 'key=value\\"with\\"quotes';
261265

262266
mockPerformance.getEntriesByType = vi.fn().mockReturnValue([
263267
{
264268
responseStart: 100,
265269
serverTiming: [
266270
{ name: 'sentry-trace', description: sentryTrace },
267-
{ name: 'baggage', description: malformedBaggage },
271+
{ name: 'baggage', description: escapedBaggage },
268272
],
269273
},
270274
]);
271275

272276
const result = getNavigationTraceContext();
273277

274-
expect(result?.baggage).toBe(malformedBaggage);
278+
// Should unescape the backslash-escaped quotes
279+
expect(result?.baggage).toBe('key=value"with"quotes');
275280
});
276281
});
277282

packages/remix/test/server/serverTimingTracePropagation.test.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,8 @@ describe('serverTimingTracePropagation', () => {
101101

102102
expect(result).toContain('sentry-trace;desc="12345678901234567890123456789012-1234567890123456-1"');
103103
expect(result).toContain('baggage;desc=');
104-
expect(result).toContain(encodeURIComponent('sentry-environment=production,sentry-release=1.0.0'));
104+
// Baggage is escaped for quoted-string context (not URL-encoded)
105+
expect(result).toContain('sentry-environment=production,sentry-release=1.0.0');
105106
});
106107

107108
it('generates header without baggage when includeBaggage is false', () => {
@@ -138,7 +139,8 @@ describe('serverTimingTracePropagation', () => {
138139
const result = generateSentryServerTimingHeader();
139140

140141
expect(result).toContain('sentry-trace;desc="fallback-trace-id-1234567890123456-0"');
141-
expect(result).toContain(encodeURIComponent('sentry-fallback=true'));
142+
// Baggage is escaped for quoted-string context (not URL-encoded)
143+
expect(result).toContain('sentry-fallback=true');
142144
});
143145

144146
it('works in Cloudflare environment', () => {

0 commit comments

Comments
 (0)