diff --git a/packages/browser-utils/src/instrumentation/location.ts b/packages/browser-utils/src/instrumentation/location.ts index 7b6dfc5243a0..5e62a777129e 100644 --- a/packages/browser-utils/src/instrumentation/location.ts +++ b/packages/browser-utils/src/instrumentation/location.ts @@ -6,6 +6,8 @@ import { WINDOW } from '../types'; * * @return the absolute URL when handed a relative URL path, by * combining the current `window.location.origin` with param urlOrPath. + * A path starting with `//` is treated as a path on the current origin, + * not as a protocol-relative URL. * * If param urlOrPath is already a full or an invalid URL, * or URL determination fails, the original param urlOrPath is returned unchanged. @@ -13,7 +15,10 @@ import { WINDOW } from '../types'; */ export function getAbsoluteUrl(urlOrPath: string): string { try { - const url = new URL(urlOrPath, WINDOW.location.origin); + const origin = WINDOW.location.origin; + // Callers pass untyped router arguments, which `new URL` used to stringify for us. + const path = String(urlOrPath); + const url = path.startsWith('//') ? new URL(`${origin}${path}`) : new URL(path, origin); return url.toString(); } catch { // fallback, just do nothing diff --git a/packages/browser-utils/test/instrumentation/location.test.ts b/packages/browser-utils/test/instrumentation/location.test.ts index c67820d33fff..8d28b55d7a3b 100644 --- a/packages/browser-utils/test/instrumentation/location.test.ts +++ b/packages/browser-utils/test/instrumentation/location.test.ts @@ -24,4 +24,11 @@ describe('getAbsoluteUrl', () => { it('returns fragments', () => { expect(getAbsoluteUrl('/foo#bar')).toBe('https://sentry.io/foo#bar'); }); + + it.each([ + ['//foo/bar', 'https://sentry.io//foo/bar'], + ['//foo/bar?x=1#h', 'https://sentry.io//foo/bar?x=1#h'], + ])('keeps the current origin for a path starting with `//` (%s)', (path, expected) => { + expect(getAbsoluteUrl(path)).toBe(expected); + }); }); diff --git a/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts b/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts index f2f933bc11bf..5a20f7b68b76 100644 --- a/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts +++ b/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts @@ -26,10 +26,20 @@ import { } from '@sentry/conventions/attributes'; import { NAVIGATION } from '@sentry/conventions/op'; +/** + * Unlike `getAbsoluteUrl`, keeps a `//host/path` target protocol-relative: Next.js treats such an + * href as an external URL and navigates to that host. + */ +function getNavigationTargetUrl(href: string): string { + // `router.push()` arguments reach this untyped, so a non-string must not throw here. + const isProtocolRelative = typeof href === 'string' && href.startsWith('//'); + return getAbsoluteUrl(isProtocolRelative ? `${WINDOW.location.protocol}${href}` : href); +} + function setNavigationSpanUrlAttributes(span: Span, urlPath: string, urlOrPath: string): void { span.setAttributes({ [URL_PATH]: urlPath, - [URL_FULL]: filterCollectedUrl(getAbsoluteUrl(urlOrPath)), + [URL_FULL]: filterCollectedUrl(getNavigationTargetUrl(urlOrPath)), }); } @@ -170,7 +180,7 @@ export function appRouterInstrumentNavigation(client: Client): void { [URL_TEMPLATE]: parameterizedPathname, }, }, - { url: getAbsoluteUrl(normalizedHref) }, + { url: getNavigationTargetUrl(normalizedHref) }, ); } }; @@ -306,7 +316,7 @@ function patchRouter(client: Client, router: NextRouter, currentNavigationSpanRe [URL_TEMPLATE]: parameterizedPathname, }, }, - { url: getAbsoluteUrl(normalizedHref) }, + { url: getNavigationTargetUrl(normalizedHref) }, ); return target.apply(thisArg, argArray); diff --git a/packages/nextjs/test/client/appRouterRoutingInstrumentation.test.ts b/packages/nextjs/test/client/appRouterRoutingInstrumentation.test.ts index 25ecbfdd3f80..4d94c5dd2cdc 100644 --- a/packages/nextjs/test/client/appRouterRoutingInstrumentation.test.ts +++ b/packages/nextjs/test/client/appRouterRoutingInstrumentation.test.ts @@ -196,6 +196,26 @@ describe('appRouterInstrumentNavigation (router-patch mode)', () => { expect(core.spanToJSON(pushSpan!).end_timestamp).toBeDefined(); }); + it('records a protocol-relative `router.push()` target as an external URL', async () => { + const { core, router } = await setup(traceLifecycle); + + router.push('//cdn.example.com/x'); + + expect(core.spanToJSON(core.getActiveSpan()!).attributes).toEqual( + expect.objectContaining({ [URL_FULL]: 'http://cdn.example.com/x' }), + ); + }); + + it('records a protocol-relative transition-start-hook target as an external URL', async () => { + const { core, instrumentation } = await setup(traceLifecycle); + + instrumentation.captureRouterTransitionStart('//cdn.example.com/x', 'push'); + + expect(core.spanToJSON(core.getActiveSpan()!).attributes).toEqual( + expect.objectContaining({ [URL_FULL]: 'http://cdn.example.com/x' }), + ); + }); + it('tags a popstate without a preceding router call with `browser.popstate`', async () => { const { core } = await setup(traceLifecycle); @@ -260,6 +280,16 @@ describe('appRouterInstrumentNavigation with basePath', () => { ); }); + it('records a `//` target as a path under basePath, like Next.js does', async () => { + const { core, router } = await setup('static'); + + router.push('//cdn.example.com/x'); + + expect(core.spanToJSON(core.getActiveSpan()!).attributes).toEqual( + expect.objectContaining({ [URL_FULL]: 'http://localhost:3000/my-app//cdn.example.com/x' }), + ); + }); + it.each([ ['a root-relative path without basePath', '/navigation/42/router-push'], ['a root-relative path with basePath', '/my-app/navigation/42/router-push'],