Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

- Attach the on-error Session Replay buffer to feedback submitted from the feedback form ([#6870](https://github.com/getsentry/sentry-react-native/pull/6870))
- Do not mark `sendDefaultPii` as deprecated in the React Native options ([#6864](https://github.com/getsentry/sentry-react-native/pull/6864))
- Fix duplicate HTTP breadcrumbs for requests with a query string or differently formatted URLs ([#6881](https://github.com/getsentry/sentry-react-native/pull/6881))
- Include CPU and memory measurements in Android profiles ([#6891](https://github.com/getsentry/sentry-react-native/pull/6891))

### Dependencies
Expand Down
61 changes: 60 additions & 1 deletion packages/core/src/js/integrations/devicecontext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,19 +119,25 @@ function deduplicateNativeHttpBreadcrumbs(nativeBreadcrumbs: Breadcrumb[], jsBre
}

const consumedJsIndices = new Set<number>();
const jsUrls = jsHttpBreadcrumbs.map(b => normalizeJsHttpUrl(b.data?.url));

return nativeBreadcrumbs.filter(nativeBreadcrumb => {
if (nativeBreadcrumb.type !== 'http' || nativeBreadcrumb.category !== 'http') {
return true;
}

const nativeUrl = normalizeNativeHttpUrl(nativeBreadcrumb.data?.url);

const matchIndex = jsHttpBreadcrumbs.findIndex((jsBreadcrumb, index) => {
if (consumedJsIndices.has(index)) {
return false;
}

const jsUrl = jsUrls[index];
const sameMethod = nativeBreadcrumb.data?.method === jsBreadcrumb.data?.method;
const sameUrl = nativeBreadcrumb.data?.url === jsBreadcrumb.data?.url;
const sameUrl =
nativeBreadcrumb.data?.url === jsBreadcrumb.data?.url ||
(nativeUrl !== undefined && jsUrl !== undefined && isSameHttpUrl(nativeUrl, jsUrl));
const nativeStatus = nativeBreadcrumb.data?.status_code;
const jsStatus = jsBreadcrumb.data?.status_code;
const sameStatus =
Expand All @@ -155,3 +161,56 @@ function deduplicateNativeHttpBreadcrumbs(nativeBreadcrumbs: Breadcrumb[], jsBre
return true;
});
}

/*
* The JS breadcrumb keeps the raw request URL. The native one is sanitized: credentials are replaced,
* the fragment is removed, the query is moved to `data["http.query"]` or kept filtered, and the URL
* is percent-decoded. The query is ignored because native may filter or drop it.
*/

/** Returns the decoded JS URL without query, fragment or trailing slash. */
function normalizeJsHttpUrl(url: unknown): string | undefined {
if (typeof url !== 'string') {
return undefined;
}

// Cut on the raw URL, where `?` and `#` are always delimiters.
const queryOrFragmentStart = url.search(/[?#]/);
const normalized = decodeUrl(
normalizeUrlAuthority(queryOrFragmentStart === -1 ? url : url.slice(0, queryOrFragmentStart)),
);

return normalized.length > 1 && normalized.endsWith('/') ? normalized.slice(0, -1) : normalized;
}

/** Returns the decoded native URL. It may still contain a (filtered) query. */
function normalizeNativeHttpUrl(url: unknown): string | undefined {
return typeof url === 'string' ? decodeUrl(normalizeUrlAuthority(url)) : undefined;
}

/** Matches the native URL to the JS one, with or without a trailing slash or query. */
function isSameHttpUrl(nativeUrl: string, jsUrl: string): boolean {
return (
nativeUrl === jsUrl ||

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isnt this check redundant since we do an equal check on line 139?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question ๐Ÿ‘ I rechecked and line 139 compares the raw URLs while this compares the normalised URLs

nativeUrl === `${jsUrl}/` ||
nativeUrl.startsWith(`${jsUrl}?`) ||
nativeUrl.startsWith(`${jsUrl}/?`)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a chance the question mark will be %3F?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is already covered since it is decoded at this point. Also checked with the the path has an encoded question mark test

);
}

/** Removes credentials and lowercases the scheme and host. */
function normalizeUrlAuthority(url: string): string {
return url.replace(/^([a-zA-Z][a-zA-Z0-9+.-]*:\/\/)([^/?#]*)/, (_match, scheme: string, authority: string) => {
const host = authority.slice(authority.lastIndexOf('@') + 1);
return `${scheme}${host}`.toLowerCase();
});
}

function decodeUrl(url: string): string {
try {
return decodeURIComponent(url);
} catch {
// Malformed percent-encoding, compare as is.
return url;
}
}
113 changes: 113 additions & 0 deletions packages/core/test/integrations/devicecontext.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -614,6 +614,119 @@ describe('Device Context Integration', () => {
},
]);
});

it.each([
['the js url has a query string', 'https://api.example.com/posts', 'https://api.example.com/posts?page=1'],
['the js url has a fragment', 'https://api.example.com/posts', 'https://api.example.com/posts#top'],
[
'the native url keeps a filtered query',
'https://api.example.com/posts?page=1&token=[Filtered]',
'https://api.example.com/posts?page=1&token=abc',
],
['host casing and trailing slashes differ', 'https://API.Example.com/data/', 'https://api.example.com/data'],
['the url is the root with a port', 'https://api.example.com:8080/', 'https://api.example.com:8080'],
[
'the native url is percent-decoded',
'https://api.example.com/users/john doe',
'https://api.example.com/users/john%20doe',
],
['the path has an encoded question mark', 'https://api.example.com/a?b', 'https://api.example.com/a%3Fb?x=1'],
[
'the native url has filtered credentials',
'https://[Filtered]:[Filtered]@api.example.com/data',
'https://user:Pass%2Fword@api.example.com/data',
],
['the url has malformed percent-encoding', 'https://api.example.com/100%', 'https://api.example.com/100%?x=1'],
[
'the urls are identical with a fragment',
'https://api.example.com/docs#intro',
'https://api.example.com/docs#intro',
],
])('deduplicates when %s', async (_name, nativeUrl, jsUrl) => {
const jsBreadcrumb = {
category: 'xhr',
type: 'http',
data: { method: 'GET', url: jsUrl, status_code: 200 },
timestamp: 1730985899,
};

const { processedEvent } = await processEventWith(
{
nativeContexts: {
breadcrumbs: [
{
category: 'http',
type: 'http',
data: { method: 'GET', url: nativeUrl, status_code: 200 },
timestamp: 'Thursday, November 7, 2024 3:24:59 PM GMT+02:00',
},
],
},
mockEvent: { breadcrumbs: [jsBreadcrumb] },
},
mockClient,
);

expect(processedEvent?.breadcrumbs).toStrictEqual([jsBreadcrumb]);
});

it.each([
['the paths differ only in case', 'https://api.example.com/Data', 'https://api.example.com/data'],
['the native path is longer', 'https://api.example.com/data-archive', 'https://api.example.com/data'],
])('keeps both breadcrumbs when %s', async (_name, nativeUrl, jsUrl) => {
const nativeBreadcrumb = {
category: 'http',
type: 'http',
data: { method: 'GET', url: nativeUrl, status_code: 200 },
timestamp: 1730985899,
};
const jsBreadcrumb = {
category: 'xhr',
type: 'http',
data: { method: 'GET', url: jsUrl, status_code: 200 },
timestamp: 1730985899,
};

const { processedEvent } = await processEventWith(
{
nativeContexts: {
breadcrumbs: [{ ...nativeBreadcrumb, timestamp: 'Thursday, November 7, 2024 3:24:59 PM GMT+02:00' }],
},
mockEvent: { breadcrumbs: [jsBreadcrumb] },
},
mockClient,
);

expect(processedEvent?.breadcrumbs).toStrictEqual([nativeBreadcrumb, jsBreadcrumb]);
});

it('deduplicates when the native query differs, since the query is ignored', async () => {
const jsBreadcrumb = {
category: 'xhr',
type: 'http',
data: { method: 'GET', url: 'https://api.example.com/img?w=1', status_code: 200 },
timestamp: 1730985899,
};

const { processedEvent } = await processEventWith(
{
nativeContexts: {
breadcrumbs: [
{
category: 'http',
type: 'http',
data: { method: 'GET', url: 'https://api.example.com/img', 'http.query': 'w=2', status_code: 200 },
timestamp: 'Thursday, November 7, 2024 3:24:59 PM GMT+02:00',
},
],
},
mockEvent: { breadcrumbs: [jsBreadcrumb] },
},
mockClient,
);

expect(processedEvent?.breadcrumbs).toStrictEqual([jsBreadcrumb]);
});
});

it('do not add in_foreground if unknown', async () => {
Expand Down
Loading