Skip to content

Commit ff84d97

Browse files
mydeamydea
committed
fix(replay): Check relative URLs correctly
1 parent f1c8d36 commit ff84d97

File tree

8 files changed

+219
-8
lines changed

8 files changed

+219
-8
lines changed

‎packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureRequestBody/init.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ window.Replay = new Sentry.Replay({
55
flushMinDelay: 200,
66
flushMaxDelay: 200,
77

8-
networkDetailAllowUrls: ['http://localhost:7654/foo'],
8+
networkDetailAllowUrls: ['http://localhost:7654/foo', 'http://sentry-test.io/foo'],
99
networkCaptureBodies: true,
1010
});
1111

‎packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureRequestBody/test.ts

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,85 @@ sentryTest('captures non-text request body', async ({ getLocalTestPath, page, br
249249
]);
250250
});
251251

252+
sentryTest('captures text request body when matching relative URL', async ({ getLocalTestUrl, page, browserName }) => {
253+
if (shouldSkipReplayTest()) {
254+
sentryTest.skip();
255+
}
256+
257+
const additionalHeaders = browserName === 'webkit' ? { 'content-type': 'text/plain' } : undefined;
258+
259+
await page.route('**/foo', route => {
260+
return route.fulfill({
261+
status: 200,
262+
});
263+
});
264+
265+
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
266+
return route.fulfill({
267+
status: 200,
268+
contentType: 'application/json',
269+
body: JSON.stringify({ id: 'test-id' }),
270+
});
271+
});
272+
273+
const requestPromise = waitForErrorRequest(page);
274+
const replayRequestPromise1 = waitForReplayRequest(page, 0);
275+
276+
const url = await getLocalTestUrl({ testDir: __dirname });
277+
await page.goto(url);
278+
279+
await page.evaluate(() => {
280+
/* eslint-disable */
281+
fetch('/foo', {
282+
method: 'POST',
283+
body: 'input body',
284+
}).then(() => {
285+
// @ts-ignore Sentry is a global
286+
Sentry.captureException('test error');
287+
});
288+
/* eslint-enable */
289+
});
290+
291+
const request = await requestPromise;
292+
const eventData = envelopeRequestParser(request);
293+
294+
expect(eventData.exception?.values).toHaveLength(1);
295+
296+
expect(eventData?.breadcrumbs?.length).toBe(1);
297+
expect(eventData!.breadcrumbs![0]).toEqual({
298+
timestamp: expect.any(Number),
299+
category: 'fetch',
300+
type: 'http',
301+
data: {
302+
method: 'POST',
303+
request_body_size: 10,
304+
status_code: 200,
305+
url: '/foo',
306+
},
307+
});
308+
309+
const replayReq1 = await replayRequestPromise1;
310+
const { performanceSpans: performanceSpans1 } = getCustomRecordingEvents(replayReq1);
311+
expect(performanceSpans1.filter(span => span.op === 'resource.fetch')).toEqual([
312+
{
313+
data: {
314+
method: 'POST',
315+
statusCode: 200,
316+
request: {
317+
size: 10,
318+
headers: {},
319+
body: 'input body',
320+
},
321+
response: additionalHeaders ? { headers: additionalHeaders } : undefined,
322+
},
323+
description: '/foo',
324+
endTimestamp: expect.any(Number),
325+
op: 'resource.fetch',
326+
startTimestamp: expect.any(Number),
327+
},
328+
]);
329+
});
330+
252331
sentryTest('does not capture request body when URL does not match', async ({ getLocalTestPath, page }) => {
253332
if (shouldSkipReplayTest()) {
254333
sentryTest.skip();

‎packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/xhr/captureRequestBody/init.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ window.Replay = new Sentry.Replay({
55
flushMinDelay: 200,
66
flushMaxDelay: 200,
77

8-
networkDetailAllowUrls: ['http://localhost:7654/foo'],
8+
networkDetailAllowUrls: ['http://localhost:7654/foo', 'http://sentry-test.io/foo'],
99
networkCaptureBodies: true,
1010
});
1111

‎packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/xhr/captureRequestBody/test.ts

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,87 @@ sentryTest('captures non-text request body', async ({ getLocalTestPath, page, br
255255
]);
256256
});
257257

258+
sentryTest('captures text request body when matching relative URL', async ({ getLocalTestUrl, page, browserName }) => {
259+
// These are a bit flaky on non-chromium browsers
260+
if (shouldSkipReplayTest() || browserName !== 'chromium') {
261+
sentryTest.skip();
262+
}
263+
264+
await page.route('**/foo', route => {
265+
return route.fulfill({
266+
status: 200,
267+
});
268+
});
269+
270+
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
271+
return route.fulfill({
272+
status: 200,
273+
contentType: 'application/json',
274+
body: JSON.stringify({ id: 'test-id' }),
275+
});
276+
});
277+
278+
const requestPromise = waitForErrorRequest(page);
279+
const replayRequestPromise1 = waitForReplayRequest(page, 0);
280+
281+
const url = await getLocalTestUrl({ testDir: __dirname });
282+
await page.goto(url);
283+
284+
void page.evaluate(() => {
285+
/* eslint-disable */
286+
const xhr = new XMLHttpRequest();
287+
288+
xhr.open('POST', '/foo');
289+
xhr.send('input body');
290+
291+
xhr.addEventListener('readystatechange', function () {
292+
if (xhr.readyState === 4) {
293+
// @ts-ignore Sentry is a global
294+
setTimeout(() => Sentry.captureException('test error', 0));
295+
}
296+
});
297+
/* eslint-enable */
298+
});
299+
300+
const request = await requestPromise;
301+
const eventData = envelopeRequestParser(request);
302+
303+
expect(eventData.exception?.values).toHaveLength(1);
304+
305+
expect(eventData?.breadcrumbs?.length).toBe(1);
306+
expect(eventData!.breadcrumbs![0]).toEqual({
307+
timestamp: expect.any(Number),
308+
category: 'xhr',
309+
type: 'http',
310+
data: {
311+
method: 'POST',
312+
request_body_size: 10,
313+
status_code: 200,
314+
url: '/foo',
315+
},
316+
});
317+
318+
const replayReq1 = await replayRequestPromise1;
319+
const { performanceSpans: performanceSpans1 } = getCustomRecordingEvents(replayReq1);
320+
expect(performanceSpans1.filter(span => span.op === 'resource.xhr')).toEqual([
321+
{
322+
data: {
323+
method: 'POST',
324+
statusCode: 200,
325+
request: {
326+
size: 10,
327+
headers: {},
328+
body: 'input body',
329+
},
330+
},
331+
description: '/foo',
332+
endTimestamp: expect.any(Number),
333+
op: 'resource.xhr',
334+
startTimestamp: expect.any(Number),
335+
},
336+
]);
337+
});
338+
258339
sentryTest('does not capture request body when URL does not match', async ({ getLocalTestPath, page, browserName }) => {
259340
// These are a bit flaky on non-chromium browsers
260341
if (shouldSkipReplayTest() || browserName !== 'chromium') {

‎packages/browser-integration-tests/utils/fixtures.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { test as base } from '@playwright/test';
33
import fs from 'fs';
44
import path from 'path';
55

6-
import { generateLoader, generatePage } from './generatePage';
6+
import { maybeGenerateLoader, generatePage } from './generatePage';
77

88
export const TEST_HOST = 'http://sentry-test.io';
99

@@ -53,7 +53,7 @@ const sentryTest = base.extend<TestFixtures>({
5353
const pagePath = `${TEST_HOST}/index.html`;
5454

5555
await build(testDir);
56-
generateLoader(testDir);
56+
maybeGenerateLoader(testDir);
5757

5858
// Serve all assets under
5959
await page.route(`${TEST_HOST}/*.*`, route => {

‎packages/browser-integration-tests/utils/generatePage.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,10 @@ const LOADER_CONFIGS: Record<string, { bundle: string; options: Record<string, u
4444

4545
const bundleKey = process.env.PW_BUNDLE || '';
4646

47-
export function generateLoader(outPath: string): void {
47+
export function maybeGenerateLoader(outPath: string): void {
4848
const localPath = `${outPath}/dist`;
4949

50-
if (!existsSync(localPath)) {
50+
if (!existsSync(localPath) || !bundleKey.includes('loader')) {
5151
return;
5252
}
5353

‎packages/replay/src/coreHandlers/util/networkUtils.ts

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import type { TextEncoderInternal } from '@sentry/types';
22
import { dropUndefinedKeys, stringMatchesSomePattern } from '@sentry/utils';
33

4-
import { NETWORK_BODY_MAX_SIZE } from '../../constants';
4+
import { NETWORK_BODY_MAX_SIZE, WINDOW } from '../../constants';
55
import type {
66
NetworkBody,
77
NetworkMetaWarning,
@@ -234,5 +234,30 @@ function _strIsProbablyJson(str: string): boolean {
234234

235235
/** Match an URL against a list of strings/Regex. */
236236
export function urlMatches(url: string, urls: (string | RegExp)[]): boolean {
237-
return stringMatchesSomePattern(url, urls);
237+
const fullUrl = getFullUrl(url);
238+
239+
return stringMatchesSomePattern(fullUrl, urls);
240+
}
241+
242+
/** exported for tests */
243+
export function getFullUrl(url: string, baseURI = WINDOW.document.baseURI): string {
244+
// Short circuit for common cases:
245+
if (url.startsWith('http://') || url.startsWith('https://') || url.startsWith(WINDOW.location.origin)) {
246+
return url;
247+
}
248+
const fixedUrl = new URL(url, baseURI);
249+
250+
// If these do not match, we are not dealing with a relative URL, so just return it
251+
if (fixedUrl.origin !== new URL(baseURI).origin) {
252+
return url;
253+
}
254+
255+
const fullUrl = fixedUrl.href;
256+
257+
// Remove trailing slashes, if they don't match the original URL
258+
if (!url.endsWith('/') && fullUrl.endsWith('/')) {
259+
return fullUrl.slice(0, -1);
260+
}
261+
262+
return fullUrl;
238263
}

‎packages/replay/test/unit/coreHandlers/util/networkUtils.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { NETWORK_BODY_MAX_SIZE } from '../../../../src/constants';
44
import {
55
buildNetworkRequestOrResponse,
66
getBodySize,
7+
getFullUrl,
78
parseContentLengthHeader,
89
} from '../../../../src/coreHandlers/util/networkUtils';
910

@@ -219,4 +220,29 @@ describe('Unit | coreHandlers | util | networkUtils', () => {
219220
expect(actual).toEqual({ size: 1, headers: {}, body: expectedBody, _meta: expectedMeta });
220221
});
221222
});
223+
224+
describe('getFullUrl', () => {
225+
it.each([
226+
['http://example.com', 'http://example.com', 'http://example.com'],
227+
['https://example.com', 'https://example.com', 'https://example.com'],
228+
['//example.com', 'https://example.com', 'https://example.com'],
229+
['//example.com', 'http://example.com', 'http://example.com'],
230+
['//example.com/', 'http://example.com', 'http://example.com/'],
231+
['//example.com/sub/aha.html', 'http://example.com', 'http://example.com/sub/aha.html'],
232+
['https://example.com/sub/aha.html', 'http://example.com', 'https://example.com/sub/aha.html'],
233+
['sub/aha.html', 'http://example.com', 'http://example.com/sub/aha.html'],
234+
['sub/aha.html', 'http://example.com/initial', 'http://example.com/sub/aha.html'],
235+
['sub/aha', 'http://example.com/initial/', 'http://example.com/initial/sub/aha'],
236+
['sub/aha/', 'http://example.com/initial/', 'http://example.com/initial/sub/aha/'],
237+
['sub/aha.html', 'http://example.com/initial/', 'http://example.com/initial/sub/aha.html'],
238+
['/sub/aha.html', 'http://example.com/initial/', 'http://example.com/sub/aha.html'],
239+
['./sub/aha.html', 'http://example.com/initial/', 'http://example.com/initial/sub/aha.html'],
240+
['../sub/aha.html', 'http://example.com/initial/', 'http://example.com/sub/aha.html'],
241+
['sub/aha.html', 'file:///Users/folder/file.html', 'file:///Users/folder/sub/aha.html'],
242+
['ws://example.com/sub/aha.html', 'http://example.com/initial/', 'ws://example.com/sub/aha.html'],
243+
])('works with %s & baseURI %s', (url, baseURI, expected) => {
244+
const actual = getFullUrl(url, baseURI);
245+
expect(actual).toBe(expected);
246+
});
247+
});
222248
});

0 commit comments

Comments
 (0)