improve check for DSN by extracging URL parts

Author: Lms24

packages/core/src/utils/isSentryRequestUrl.ts

@@ -1,5 +1,6 @@
 import type { Client } from '../client';
 import type { DsnComponents } from '../types-hoist/dsn';
+import { isURLObjectRelative, parseStringToURLObject } from './url';
 
 /**
  * Checks whether given url points to Sentry server
@@ -24,9 +25,14 @@ function checkDsn(url: string, dsn: DsnComponents | undefined): boolean {
   // Requests to Sentry's ingest endpoint must have a `sentry_key` in the query string
   // This is equivalent to the public_key which is required in the DSN
   // see https://develop.sentry.dev/sdk/overview/#parsing-the-dsn
-  // Therefore a request to the same host and with a `sentry_key` in the query string
-  // can be considered a request to the ingest endpoint
-  return dsn ? url.includes(dsn.host) && !!url.match(/sentry_key/) : false;
+  // Therefore, a request to the same host and with a `sentry_key` in the query string
+  // can be considered a request to the ingest endpoint.
+  const urlParts = parseStringToURLObject(url);
+  if (!urlParts || isURLObjectRelative(urlParts)) {
+    return false;
+  }
+
+  return dsn ? urlParts.host.includes(dsn.host) && /(^|&|\?)sentry_key=/.test(urlParts.search) : false;
 }
 
 function removeTrailingSlash(str: string): string {

packages/core/test/lib/utils/isSentryRequestUrl.test.ts

@@ -4,25 +4,46 @@ import type { Client } from '../../../src/client';
 
 describe('isSentryRequestUrl', () => {
   it.each([
-    ['http://sentry-dsn.com/my-url?sentry_key=123', 'sentry-dsn.com', '', true],
-    ['http://tunnel:4200', 'sentry-dsn.com', 'http://tunnel:4200', true],
-    ['http://tunnel:4200', 'sentry-dsn.com', 'http://tunnel:4200/', true],
-    ['http://tunnel:4200/', 'sentry-dsn.com', 'http://tunnel:4200', true],
-    ['http://tunnel:4200/', 'another-dsn.com', 'http://tunnel:4200', true],
+    ['http://sentry-dsn.com/my-url?sentry_key=123', 'sentry-dsn.com', ''],
 
-    ['http://tunnel:4200/?sentry_key=123', 'another-dsn.com', '', false],
-    ['http://sentry-dsn.com/my-url', 'sentry-dsn.com', '', false],
-    ['http://sentry-dsn.com', 'sentry-dsn.com', '', false],
-    ['http://tunnel:4200/', 'another-dsn.com', 'http://tunnel:4200/sentry-tunnel', false],
-    ['', 'sentry-dsn.com', '', false],
-    ['http://tunnel:4200/a', 'sentry-dsn.com', 'http://tunnel:4200', false],
-  ])('works with url=%s, dsn=%s, tunnel=%s', (url: string, dsn: string, tunnel: string, expected: boolean) => {
+    ['http://tunnel:4200', 'sentry-dsn.com', 'http://tunnel:4200'],
+    ['http://tunnel:4200', 'sentry-dsn.com', 'http://tunnel:4200/'],
+    ['http://tunnel:4200/', 'sentry-dsn.com', 'http://tunnel:4200'],
+    ['http://tunnel:4200/', 'another-dsn.com', 'http://tunnel:4200'],
+  ])('returns `true` for url=%s, dsn=%s, tunnel=%s', (url: string, dsn: string, tunnel: string) => {
     const client = {
       getOptions: () => ({ tunnel }),
       getDsn: () => ({ host: dsn }),
     } as unknown as Client;
 
-    // Works with client passed
-    expect(isSentryRequestUrl(url, client)).toBe(expected);
+    expect(isSentryRequestUrl(url, client)).toBe(true);
+  });
+
+  it.each([
+    ['http://tunnel:4200/?sentry_key=123', 'another-dsn.com', ''],
+    ['http://sentry-dsn.com/my-url', 'sentry-dsn.com', ''],
+    ['http://sentry-dsn.com', 'sentry-dsn.com', ''],
+    ['http://sAntry-dsn.com/?sentry_key=123', 'sentry-dsn.com', ''],
+    ['http://sAntry-dsn.com/?sAntry_key=123', 'sAntry-dsn.com', ''],
+    ['/ingest', 'sentry-dsn.com', ''],
+    ['/ingest?sentry_key=123', 'sentry-dsn.com', ''],
+    ['/ingest', '', ''],
+    ['', '', ''],
+    ['', 'sentry-dsn.com', ''],
+
+    ['http://tunnel:4200/', 'another-dsn.com', 'http://tunnel:4200/sentry-tunnel'],
+    ['http://tunnel:4200/a', 'sentry-dsn.com', 'http://tunnel:4200'],
+    ['http://tunnel:4200/a', '', 'http://tunnel:4200/'],
+  ])('returns `false` for url=%s, dsn=%s, tunnel=%s', (url: string, dsn: string, tunnel: string) => {
+    const client = {
+      getOptions: () => ({ tunnel }),
+      getDsn: () => ({ host: dsn }),
+    } as unknown as Client;
+
+    expect(isSentryRequestUrl(url, client)).toBe(false);
+  });
+
+  it('handles undefined client', () => {
+    expect(isSentryRequestUrl('http://sentry-dsn.com/my-url?sentry_key=123', undefined)).toBe(false);
   });
 });

packages/replay-internal/test/integration/shouldFilterRequest.test.ts

@@ -20,6 +20,6 @@ describe('Integration | shouldFilterRequest', () => {
   it('should filter requests for Sentry ingest URLs', async () => {
     const { replay } = await mockSdk();
 
-    expect(shouldFilterRequest(replay, 'https://03031aa.ingest.f00.f00/api/129312/')).toBe(true);
+    expect(shouldFilterRequest(replay, 'https://03031aa.ingest.f00.f00/api/129312/?sentry_key=123')).toBe(true);
   });
 });

packages/vercel-edge/test/wintercg-fetch.test.ts

@@ -102,8 +102,8 @@ describe('WinterCGFetch instrumentation', () => {
     expect(fetchInstrumentationHandlerCallback).toBeDefined();
 
     const startHandlerData: HandlerDataFetch = {
-      fetchData: { url: 'https://dsn.ingest.sentry.io/1337', method: 'POST' },
-      args: ['https://dsn.ingest.sentry.io/1337'],
+      fetchData: { url: 'https://dsn.ingest.sentry.io/1337?sentry_key=123', method: 'POST' },
+      args: ['https://dsn.ingest.sentry.io/1337?sentry_key=123'],
       startTimestamp: Date.now(),
     };
     fetchInstrumentationHandlerCallback(startHandlerData);