fix(utils): Fail silently if the provided Dsn is invalid (#8121)

Lms24 · web-flow · commit 6bf5d3aed0a0 · 2023-05-17T13:36:08.000+02:00
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.
diff --git a/packages/core/src/api.ts b/packages/core/src/api.ts
@@ -58,6 +58,10 @@ export function getReportDialogEndpoint(
   },
 ): string {
   const dsn = makeDsn(dsnLike);
+  if (!dsn) {
+    return '';
+  }
+
   const endpoint = `${getBaseApiEndpoint(dsn)}embed/error-page/`;
 
   let encodedOptions = `dsn=${dsnToString(dsn)}`;
diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts
@@ -112,16 +112,20 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
    */
   protected constructor(options: O) {
     this._options = options;
+
     if (options.dsn) {
       this._dsn = makeDsn(options.dsn);
+    } else {
+      __DEBUG_BUILD__ && logger.warn('No DSN provided, client will not do anything.');
+    }
+
+    if (this._dsn) {
       const url = getEnvelopeEndpointWithUrlEncodedAuth(this._dsn, options);
       this._transport = options.transport({
         recordDroppedEvent: this.recordDroppedEvent.bind(this),
         ...options.transportOptions,
         url,
       });
-    } else {
-      __DEBUG_BUILD__ && logger.warn('No DSN provided, client will not do anything.');
     }
   }
 
diff --git a/packages/core/src/transports/multiplexed.ts b/packages/core/src/transports/multiplexed.ts
@@ -51,9 +51,13 @@ export function makeMultiplexedTransport<TO extends BaseTransportOptions>(
     const fallbackTransport = createTransport(options);
     const otherTransports: Record<string, Transport> = {};
 
-    function getTransport(dsn: string): Transport {
+    function getTransport(dsn: string): Transport | undefined {
       if (!otherTransports[dsn]) {
-        const url = getEnvelopeEndpointWithUrlEncodedAuth(dsnFromString(dsn));
+        const validatedDsn = dsnFromString(dsn);
+        if (!validatedDsn) {
+          return undefined;
+        }
+        const url = getEnvelopeEndpointWithUrlEncodedAuth(validatedDsn);
         otherTransports[dsn] = createTransport({ ...options, url });
       }
 
@@ -66,7 +70,9 @@ export function makeMultiplexedTransport<TO extends BaseTransportOptions>(
         return eventFromEnvelope(envelope, eventTypes);
       }
 
-      const transports = matcher({ envelope, getEvent }).map(dsn => getTransport(dsn));
+      const transports = matcher({ envelope, getEvent })
+        .map(dsn => getTransport(dsn))
+        .filter((t): t is Transport => !!t);
 
       // If we have no transports to send to, use the fallback transport
       if (transports.length === 0) {
diff --git a/packages/core/test/lib/api.test.ts b/packages/core/test/lib/api.test.ts
@@ -9,7 +9,7 @@ const dsnPublic = 'https://abc@sentry.io:1234/subpath/123';
 const tunnel = 'https://hello.com/world';
 const _metadata = { sdk: { name: 'sentry.javascript.browser', version: '12.31.12' } } as ClientOptions['_metadata'];
 
-const dsnPublicComponents = makeDsn(dsnPublic);
+const dsnPublicComponents = makeDsn(dsnPublic)!;
 
 describe('API', () => {
   describe('getEnvelopeEndpointWithUrlEncodedAuth', () => {
diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts
@@ -70,19 +70,19 @@ describe('BaseClient', () => {
     });
 
     test('allows missing Dsn', () => {
-      expect.assertions(1);
-
       const options = getDefaultTestClientOptions();
       const client = new TestClient(options);
 
       expect(client.getDsn()).toBeUndefined();
+      expect(client.getTransport()).toBeUndefined();
     });
 
-    test('throws with invalid Dsn', () => {
-      expect.assertions(1);
-
+    test('handles being passed an invalid Dsn', () => {
       const options = getDefaultTestClientOptions({ dsn: 'abc' });
-      expect(() => new TestClient(options)).toThrow(SentryError);
+      const client = new TestClient(options);
+
+      expect(client.getDsn()).toBeUndefined();
+      expect(client.getTransport()).toBeUndefined();
     });
   });
 
diff --git a/packages/core/test/lib/transports/multiplexed.test.ts b/packages/core/test/lib/transports/multiplexed.test.ts
@@ -12,10 +12,10 @@ import { TextEncoder } from 'util';
 import { createTransport, getEnvelopeEndpointWithUrlEncodedAuth, makeMultiplexedTransport } from '../../../src';
 
 const DSN1 = 'https://1234@5678.ingest.sentry.io/4321';
-const DSN1_URL = getEnvelopeEndpointWithUrlEncodedAuth(dsnFromString(DSN1));
+const DSN1_URL = getEnvelopeEndpointWithUrlEncodedAuth(dsnFromString(DSN1)!);
 
 const DSN2 = 'https://5678@1234.ingest.sentry.io/8765';
-const DSN2_URL = getEnvelopeEndpointWithUrlEncodedAuth(dsnFromString(DSN2));
+const DSN2_URL = getEnvelopeEndpointWithUrlEncodedAuth(dsnFromString(DSN2)!);
 
 const ERROR_EVENT = { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' };
 const ERROR_ENVELOPE = createEnvelope<EventEnvelope>({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, [
@@ -83,6 +83,20 @@ describe('makeMultiplexedTransport', () => {
     await transport.send(ERROR_ENVELOPE);
   });
 
+  it('Falls back to options DSN when a matched DSN is invalid', async () => {
+    expect.assertions(1);
+
+    const makeTransport = makeMultiplexedTransport(
+      createTestTransport(url => {
+        expect(url).toBe(DSN1_URL);
+      }),
+      () => ['invalidDsn'],
+    );
+
+    const transport = makeTransport({ url: DSN1_URL, ...transportOptions });
+    await transport.send(ERROR_ENVELOPE);
+  });
+
   it('DSN can be overridden via match callback', async () => {
     expect.assertions(1);
 
diff --git a/packages/core/test/lib/transports/offline.test.ts b/packages/core/test/lib/transports/offline.test.ts
@@ -38,7 +38,7 @@ const REPLAY_EVENT: ReplayEvent = {
   replay_type: 'buffer',
 };
 
-const DSN = dsnFromString('https://public@dsn.ingest.sentry.io/1337');
+const DSN = dsnFromString('https://public@dsn.ingest.sentry.io/1337')!;
 
 const DATA = 'nothing';
 
diff --git a/packages/nextjs/src/client/tunnelRoute.ts b/packages/nextjs/src/client/tunnelRoute.ts
@@ -12,6 +12,9 @@ export function applyTunnelRouteOption(options: BrowserOptions): void {
   const tunnelRouteOption = globalWithInjectedValues.__sentryRewritesTunnelPath__;
   if (tunnelRouteOption && options.dsn) {
     const dsnComponents = dsnFromString(options.dsn);
+    if (!dsnComponents) {
+      return;
+    }
     const sentrySaasDsnMatch = dsnComponents.host.match(/^o(\d+)\.ingest\.sentry\.io$/);
     if (sentrySaasDsnMatch) {
       const orgId = sentrySaasDsnMatch[1];
diff --git a/packages/nextjs/test/utils/tunnelRoute.test.ts b/packages/nextjs/test/utils/tunnelRoute.test.ts
@@ -11,7 +11,7 @@ beforeEach(() => {
 });
 
 describe('applyTunnelRouteOption()', () => {
-  it('should correctly apply `tunnelRoute` option when conditions are met', () => {
+  it('Correctly applies `tunnelRoute` option when conditions are met', () => {
     globalWithInjectedValues.__sentryRewritesTunnelPath__ = '/my-error-monitoring-route';
     const options: any = {
       dsn: 'https://11111111111111111111111111111111@o2222222.ingest.sentry.io/3333333',
@@ -22,7 +22,7 @@ describe('applyTunnelRouteOption()', () => {
     expect(options.tunnel).toBe('/my-error-monitoring-route?o=2222222&p=3333333');
   });
 
-  it('should not apply `tunnelRoute` when DSN is missing', () => {
+  it("Doesn't apply `tunnelRoute` when DSN is missing", () => {
     globalWithInjectedValues.__sentryRewritesTunnelPath__ = '/my-error-monitoring-route';
     const options: any = {
       // no dsn
@@ -33,7 +33,18 @@ describe('applyTunnelRouteOption()', () => {
     expect(options.tunnel).toBeUndefined();
   });
 
-  it("should not apply `tunnelRoute` option when `tunnelRoute` option wasn't injected", () => {
+  it("Doesn't apply `tunnelRoute` when DSN is invalid", () => {
+    globalWithInjectedValues.__sentryRewritesTunnelPath__ = '/my-error-monitoring-route';
+    const options: any = {
+      dsn: 'invalidDsn',
+    } as BrowserOptions;
+
+    applyTunnelRouteOption(options);
+
+    expect(options.tunnel).toBeUndefined();
+  });
+
+  it("Doesn't apply `tunnelRoute` option when `tunnelRoute` option wasn't injected", () => {
     const options: any = {
       dsn: 'https://11111111111111111111111111111111@o2222222.ingest.sentry.io/3333333',
     } as BrowserOptions;
@@ -43,7 +54,7 @@ describe('applyTunnelRouteOption()', () => {
     expect(options.tunnel).toBeUndefined();
   });
 
-  it('should not apply `tunnelRoute` option when DSN is not a SaaS DSN', () => {
+  it("Doesn't `tunnelRoute` option when DSN is not a SaaS DSN", () => {
     globalWithInjectedValues.__sentryRewritesTunnelPath__ = '/my-error-monitoring-route';
     const options: any = {
       dsn: 'https://11111111111111111111111111111111@example.com/3333333',
diff --git a/packages/node/test/transports/http.test.ts b/packages/node/test/transports/http.test.ts
@@ -83,6 +83,9 @@ const defaultOptions = {
   textEncoder,
 };
 
+// empty function to keep test output clean
+const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
+
 describe('makeNewHttpTransport()', () => {
   afterEach(() => {
     jest.clearAllMocks();
@@ -403,7 +406,6 @@ describe('makeNewHttpTransport()', () => {
   });
 
   it('should warn if an invalid url is passed', async () => {
-    const consoleWarnSpy = jest.spyOn(console, 'warn');
     const transport = makeNodeTransport({ ...defaultOptions, url: 'invalid url' });
     await transport.send(EVENT_ENVELOPE);
     expect(consoleWarnSpy).toHaveBeenCalledWith(
diff --git a/packages/replay/test/unit/util/createReplayEnvelope.test.ts b/packages/replay/test/unit/util/createReplayEnvelope.test.ts
@@ -42,7 +42,7 @@ describe('Unit | util | createReplayEnvelope', () => {
     projectId: '123',
     protocol: 'https',
     publicKey: 'abc',
-  });
+  })!;
 
   it('creates an envelope for a given Replay event', () => {
     const envelope = createReplayEnvelope(replayEvent, payloadWithSequence, dsn);
diff --git a/packages/utils/src/dsn.ts b/packages/utils/src/dsn.ts
@@ -1,6 +1,6 @@
 import type { DsnComponents, DsnLike, DsnProtocol } from '@sentry/types';
 
-import { SentryError } from './error';
+import { logger } from './logger';
 
 /** Regular expression used to parse a Dsn. */
 const DSN_REGEX = /^(?:(\w+):)\/\/(?:(\w+)(?::(\w+)?)?@)([\w.-]+)(?::(\d+))?\/(.+)/;
@@ -30,13 +30,16 @@ export function dsnToString(dsn: DsnComponents, withPassword: boolean = false):
  * Parses a Dsn from a given string.
  *
  * @param str A Dsn as string
- * @returns Dsn as DsnComponents
+ * @returns Dsn as DsnComponents or undefined if @param str is not a valid DSN string
  */
-export function dsnFromString(str: string): DsnComponents {
+export function dsnFromString(str: string): DsnComponents | undefined {
   const match = DSN_REGEX.exec(str);
 
   if (!match) {
-    throw new SentryError(`Invalid Sentry Dsn: ${str}`);
+    // This should be logged to the console
+    // eslint-disable-next-line no-console
+    console.error(`Invalid Sentry Dsn: ${str}`);
+    return undefined;
   }
 
   const [protocol, publicKey, pass = '', host, port = '', lastPath] = match.slice(1);
@@ -71,38 +74,52 @@ function dsnFromComponents(components: DsnComponents): DsnComponents {
   };
 }
 
-function validateDsn(dsn: DsnComponents): boolean | void {
+function validateDsn(dsn: DsnComponents): boolean {
   if (!__DEBUG_BUILD__) {
-    return;
+    return true;
   }
 
   const { port, projectId, protocol } = dsn;
 
   const requiredComponents: ReadonlyArray<keyof DsnComponents> = ['protocol', 'publicKey', 'host', 'projectId'];
-  requiredComponents.forEach(component => {
+  const hasMissingRequiredComponent = requiredComponents.find(component => {
     if (!dsn[component]) {
-      throw new SentryError(`Invalid Sentry Dsn: ${component} missing`);
+      logger.error(`Invalid Sentry Dsn: ${component} missing`);
+      return true;
     }
+    return false;
   });
 
+  if (hasMissingRequiredComponent) {
+    return false;
+  }
+
   if (!projectId.match(/^\d+$/)) {
-    throw new SentryError(`Invalid Sentry Dsn: Invalid projectId ${projectId}`);
+    logger.error(`Invalid Sentry Dsn: Invalid projectId ${projectId}`);
+    return false;
   }
 
   if (!isValidProtocol(protocol)) {
-    throw new SentryError(`Invalid Sentry Dsn: Invalid protocol ${protocol}`);
+    logger.error(`Invalid Sentry Dsn: Invalid protocol ${protocol}`);
+    return false;
   }
 
   if (port && isNaN(parseInt(port, 10))) {
-    throw new SentryError(`Invalid Sentry Dsn: Invalid port ${port}`);
+    logger.error(`Invalid Sentry Dsn: Invalid port ${port}`);
+    return false;
   }
 
   return true;
 }
 
-/** The Sentry Dsn, identifying a Sentry instance and project. */
-export function makeDsn(from: DsnLike): DsnComponents {
+/**
+ * Creates a valid Sentry Dsn object, identifying a Sentry instance and project.
+ * @returns a valid DsnComponents object or `undefined` if @param from is an invalid DSN source
+ */
+export function makeDsn(from: DsnLike): DsnComponents | undefined {
   const components = typeof from === 'string' ? dsnFromString(from) : dsnFromComponents(from);
-  validateDsn(components);
+  if (!components || !validateDsn(components)) {
+    return undefined;
+  }
   return components;
 }
diff --git a/packages/utils/test/dsn.test.ts b/packages/utils/test/dsn.test.ts

-Original file line number
+Diff line change
   replay_type: 'buffer',
 };
 -const DSN = dsnFromString('https://[email protected]/1337');
 +const DSN = dsnFromString('https://[email protected]/1337')!;
 const DATA = 'nothing';