Skip to content

Commit 6bf5d3a

Browse files
authored
fix(utils): Fail silently if the provided Dsn is invalid (#8121)
We don't want to cause app crashes if the SDK gets an invalid DSN. This patch fixes that by allowing `makeDsn` and `dsnFromString` to return `undefined`, which we do if the Dsn is invalid.
1 parent 23a1697 commit 6bf5d3a

File tree

13 files changed

+191
-115
lines changed

13 files changed

+191
-115
lines changed

packages/core/src/api.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,10 @@ export function getReportDialogEndpoint(
5858
},
5959
): string {
6060
const dsn = makeDsn(dsnLike);
61+
if (!dsn) {
62+
return '';
63+
}
64+
6165
const endpoint = `${getBaseApiEndpoint(dsn)}embed/error-page/`;
6266

6367
let encodedOptions = `dsn=${dsnToString(dsn)}`;

packages/core/src/baseclient.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,16 +112,20 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
112112
*/
113113
protected constructor(options: O) {
114114
this._options = options;
115+
115116
if (options.dsn) {
116117
this._dsn = makeDsn(options.dsn);
118+
} else {
119+
__DEBUG_BUILD__ && logger.warn('No DSN provided, client will not do anything.');
120+
}
121+
122+
if (this._dsn) {
117123
const url = getEnvelopeEndpointWithUrlEncodedAuth(this._dsn, options);
118124
this._transport = options.transport({
119125
recordDroppedEvent: this.recordDroppedEvent.bind(this),
120126
...options.transportOptions,
121127
url,
122128
});
123-
} else {
124-
__DEBUG_BUILD__ && logger.warn('No DSN provided, client will not do anything.');
125129
}
126130
}
127131

packages/core/src/transports/multiplexed.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,13 @@ export function makeMultiplexedTransport<TO extends BaseTransportOptions>(
5151
const fallbackTransport = createTransport(options);
5252
const otherTransports: Record<string, Transport> = {};
5353

54-
function getTransport(dsn: string): Transport {
54+
function getTransport(dsn: string): Transport | undefined {
5555
if (!otherTransports[dsn]) {
56-
const url = getEnvelopeEndpointWithUrlEncodedAuth(dsnFromString(dsn));
56+
const validatedDsn = dsnFromString(dsn);
57+
if (!validatedDsn) {
58+
return undefined;
59+
}
60+
const url = getEnvelopeEndpointWithUrlEncodedAuth(validatedDsn);
5761
otherTransports[dsn] = createTransport({ ...options, url });
5862
}
5963

@@ -66,7 +70,9 @@ export function makeMultiplexedTransport<TO extends BaseTransportOptions>(
6670
return eventFromEnvelope(envelope, eventTypes);
6771
}
6872

69-
const transports = matcher({ envelope, getEvent }).map(dsn => getTransport(dsn));
73+
const transports = matcher({ envelope, getEvent })
74+
.map(dsn => getTransport(dsn))
75+
.filter((t): t is Transport => !!t);
7076

7177
// If we have no transports to send to, use the fallback transport
7278
if (transports.length === 0) {

packages/core/test/lib/api.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ const dsnPublic = 'https://abc@sentry.io:1234/subpath/123';
99
const tunnel = 'https://hello.com/world';
1010
const _metadata = { sdk: { name: 'sentry.javascript.browser', version: '12.31.12' } } as ClientOptions['_metadata'];
1111

12-
const dsnPublicComponents = makeDsn(dsnPublic);
12+
const dsnPublicComponents = makeDsn(dsnPublic)!;
1313

1414
describe('API', () => {
1515
describe('getEnvelopeEndpointWithUrlEncodedAuth', () => {

packages/core/test/lib/base.test.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,19 +70,19 @@ describe('BaseClient', () => {
7070
});
7171

7272
test('allows missing Dsn', () => {
73-
expect.assertions(1);
74-
7573
const options = getDefaultTestClientOptions();
7674
const client = new TestClient(options);
7775

7876
expect(client.getDsn()).toBeUndefined();
77+
expect(client.getTransport()).toBeUndefined();
7978
});
8079

81-
test('throws with invalid Dsn', () => {
82-
expect.assertions(1);
83-
80+
test('handles being passed an invalid Dsn', () => {
8481
const options = getDefaultTestClientOptions({ dsn: 'abc' });
85-
expect(() => new TestClient(options)).toThrow(SentryError);
82+
const client = new TestClient(options);
83+
84+
expect(client.getDsn()).toBeUndefined();
85+
expect(client.getTransport()).toBeUndefined();
8686
});
8787
});
8888

packages/core/test/lib/transports/multiplexed.test.ts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,10 @@ import { TextEncoder } from 'util';
1212
import { createTransport, getEnvelopeEndpointWithUrlEncodedAuth, makeMultiplexedTransport } from '../../../src';
1313

1414
const DSN1 = 'https://1234@5678.ingest.sentry.io/4321';
15-
const DSN1_URL = getEnvelopeEndpointWithUrlEncodedAuth(dsnFromString(DSN1));
15+
const DSN1_URL = getEnvelopeEndpointWithUrlEncodedAuth(dsnFromString(DSN1)!);
1616

1717
const DSN2 = 'https://5678@1234.ingest.sentry.io/8765';
18-
const DSN2_URL = getEnvelopeEndpointWithUrlEncodedAuth(dsnFromString(DSN2));
18+
const DSN2_URL = getEnvelopeEndpointWithUrlEncodedAuth(dsnFromString(DSN2)!);
1919

2020
const ERROR_EVENT = { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' };
2121
const ERROR_ENVELOPE = createEnvelope<EventEnvelope>({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, [
@@ -83,6 +83,20 @@ describe('makeMultiplexedTransport', () => {
8383
await transport.send(ERROR_ENVELOPE);
8484
});
8585

86+
it('Falls back to options DSN when a matched DSN is invalid', async () => {
87+
expect.assertions(1);
88+
89+
const makeTransport = makeMultiplexedTransport(
90+
createTestTransport(url => {
91+
expect(url).toBe(DSN1_URL);
92+
}),
93+
() => ['invalidDsn'],
94+
);
95+
96+
const transport = makeTransport({ url: DSN1_URL, ...transportOptions });
97+
await transport.send(ERROR_ENVELOPE);
98+
});
99+
86100
it('DSN can be overridden via match callback', async () => {
87101
expect.assertions(1);
88102

packages/core/test/lib/transports/offline.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ const REPLAY_EVENT: ReplayEvent = {
3838
replay_type: 'buffer',
3939
};
4040

41-
const DSN = dsnFromString('https://public@dsn.ingest.sentry.io/1337');
41+
const DSN = dsnFromString('https://public@dsn.ingest.sentry.io/1337')!;
4242

4343
const DATA = 'nothing';
4444

packages/nextjs/src/client/tunnelRoute.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ export function applyTunnelRouteOption(options: BrowserOptions): void {
1212
const tunnelRouteOption = globalWithInjectedValues.__sentryRewritesTunnelPath__;
1313
if (tunnelRouteOption && options.dsn) {
1414
const dsnComponents = dsnFromString(options.dsn);
15+
if (!dsnComponents) {
16+
return;
17+
}
1518
const sentrySaasDsnMatch = dsnComponents.host.match(/^o(\d+)\.ingest\.sentry\.io$/);
1619
if (sentrySaasDsnMatch) {
1720
const orgId = sentrySaasDsnMatch[1];

packages/nextjs/test/utils/tunnelRoute.test.ts

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ beforeEach(() => {
1111
});
1212

1313
describe('applyTunnelRouteOption()', () => {
14-
it('should correctly apply `tunnelRoute` option when conditions are met', () => {
14+
it('Correctly applies `tunnelRoute` option when conditions are met', () => {
1515
globalWithInjectedValues.__sentryRewritesTunnelPath__ = '/my-error-monitoring-route';
1616
const options: any = {
1717
dsn: 'https://11111111111111111111111111111111@o2222222.ingest.sentry.io/3333333',
@@ -22,7 +22,7 @@ describe('applyTunnelRouteOption()', () => {
2222
expect(options.tunnel).toBe('/my-error-monitoring-route?o=2222222&p=3333333');
2323
});
2424

25-
it('should not apply `tunnelRoute` when DSN is missing', () => {
25+
it("Doesn't apply `tunnelRoute` when DSN is missing", () => {
2626
globalWithInjectedValues.__sentryRewritesTunnelPath__ = '/my-error-monitoring-route';
2727
const options: any = {
2828
// no dsn
@@ -33,7 +33,18 @@ describe('applyTunnelRouteOption()', () => {
3333
expect(options.tunnel).toBeUndefined();
3434
});
3535

36-
it("should not apply `tunnelRoute` option when `tunnelRoute` option wasn't injected", () => {
36+
it("Doesn't apply `tunnelRoute` when DSN is invalid", () => {
37+
globalWithInjectedValues.__sentryRewritesTunnelPath__ = '/my-error-monitoring-route';
38+
const options: any = {
39+
dsn: 'invalidDsn',
40+
} as BrowserOptions;
41+
42+
applyTunnelRouteOption(options);
43+
44+
expect(options.tunnel).toBeUndefined();
45+
});
46+
47+
it("Doesn't apply `tunnelRoute` option when `tunnelRoute` option wasn't injected", () => {
3748
const options: any = {
3849
dsn: 'https://11111111111111111111111111111111@o2222222.ingest.sentry.io/3333333',
3950
} as BrowserOptions;
@@ -43,7 +54,7 @@ describe('applyTunnelRouteOption()', () => {
4354
expect(options.tunnel).toBeUndefined();
4455
});
4556

46-
it('should not apply `tunnelRoute` option when DSN is not a SaaS DSN', () => {
57+
it("Doesn't `tunnelRoute` option when DSN is not a SaaS DSN", () => {
4758
globalWithInjectedValues.__sentryRewritesTunnelPath__ = '/my-error-monitoring-route';
4859
const options: any = {
4960
dsn: 'https://11111111111111111111111111111111@example.com/3333333',

packages/node/test/transports/http.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,9 @@ const defaultOptions = {
8383
textEncoder,
8484
};
8585

86+
// empty function to keep test output clean
87+
const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
88+
8689
describe('makeNewHttpTransport()', () => {
8790
afterEach(() => {
8891
jest.clearAllMocks();
@@ -403,7 +406,6 @@ describe('makeNewHttpTransport()', () => {
403406
});
404407

405408
it('should warn if an invalid url is passed', async () => {
406-
const consoleWarnSpy = jest.spyOn(console, 'warn');
407409
const transport = makeNodeTransport({ ...defaultOptions, url: 'invalid url' });
408410
await transport.send(EVENT_ENVELOPE);
409411
expect(consoleWarnSpy).toHaveBeenCalledWith(

packages/replay/test/unit/util/createReplayEnvelope.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ describe('Unit | util | createReplayEnvelope', () => {
4242
projectId: '123',
4343
protocol: 'https',
4444
publicKey: 'abc',
45-
});
45+
})!;
4646

4747
it('creates an envelope for a given Replay event', () => {
4848
const envelope = createReplayEnvelope(replayEvent, payloadWithSequence, dsn);

packages/utils/src/dsn.ts

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { DsnComponents, DsnLike, DsnProtocol } from '@sentry/types';
22

3-
import { SentryError } from './error';
3+
import { logger } from './logger';
44

55
/** Regular expression used to parse a Dsn. */
66
const DSN_REGEX = /^(?:(\w+):)\/\/(?:(\w+)(?::(\w+)?)?@)([\w.-]+)(?::(\d+))?\/(.+)/;
@@ -30,13 +30,16 @@ export function dsnToString(dsn: DsnComponents, withPassword: boolean = false):
3030
* Parses a Dsn from a given string.
3131
*
3232
* @param str A Dsn as string
33-
* @returns Dsn as DsnComponents
33+
* @returns Dsn as DsnComponents or undefined if @param str is not a valid DSN string
3434
*/
35-
export function dsnFromString(str: string): DsnComponents {
35+
export function dsnFromString(str: string): DsnComponents | undefined {
3636
const match = DSN_REGEX.exec(str);
3737

3838
if (!match) {
39-
throw new SentryError(`Invalid Sentry Dsn: ${str}`);
39+
// This should be logged to the console
40+
// eslint-disable-next-line no-console
41+
console.error(`Invalid Sentry Dsn: ${str}`);
42+
return undefined;
4043
}
4144

4245
const [protocol, publicKey, pass = '', host, port = '', lastPath] = match.slice(1);
@@ -71,38 +74,52 @@ function dsnFromComponents(components: DsnComponents): DsnComponents {
7174
};
7275
}
7376

74-
function validateDsn(dsn: DsnComponents): boolean | void {
77+
function validateDsn(dsn: DsnComponents): boolean {
7578
if (!__DEBUG_BUILD__) {
76-
return;
79+
return true;
7780
}
7881

7982
const { port, projectId, protocol } = dsn;
8083

8184
const requiredComponents: ReadonlyArray<keyof DsnComponents> = ['protocol', 'publicKey', 'host', 'projectId'];
82-
requiredComponents.forEach(component => {
85+
const hasMissingRequiredComponent = requiredComponents.find(component => {
8386
if (!dsn[component]) {
84-
throw new SentryError(`Invalid Sentry Dsn: ${component} missing`);
87+
logger.error(`Invalid Sentry Dsn: ${component} missing`);
88+
return true;
8589
}
90+
return false;
8691
});
8792

93+
if (hasMissingRequiredComponent) {
94+
return false;
95+
}
96+
8897
if (!projectId.match(/^\d+$/)) {
89-
throw new SentryError(`Invalid Sentry Dsn: Invalid projectId ${projectId}`);
98+
logger.error(`Invalid Sentry Dsn: Invalid projectId ${projectId}`);
99+
return false;
90100
}
91101

92102
if (!isValidProtocol(protocol)) {
93-
throw new SentryError(`Invalid Sentry Dsn: Invalid protocol ${protocol}`);
103+
logger.error(`Invalid Sentry Dsn: Invalid protocol ${protocol}`);
104+
return false;
94105
}
95106

96107
if (port && isNaN(parseInt(port, 10))) {
97-
throw new SentryError(`Invalid Sentry Dsn: Invalid port ${port}`);
108+
logger.error(`Invalid Sentry Dsn: Invalid port ${port}`);
109+
return false;
98110
}
99111

100112
return true;
101113
}
102114

103-
/** The Sentry Dsn, identifying a Sentry instance and project. */
104-
export function makeDsn(from: DsnLike): DsnComponents {
115+
/**
116+
* Creates a valid Sentry Dsn object, identifying a Sentry instance and project.
117+
* @returns a valid DsnComponents object or `undefined` if @param from is an invalid DSN source
118+
*/
119+
export function makeDsn(from: DsnLike): DsnComponents | undefined {
105120
const components = typeof from === 'string' ? dsnFromString(from) : dsnFromComponents(from);
106-
validateDsn(components);
121+
if (!components || !validateDsn(components)) {
122+
return undefined;
123+
}
107124
return components;
108125
}

0 commit comments

Comments
 (0)