Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Fix encoding-related issues with non-utf8 chars #659

Merged
merged 1 commit into from
Jan 20, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Expand Up @@ -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;
Expand Down
28 changes: 22 additions & 6 deletions packages/overlay/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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'>>;
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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]);
Expand All @@ -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],
);
Expand Down
4 changes: 2 additions & 2 deletions packages/overlay/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 }),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
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';
Expand Down Expand Up @@ -115,7 +115,7 @@
},
) {
if (!event.event_id) {
event.event_id = generate_uuidv4();
event.event_id = generateUuidv4();
}

if (this.eventIds.has(event.event_id)) return;
Expand Down Expand Up @@ -245,7 +245,7 @@
}

subscribe(...args: Subscription) {
const id = generate_uuidv4();
const id = generateUuidv4();

Check warning on line 248 in packages/overlay/src/integrations/sentry/data/sentryDataCache.ts

View check run for this annotation

Codecov / codecov/patch

packages/overlay/src/integrations/sentry/data/sentryDataCache.ts#L248

Added line #L248 was not covered by tests
this.subscribers.set(id, args);

return () => {
Expand Down
32 changes: 16 additions & 16 deletions packages/overlay/src/integrations/sentry/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -60,7 +60,7 @@
.getEvents()
.filter(
e =>
e.type != 'transaction' &&
e.type !== 'transaction' &&

Check warning on line 63 in packages/overlay/src/integrations/sentry/index.ts

View check run for this annotation

Codecov / codecov/patch

packages/overlay/src/integrations/sentry/index.ts#L63

Added line #L63 was not covered by tests
(e.contexts?.trace?.trace_id ? sentryDataCache.isTraceLocal(e.contexts?.trace?.trace_id) : null) !== false,
).length;

Expand Down Expand Up @@ -118,10 +118,7 @@
* @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);
Expand Down
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,
Expand Down Expand Up @@ -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,
});
Expand All @@ -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]);
Expand Down
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));
}
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
Expand Down
9 changes: 5 additions & 4 deletions packages/overlay/src/sidecar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
Loading
Loading