fix(replay): Check relative URLs correctly (#8024)

As mentioned by @ryan953 [here](https://github.com/getsentry/getsentry/pull/10418/files#r1183010062), we should check relative URLs properly as well for capturing network details.
getsentry · May 11, 2023 · 9a267eb · 9a267eb
1 parent 07c8af9
commit 9a267eb
Show file tree

Hide file tree

Showing 6 changed files with 215 additions and 4 deletions.
diff --git a/...integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureRequestBody/init.js b/...integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureRequestBody/init.js
@@ -5,7 +5,7 @@ window.Replay = new Sentry.Replay({
   flushMinDelay: 200,
   flushMaxDelay: 200,
 
-  networkDetailAllowUrls: ['http://localhost:7654/foo'],
+  networkDetailAllowUrls: ['http://localhost:7654/foo', 'http://sentry-test.io/foo'],
   networkCaptureBodies: true,
 });
 

diff --git a/...integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureRequestBody/test.ts b/...integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureRequestBody/test.ts
@@ -249,6 +249,85 @@ sentryTest('captures non-text request body', async ({ getLocalTestPath, page, br
   ]);
 });
 
+sentryTest('captures text request body when matching relative URL', async ({ getLocalTestUrl, page, browserName }) => {
+  if (shouldSkipReplayTest()) {
+    sentryTest.skip();
+  }
+
+  const additionalHeaders = browserName === 'webkit' ? { 'content-type': 'text/plain' } : undefined;
+
+  await page.route('**/foo', route => {
+    return route.fulfill({
+      status: 200,
+    });
+  });
+
+  await page.route('https://dsn.ingest.sentry.io/**/*', route => {
+    return route.fulfill({
+      status: 200,
+      contentType: 'application/json',
+      body: JSON.stringify({ id: 'test-id' }),
+    });
+  });
+
+  const requestPromise = waitForErrorRequest(page);
+  const replayRequestPromise1 = waitForReplayRequest(page, 0);
+
+  const url = await getLocalTestUrl({ testDir: __dirname });
+  await page.goto(url);
+
+  await page.evaluate(() => {
+    /* eslint-disable */
+    fetch('/foo', {
+      method: 'POST',
+      body: 'input body',
+    }).then(() => {
+      // @ts-ignore Sentry is a global
+      Sentry.captureException('test error');
+    });
+    /* eslint-enable */
+  });
+
+  const request = await requestPromise;
+  const eventData = envelopeRequestParser(request);
+
+  expect(eventData.exception?.values).toHaveLength(1);
+
+  expect(eventData?.breadcrumbs?.length).toBe(1);
+  expect(eventData!.breadcrumbs![0]).toEqual({
+    timestamp: expect.any(Number),
+    category: 'fetch',
+    type: 'http',
+    data: {
+      method: 'POST',
+      request_body_size: 10,
+      status_code: 200,
+      url: '/foo',
+    },
+  });
+
+  const replayReq1 = await replayRequestPromise1;
+  const { performanceSpans: performanceSpans1 } = getCustomRecordingEvents(replayReq1);
+  expect(performanceSpans1.filter(span => span.op === 'resource.fetch')).toEqual([
+    {
+      data: {
+        method: 'POST',
+        statusCode: 200,
+        request: {
+          size: 10,
+          headers: {},
+          body: 'input body',
+        },
+        response: additionalHeaders ? { headers: additionalHeaders } : undefined,
+      },
+      description: '/foo',
+      endTimestamp: expect.any(Number),
+      op: 'resource.fetch',
+      startTimestamp: expect.any(Number),
+    },
+  ]);
+});
+
 sentryTest('does not capture request body when URL does not match', async ({ getLocalTestPath, page }) => {
   if (shouldSkipReplayTest()) {
     sentryTest.skip();

diff --git a/...r-integration-tests/suites/replay/extendNetworkBreadcrumbs/xhr/captureRequestBody/init.js b/...r-integration-tests/suites/replay/extendNetworkBreadcrumbs/xhr/captureRequestBody/init.js
@@ -5,7 +5,7 @@ window.Replay = new Sentry.Replay({
   flushMinDelay: 200,
   flushMaxDelay: 200,
 
-  networkDetailAllowUrls: ['http://localhost:7654/foo'],
+  networkDetailAllowUrls: ['http://localhost:7654/foo', 'http://sentry-test.io/foo'],
   networkCaptureBodies: true,
 });
 

diff --git a/...r-integration-tests/suites/replay/extendNetworkBreadcrumbs/xhr/captureRequestBody/test.ts b/...r-integration-tests/suites/replay/extendNetworkBreadcrumbs/xhr/captureRequestBody/test.ts
@@ -255,6 +255,87 @@ sentryTest('captures non-text request body', async ({ getLocalTestPath, page, br
   ]);
 });
 
+sentryTest('captures text request body when matching relative URL', async ({ getLocalTestUrl, page, browserName }) => {
+  // These are a bit flaky on non-chromium browsers
+  if (shouldSkipReplayTest() || browserName !== 'chromium') {
+    sentryTest.skip();
+  }
+
+  await page.route('**/foo', route => {
+    return route.fulfill({
+      status: 200,
+    });
+  });
+
+  await page.route('https://dsn.ingest.sentry.io/**/*', route => {
+    return route.fulfill({
+      status: 200,
+      contentType: 'application/json',
+      body: JSON.stringify({ id: 'test-id' }),
+    });
+  });
+
+  const requestPromise = waitForErrorRequest(page);
+  const replayRequestPromise1 = waitForReplayRequest(page, 0);
+
+  const url = await getLocalTestUrl({ testDir: __dirname });
+  await page.goto(url);
+
+  void page.evaluate(() => {
+    /* eslint-disable */
+    const xhr = new XMLHttpRequest();
+
+    xhr.open('POST', '/foo');
+    xhr.send('input body');
+
+    xhr.addEventListener('readystatechange', function () {
+      if (xhr.readyState === 4) {
+        // @ts-ignore Sentry is a global
+        setTimeout(() => Sentry.captureException('test error', 0));
+      }
+    });
+    /* eslint-enable */
+  });
+
+  const request = await requestPromise;
+  const eventData = envelopeRequestParser(request);
+
+  expect(eventData.exception?.values).toHaveLength(1);
+
+  expect(eventData?.breadcrumbs?.length).toBe(1);
+  expect(eventData!.breadcrumbs![0]).toEqual({
+    timestamp: expect.any(Number),
+    category: 'xhr',
+    type: 'http',
+    data: {
+      method: 'POST',
+      request_body_size: 10,
+      status_code: 200,
+      url: '/foo',
+    },
+  });
+
+  const replayReq1 = await replayRequestPromise1;
+  const { performanceSpans: performanceSpans1 } = getCustomRecordingEvents(replayReq1);
+  expect(performanceSpans1.filter(span => span.op === 'resource.xhr')).toEqual([
+    {
+      data: {
+        method: 'POST',
+        statusCode: 200,
+        request: {
+          size: 10,
+          headers: {},
+          body: 'input body',
+        },
+      },
+      description: '/foo',
+      endTimestamp: expect.any(Number),
+      op: 'resource.xhr',
+      startTimestamp: expect.any(Number),
+    },
+  ]);
+});
+
 sentryTest('does not capture request body when URL does not match', async ({ getLocalTestPath, page, browserName }) => {
   // These are a bit flaky on non-chromium browsers
   if (shouldSkipReplayTest() || browserName !== 'chromium') {

diff --git a/packages/replay/src/coreHandlers/util/networkUtils.ts b/packages/replay/src/coreHandlers/util/networkUtils.ts
@@ -1,7 +1,7 @@
 import type { TextEncoderInternal } from '@sentry/types';
 import { dropUndefinedKeys, stringMatchesSomePattern } from '@sentry/utils';
 
-import { NETWORK_BODY_MAX_SIZE } from '../../constants';
+import { NETWORK_BODY_MAX_SIZE, WINDOW } from '../../constants';
 import type {
   NetworkBody,
   NetworkMetaWarning,
@@ -234,5 +234,30 @@ function _strIsProbablyJson(str: string): boolean {
 
 /** Match an URL against a list of strings/Regex. */
 export function urlMatches(url: string, urls: (string | RegExp)[]): boolean {
-  return stringMatchesSomePattern(url, urls);
+  const fullUrl = getFullUrl(url);
+
+  return stringMatchesSomePattern(fullUrl, urls);
+}
+
+/** exported for tests */
+export function getFullUrl(url: string, baseURI = WINDOW.document.baseURI): string {
+  // Short circuit for common cases:
+  if (url.startsWith('http://') || url.startsWith('https://') || url.startsWith(WINDOW.location.origin)) {
+    return url;
+  }
+  const fixedUrl = new URL(url, baseURI);
+
+  // If these do not match, we are not dealing with a relative URL, so just return it
+  if (fixedUrl.origin !== new URL(baseURI).origin) {
+    return url;
+  }
+
+  const fullUrl = fixedUrl.href;
+
+  // Remove trailing slashes, if they don't match the original URL
+  if (!url.endsWith('/') && fullUrl.endsWith('/')) {
+    return fullUrl.slice(0, -1);
+  }
+
+  return fullUrl;
 }
diff --git a/packages/replay/test/unit/coreHandlers/util/networkUtils.test.ts b/packages/replay/test/unit/coreHandlers/util/networkUtils.test.ts
@@ -4,6 +4,7 @@ import { NETWORK_BODY_MAX_SIZE } from '../../../../src/constants';
 import {
   buildNetworkRequestOrResponse,
   getBodySize,
+  getFullUrl,
   parseContentLengthHeader,
 } from '../../../../src/coreHandlers/util/networkUtils';
 
@@ -219,4 +220,29 @@ describe('Unit | coreHandlers | util | networkUtils', () => {
       expect(actual).toEqual({ size: 1, headers: {}, body: expectedBody, _meta: expectedMeta });
     });
   });
+
+  describe('getFullUrl', () => {
+    it.each([
+      ['http://example.com', 'http://example.com', 'http://example.com'],
+      ['https://example.com', 'https://example.com', 'https://example.com'],
+      ['//example.com', 'https://example.com', 'https://example.com'],
+      ['//example.com', 'http://example.com', 'http://example.com'],
+      ['//example.com/', 'http://example.com', 'http://example.com/'],
+      ['//example.com/sub/aha.html', 'http://example.com', 'http://example.com/sub/aha.html'],
+      ['https://example.com/sub/aha.html', 'http://example.com', 'https://example.com/sub/aha.html'],
+      ['sub/aha.html', 'http://example.com', 'http://example.com/sub/aha.html'],
+      ['sub/aha.html', 'http://example.com/initial', 'http://example.com/sub/aha.html'],
+      ['sub/aha', 'http://example.com/initial/', 'http://example.com/initial/sub/aha'],
+      ['sub/aha/', 'http://example.com/initial/', 'http://example.com/initial/sub/aha/'],
+      ['sub/aha.html', 'http://example.com/initial/', 'http://example.com/initial/sub/aha.html'],
+      ['/sub/aha.html', 'http://example.com/initial/', 'http://example.com/sub/aha.html'],
+      ['./sub/aha.html', 'http://example.com/initial/', 'http://example.com/initial/sub/aha.html'],
+      ['../sub/aha.html', 'http://example.com/initial/', 'http://example.com/sub/aha.html'],
+      ['sub/aha.html', 'file:///Users/folder/file.html', 'file:///Users/folder/sub/aha.html'],
+      ['ws://example.com/sub/aha.html', 'http://example.com/initial/', 'ws://example.com/sub/aha.html'],
+    ])('works with %s & baseURI %s', (url, baseURI, expected) => {
+      const actual = getFullUrl(url, baseURI);
+      expect(actual).toBe(expected);
+    });
+  });
 });