Skip to content

Commit

Permalink
fix: Fix encoding-related issues with non-utf8 chars (#659)
Browse files Browse the repository at this point in the history
This fixes the issue certain characters getting lost or changed during
the implicit
and forced UTF-8 encoding, namely certain ANSI-escape characters when we
capture
them as breadcrumbs. This was breaking NextJS recently.

The mechanism is opt-in from Sidecar side and the new overlay
automatically opts
in to fix the issue. The new overlay is also capable of processing
messages w/o
base64 encoding so this change is both backwards and forwards compatible
meaning
a new version of overlay can work with an old sidecar and a new version
of sidecar
can work with an older overlay. That said to get the fix, both should be
on the new
version, opting into base64 encoding.
BYK authored Jan 20, 2025
1 parent 356890c commit 277bd76
Showing 18 changed files with 107 additions and 60 deletions.
17 changes: 17 additions & 0 deletions .changeset/tricky-seahorses-float.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
'@spotlightjs/overlay': minor
'@spotlightjs/sidecar': minor
---

Add base64 encoding for envelope passing

This fixes the issue certain characters getting lost or changed during the implicit
and forced UTF-8 encoding, namely certain ANSI-escape characters when we capture
them as breadcrumbs. This was breaking NextJS recently.

The mechanism is opt-in from Sidecar side and the new overlay automatically opts
in to fix the issue. The new overlay is also capable of processing messages w/o
base64 encoding so this change is both backwards and forwards compatible meaning
a new version of overlay can work with an old sidecar and a new version of sidecar
can work with an older overlay. That said to get the fix, both should be on the new
version, opting into base64 encoding.

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion packages/overlay/_fixtures/send_to_sidecar.cjs
Original file line number Diff line number Diff line change
@@ -8,7 +8,7 @@ const zlib = require('node:zlib');
async function sendData(filePath) {
let data;
try {
data = await fs.readFile(filePath, 'binary');
data = await fs.readFile(filePath);
} catch (err) {
console.error(`Error reading file ${filePath}: ${err}`);
return;
28 changes: 22 additions & 6 deletions packages/overlay/src/App.tsx
Original file line number Diff line number Diff line change
@@ -10,6 +10,7 @@ import useKeyPress from './lib/useKeyPress';
import { connectToSidecar } from './sidecar';
import type { NotificationCount, SpotlightOverlayOptions } from './types';
import { SPOTLIGHT_OPEN_CLASS_NAME } from './constants';
import { base64Decode } from './lib/base64';

type AppProps = Omit<SpotlightOverlayOptions, 'debug' | 'injectImmediately'> &
Required<Pick<SpotlightOverlayOptions, 'sidecarUrl'>>;
@@ -40,11 +41,18 @@ export default function App({
for (const integration of integrations) {
result[integration.name] = [];
for (const contentType in initialEvents) {
if (!integration.forwardedContentType?.includes(contentType)) {
const contentTypeBits = contentType.split(';');
const contentTypeWithoutEncoding = contentTypeBits[0];
if (!integration.forwardedContentType?.includes(contentTypeWithoutEncoding)) {
continue;
}
for (const data of initialEvents[contentType]) {
const processedEvent = processEvent(contentType, { data }, integration);
const shouldUseBase64 = contentTypeBits[contentTypeBits.length - 1] === 'base64';
for (const data of initialEvents[contentTypeWithoutEncoding]) {
const processedEvent = processEvent(
contentTypeWithoutEncoding,
{ data: shouldUseBase64 ? base64Decode(data as string) : data },
integration,
);
if (processedEvent) {
result[integration.name].push(processedEvent);
}
@@ -97,6 +105,7 @@ export default function App({

// `contentType` could for example be "application/x-sentry-envelope"
result[contentType] = listener;
result[`${contentType};base64`] = ({ data }) => listener({ data: base64Decode(data as string) });
}
return result;
}, [integrations]);
@@ -110,12 +119,19 @@ export default function App({

const dispatchToContentTypeListener = useCallback(
({ contentType, data }: EventData) => {
const listener = contentTypeListeners[contentType];
const contentTypeBits = contentType.split(';');
const contentTypeWithoutEncoding = contentTypeBits[0];

const listener = contentTypeListeners[contentTypeWithoutEncoding];
if (!listener) {
log('Got event for unknown content type:', contentType);
log('Got event for unknown content type:', contentTypeWithoutEncoding);
return;
}
listener({ data });
if (contentTypeBits[contentTypeBits.length - 1] === 'base64') {
listener({ data: base64Decode(data as string) });
} else {
listener({ data });
}
},
[contentTypeListeners],
);
4 changes: 2 additions & 2 deletions packages/overlay/src/index.tsx
Original file line number Diff line number Diff line change
@@ -17,8 +17,8 @@ import { activateLogger, log } from './lib/logger';
import { SpotlightContextProvider } from './lib/useSpotlightContext';
import { React, ReactDOM } from './react-instance';
import type { SpotlightOverlayOptions, WindowWithSpotlight } from './types';
import { removeURLSuffix } from './utils/removeURLSuffix';
import initSentry from './utils/instrumentation';
import { removeURLSuffix } from './lib/removeURLSuffix';
import initSentry from './lib/instrumentation';

export { default as console } from './integrations/console/index';
export { default as hydrationError } from './integrations/hydration-error/index';
Original file line number Diff line number Diff line change
@@ -2,7 +2,7 @@ import { useState } from 'react';
import CopyToClipboard from '../../../../../components/CopyToClipboard';
import OpenInEditor from '../../../../../components/OpenInEditor';
import classNames from '../../../../../lib/classNames';
import { renderValue } from '../../../../../utils/values';
import { renderValue } from '../../../../../lib/values';
import type { EventFrame, FrameVars } from '../../../types';

function resolveFilename(filename: string) {
Original file line number Diff line number Diff line change
@@ -7,7 +7,7 @@ import fs from 'node:fs';
describe('SentryDataCache', () => {
// We need to refactor this to make it actually testable
test('Process Envelope', () => {
const envelope = fs.readFileSync('./_fixtures/envelope_javascript.txt', 'utf-8');
const envelope = fs.readFileSync('./_fixtures/envelope_javascript.txt');
const processedEnvelope = processEnvelope({ data: envelope, contentType: 'test' });
expect(
sentryDataCache.pushEnvelope({ envelope: processedEnvelope.event, rawEnvelope: processedEnvelope.rawEvent }),
Original file line number Diff line number Diff line change
@@ -3,7 +3,7 @@ import { CONTEXT_LINES_ENDPOINT } from '@spotlightjs/sidecar/constants';
import { DEFAULT_SIDECAR_URL } from '~/constants';
import { RawEventContext } from '~/integrations/integration';
import { log } from '../../../lib/logger';
import { generate_uuidv4 } from '../../../lib/uuid';
import { generateUuidv4 } from '../../../lib/uuid';
import { Sdk, SentryErrorEvent, SentryEvent, SentryTransactionEvent, Span, Trace } from '../types';
import { getNativeFetchImplementation } from '../utils/fetch';
import { sdkToPlatform } from '../utils/sdkToPlatform';
@@ -115,7 +115,7 @@ class SentryDataCache {
},
) {
if (!event.event_id) {
event.event_id = generate_uuidv4();
event.event_id = generateUuidv4();
}

if (this.eventIds.has(event.event_id)) return;
@@ -245,7 +245,7 @@ class SentryDataCache {
}

subscribe(...args: Subscription) {
const id = generate_uuidv4();
const id = generateUuidv4();
this.subscribers.set(id, args);

return () => {
32 changes: 16 additions & 16 deletions packages/overlay/src/integrations/sentry/index.spec.ts
Original file line number Diff line number Diff line change
@@ -7,39 +7,39 @@ import sentryDataCache from './data/sentryDataCache';

describe('Sentry Integration', () => {
test('Process Envelope Empty', () => {
const envelope = fs.readFileSync('./_fixtures/envelope_empty.txt', 'binary');
const envelope = fs.readFileSync('./_fixtures/envelope_empty.txt');
expect(processEnvelope({ data: envelope, contentType: 'test' })).not.toBe(undefined);
});

test('Process Envelope', () => {
const envelope = fs.readFileSync('./_fixtures/envelope_javascript.txt', 'binary');
const envelope = fs.readFileSync('./_fixtures/envelope_javascript.txt');
expect(processEnvelope({ data: envelope, contentType: 'test' })).not.toBe(undefined);
});

test('Process Python Transaction Envelope', () => {
const envelope = fs.readFileSync('./_fixtures/envelope_python.txt', 'binary');
const envelope = fs.readFileSync('./_fixtures/envelope_python.txt');
expect(processEnvelope({ data: envelope, contentType: 'test' })).not.toBe(undefined);
});

test('Process PHP Transaction Envelope', () => {
const envelope = fs.readFileSync('./_fixtures/envelope_php.txt', 'binary');
const envelope = fs.readFileSync('./_fixtures/envelope_php.txt');
const processedEnvelope = processEnvelope({ data: envelope, contentType: 'test' });
expect(processedEnvelope).not.toBe(undefined);
expect((processedEnvelope.event[1][0][1] as any).type).toEqual('transaction');
});

test('Process Java Transaction Envelope', () => {
const envelope = fs.readFileSync('./_fixtures/envelope_java.txt', 'binary');
const envelope = fs.readFileSync('./_fixtures/envelope_java.txt');
const processedEnvelope = processEnvelope({ data: envelope, contentType: 'test' });
expect(processedEnvelope.event).not.toBe(undefined);
expect((processedEnvelope.event[1][0][1] as any).type).toEqual('transaction');
});

test('Process Astro SSR pageload (BE -> FE) trace', () => {
const nodeEnvelope = fs.readFileSync('./_fixtures/envelope_astro_ssr_node.txt', 'binary');
const nodeEnvelope = fs.readFileSync('./_fixtures/envelope_astro_ssr_node.txt');
const processedNodeEnvelope = processEnvelope({ data: nodeEnvelope, contentType: 'test' });

const browserEnvelope = fs.readFileSync('./_fixtures/envelope_astro_ssr_browser.txt', 'binary');
const browserEnvelope = fs.readFileSync('./_fixtures/envelope_astro_ssr_browser.txt');
const processedBrowserEnvelope = processEnvelope({ data: browserEnvelope, contentType: 'test' });

expect(processedNodeEnvelope).not.toBe(undefined);
@@ -67,47 +67,47 @@ describe('Sentry Integration', () => {
});

test('Process Angular Envelope', () => {
const envelope = fs.readFileSync('./_fixtures/envelope_angular.txt', 'binary');
const envelope = fs.readFileSync('./_fixtures/envelope_angular.txt');
expect(processEnvelope({ data: envelope, contentType: 'test' })).not.toBe(undefined);
});

test('Process Java Formatted Message Envelope', () => {
const envelope = fs.readFileSync('./_fixtures/envelope_java_formatted_message.txt', 'binary');
const envelope = fs.readFileSync('./_fixtures/envelope_java_formatted_message.txt');
expect(processEnvelope({ data: envelope, contentType: 'test' })).not.toBe(undefined);
});

test('Process Envelope w/ Binary Data', () => {
const envelope = fs.readFileSync('./_fixtures/envelope_binary.bin', 'binary');
const envelope = fs.readFileSync('./_fixtures/envelope_binary.bin');
expect(processEnvelope({ data: envelope, contentType: 'test' })).not.toBe(undefined);
});

test('Process Envelope w/ Empty Payloads', () => {
const envelope = fs.readFileSync('./_fixtures/envelope_empty_payload.txt', 'binary');
const envelope = fs.readFileSync('./_fixtures/envelope_empty_payload.txt');
expect(processEnvelope({ data: envelope, contentType: 'test' })).not.toBe(undefined);
});

test('Process Envelope w/ implicit length, terminated by newline', () => {
const envelope = fs.readFileSync('./_fixtures/envelope_no_len_w_new_line.txt', 'binary');
const envelope = fs.readFileSync('./_fixtures/envelope_no_len_w_new_line.txt');
expect(processEnvelope({ data: envelope, contentType: 'test' })).not.toBe(undefined);
});

test('Process Envelope w/ implicit length, terminated by EOF', () => {
const envelope = fs.readFileSync('./_fixtures/envelope_no_len_w_eof.txt', 'binary');
const envelope = fs.readFileSync('./_fixtures/envelope_no_len_w_eof.txt');
expect(processEnvelope({ data: envelope, contentType: 'test' })).not.toBe(undefined);
});

test('Process Envelope w/ implicit length, terminated by EOF, empty headers', () => {
const envelope = fs.readFileSync('./_fixtures/envelope_no_len_w_eof_empty_headers.txt', 'binary');
const envelope = fs.readFileSync('./_fixtures/envelope_no_len_w_eof_empty_headers.txt');
expect(processEnvelope({ data: envelope, contentType: 'test' })).not.toBe(undefined);
});

test('Process Envelope w/ flutter replay video', () => {
const envelope = fs.readFileSync('./_fixtures/envelope_flutter_replay.bin', 'binary');
const envelope = fs.readFileSync('./_fixtures/envelope_flutter_replay.bin');
expect(processEnvelope({ data: envelope, contentType: 'test' })).not.toBe(undefined);
});

test('Process Envelope w/ PNG screenshot', () => {
const envelope = fs.readFileSync('./_fixtures/envelope_with_screenshot.bin', 'binary');
const envelope = fs.readFileSync('./_fixtures/envelope_with_screenshot.bin');
expect(processEnvelope({ data: envelope, contentType: 'test' })).not.toBe(undefined);
});
});
9 changes: 3 additions & 6 deletions packages/overlay/src/integrations/sentry/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { Client, Envelope, EnvelopeItem } from '@sentry/types';
import { removeURLSuffix } from '~/utils/removeURLSuffix';
import { removeURLSuffix } from '~/lib/removeURLSuffix';
import { off, on } from '../../lib/eventTarget';
import { log, warn } from '../../lib/logger';
import type { Integration, RawEventContext } from '../integration';
@@ -60,7 +60,7 @@ export default function sentryIntegration(options: SentryIntegrationOptions = {}
.getEvents()
.filter(
e =>
e.type != 'transaction' &&
e.type !== 'transaction' &&
(e.contexts?.trace?.trace_id ? sentryDataCache.isTraceLocal(e.contexts?.trace?.trace_id) : null) !== false,
).length;

@@ -118,10 +118,7 @@ function parseJSONFromBuffer(data: Uint8Array): object {
* @returns parsed envelope
*/
export function processEnvelope(rawEvent: RawEventContext) {
let buffer =
typeof rawEvent.data === 'string'
? Uint8Array.from(Array.from(rawEvent.data, c => c.charCodeAt(0)))
: rawEvent.data;
let buffer = typeof rawEvent.data === 'string' ? Uint8Array.from(rawEvent.data, c => c.charCodeAt(0)) : rawEvent.data;

function readLine(length?: number) {
const cursor = length ?? getLineEnd(buffer);
12 changes: 6 additions & 6 deletions packages/overlay/src/integrations/sentry/utils/traces.spec.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { describe, expect, test } from 'vitest';
import { generate_uuidv4 } from '../../../lib/uuid';
import { generateUuidv4 } from '../../../lib/uuid';
import type { Span } from '../types';
import { groupSpans } from './traces';

function mockSpan({ duration, ...span }: Partial<Span> & { duration?: number } = {}): Span {
const defaultTimestamp = new Date().getTime();
return {
trace_id: generate_uuidv4(),
span_id: generate_uuidv4(),
trace_id: generateUuidv4(),
span_id: generateUuidv4(),
op: 'unknown',
status: 'unknown',
start_timestamp: defaultTimestamp,
@@ -77,7 +77,7 @@ describe('groupSpans', () => {
});

test('missing root transactions as siblings, creates faux parent', () => {
const parent_span_id = generate_uuidv4();
const parent_span_id = generateUuidv4();
const span1 = mockSpan({
parent_span_id,
});
@@ -100,10 +100,10 @@ describe('groupSpans', () => {

test('missing root transactions as independent children, creates faux parents', () => {
const span1 = mockSpan({
parent_span_id: generate_uuidv4(),
parent_span_id: generateUuidv4(),
});
const span2 = mockSpan({
parent_span_id: generate_uuidv4(),
parent_span_id: generateUuidv4(),
trace_id: span1.trace_id,
});
const result = groupSpans([span1, span2]);
5 changes: 5 additions & 0 deletions packages/overlay/src/lib/base64.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export function base64Decode(data: string): Uint8Array {
// TODO: Use Uint8Array.fromBase64 when it becomes available
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Uint8Array/fromBase64
return Uint8Array.from(atob(data), c => c.charCodeAt(0));
}
File renamed without changes.
File renamed without changes.
2 changes: 1 addition & 1 deletion packages/overlay/src/lib/uuid.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export function generate_uuidv4() {
export function generateUuidv4() {
let dt = new Date().getTime();
return 'xxxxxxxxxxxx4xxxyxxxxxxxxxxxxxxx'.replace(/[xy]/g, function (c) {
let rnd = Math.random() * 16; //random number in range 0 to 16
File renamed without changes.
9 changes: 5 additions & 4 deletions packages/overlay/src/sidecar.ts
Original file line number Diff line number Diff line change
@@ -4,15 +4,16 @@ import { log } from './lib/logger';
export function connectToSidecar(
sidecarUrl: string,
// Content Type to listener
contentTypeListeners: Record<string, (event: MessageEvent) => void>,
contentTypeListeners: Record<string, (event: { data: string | Uint8Array }) => void>,
setOnline: React.Dispatch<React.SetStateAction<boolean>>,
): () => void {
log('Connecting to sidecar at', sidecarUrl);
const sidecarStreamUrl: string = new URL('/stream', sidecarUrl).href;
const source = new EventSource(sidecarStreamUrl);
const sidecarStreamUrl = new URL('/stream', sidecarUrl);
sidecarStreamUrl.searchParams.append('base64', '1');
const source = new EventSource(sidecarStreamUrl.href);

for (const [contentType, listener] of Object.entries(contentTypeListeners)) {
source.addEventListener(contentType, listener);
source.addEventListener(`${contentType}`, listener);
}

source.addEventListener('open', () => {
34 changes: 21 additions & 13 deletions packages/sidecar/src/main.ts
Original file line number Diff line number Diff line change
@@ -9,7 +9,7 @@ import { contextLinesHandler } from './contextlines.js';
import { activateLogger, enableDebugLogging, logger, type SidecarLogger } from './logger.js';
import { MessageBuffer } from './messageBuffer.js';

type Payload = [string, string];
type Payload = [string, Buffer];

type IncomingPayloadCallback = (body: string) => void;

@@ -91,6 +91,7 @@ const enableCORS = (handler: RequestHandler): RequestHandler =>
},
{ name: 'enableCORS', op: 'sidecar.http.middleware.cors' },
);

const streamRequestHandler = (buffer: MessageBuffer<Payload>, incomingPayload?: IncomingPayloadCallback) => {
return function handleStreamRequest(
req: IncomingMessage,
@@ -111,16 +112,27 @@ const streamRequestHandler = (buffer: MessageBuffer<Payload>, incomingPayload?:
});
res.flushHeaders();
// Send something in the body to trigger the `open` event
// This is mostly for Firefox -- see #376
// This is mostly for Firefox -- see getsentry/spotlight#376
res.write('\n');

const useBase64 = searchParams?.get('base64') != null;
const base64Indicator = useBase64 ? ';base64' : '';
const dataWriter = useBase64
? (data: Buffer) => res.write(`data:${data.toString('base64')}\n`)
: (data: Buffer) => {
// The utf-8 encoding here is wrong and is a hack as we are
// sending binary data as utf-8 over SSE which enforces utf-8
// encoding. This is only for backwards compatibility
for (const line of data.toString('utf-8').split('\n')) {
// This is very important - SSE events are delimited by two newlines
res.write(`data:${line}\n`);
}
};
const sub = buffer.subscribe(([payloadType, data]) => {
logger.debug('🕊️ sending to Spotlight');
res.write(`event:${payloadType}\n`);
// This is very important - SSE events are delimited by two newlines
for (const line of data.split('\n')) {
res.write(`data:${line}\n`);
}
res.write(`event:${payloadType}${base64Indicator}\n`);
dataWriter(data);
// This last \n is important as every message ends with an empty line in SSE
res.write('\n');
});

@@ -129,7 +141,7 @@ const streamRequestHandler = (buffer: MessageBuffer<Payload>, incomingPayload?:
res.end();
});
} else if (req.method === 'POST') {
logger.debug(`📩 Received event`);
logger.debug('📩 Received event');
let stream = req;
// Check for gzip or deflate encoding and create appropriate stream
const encoding = req.headers['content-encoding'];
@@ -161,11 +173,7 @@ const streamRequestHandler = (buffer: MessageBuffer<Payload>, incomingPayload?:
if (!contentType) {
logger.warn('No content type, skipping payload...');
} else {
// The utf-8 encoding here is wrong and is a hack as we are
// sending binary data as utf-8 over SSE which enforces utf-8
// encoding. Ideally we'd use base64 encoding for binary data
// but that means a breaking change so leaving this as is for now
buffer.put([contentType, body.toString('utf-8')]);
buffer.put([contentType, body]);
}

if (process.env.SPOTLIGHT_CAPTURE || incomingPayload) {

0 comments on commit 277bd76

Please sign in to comment.