fix(react): Parameterize React Router transactions with encoded params

packages/react/src/reactrouter-compat-utils/utils.ts

@@ -137,6 +137,31 @@ export function prefixWithSlash(path: string): string {
   return path[0] === '/' ? path : `/${path}`;
 }
 
+/**
+ * Removes as many leading segments from `pathname` as `pathnameBase` has, the way `useRoutes` strips the parent
+ * `pathnameBase` for descendant routes.
+ *
+ * `location.pathname` is percent-encoded as the browser holds it, while React Router decodes the pathname before
+ * matching, so `pathnameBase` on a `RouteMatch` is decoded (older 6.x routers decoded only partially, or not at all).
+ * Stripping by segment count works regardless of which side is encoded, and keeps the remainder encoded so that
+ * matching it again decodes it exactly once.
+ */
+function stripPathnameBaseSegments(pathname: string, pathnameBase: string): string {
+  const baseSegmentCount = countPathnameSegments(pathnameBase);
+  if (baseSegmentCount === 0) {
+    return pathname;
+  }
+  return prefixWithSlash(pathname.split('/').filter(Boolean).slice(baseSegmentCount).join('/'));
+}
+
+/**
+ * Counts the segments of a pathname with a plain split (`getNumberOfUrlSegments` also drops `,` segments, which a
+ * route parameter can legitimately be).
+ */
+function countPathnameSegments(pathname: string): number {
+  return pathname.split('/').filter(Boolean).length;
+}
+
 /**
  * Rebuilds the route path from all available routes by matching against the current location.
  */
@@ -154,7 +179,7 @@ export function rebuildRoutePathFromAllRoutes(
   for (const match of matchedRoutes) {
     if (match.route.path && match.route.path !== '*') {
       const path = pickPath(match);
-      const strippedPath = stripBasenameFromPathname(location.pathname, prefixWithSlash(match.pathnameBase));
+      const strippedPath = stripPathnameBaseSegments(location.pathname, prefixWithSlash(match.pathnameBase));
 
       if (location.pathname === strippedPath) {
         return trimSlash(strippedPath);
@@ -220,8 +245,7 @@ function reconstructNameFromDescendantParent(
     return undefined;
   }
 
-  const remainingPathname =
-    stripBasenameFromPathname(location.pathname, prefixWithSlash(parentMatch.pathnameBase)) || '/';
+  const remainingPathname = stripPathnameBaseSegments(location.pathname, prefixWithSlash(parentMatch.pathnameBase));
   const remainingName = rebuildRoutePathFromAllRoutes(
     allRoutes.filter(route => route !== parentMatch.route),
     { pathname: remainingPathname },
@@ -302,8 +326,10 @@ export function getNormalizedName(
     const newPath = path[0] === '/' || pathBuilder[pathBuilder.length - 1] === '/' ? path : `/${path}`;
     pathBuilder = trimSlash(pathBuilder) + prefixWithSlash(newPath);
 
-    // Check if this path matches the current location
-    if (trimSlash(location.pathname) !== trimSlash(basename + branch.pathname)) {
+    // Check if this branch spans the whole location. Compare segment counts rather than strings: the location
+    // pathname is percent-encoded while React Router decoded the branch pathname, and decoding never changes the
+    // number of segments.
+    if (countPathnameSegments(location.pathname) !== countPathnameSegments(basename + branch.pathname)) {
       continue;
     }
 
@@ -312,15 +338,15 @@ export function getNormalizedName(
       getNumberOfUrlSegments(pathBuilder) !== getNumberOfUrlSegments(branch.pathname) &&
       !pathEndsWithWildcard(pathBuilder)
     ) {
-      return [(stripBasename ? '' : basename) + newPath, 'route'];
+      return [(stripBasename ? '' : trimSlash(basename)) + newPath, 'route'];
     }
 
     // Handle wildcard routes with children - strip trailing wildcard
     if (pathIsWildcardAndHasChildren(pathBuilder, branch)) {
       pathBuilder = pathBuilder.slice(0, -1);
     }
 
-    return [(stripBasename ? '' : basename) + pathBuilder, 'route'];
+    return [(stripBasename ? '' : trimSlash(basename)) + pathBuilder, 'route'];
   }
 
   // Fallback when no matching route found

packages/react/test/react-router.test.tsx

@@ -80,6 +80,71 @@ describe('@sentry/react/react-router', () => {
     expect(getCurrentScope().getScopeData().transactionName).toEqual('/about');
   });
 
+  it.each([
+    ['/clubs/%ED%95%9C%EA%B8%80', 'non-ASCII, what a browser holds for `/clubs/한글`'],
+    ['/clubs/%2541', 'still looks percent-encoded after decoding (`%41`)'],
+    ['/clubs/100%25', 'not valid percent-encoding after decoding (`100%`)'],
+  ])('parameterizes a pageload whose pathname has a percent-encoded parameter (%s: %s)', pathname => {
+    const client = createMockBrowserClient();
+    setCurrentClient(client);
+
+    client.addIntegration(reactRouterBrowserTracingIntegration());
+
+    const SentryRoutes = wrapReactRouterRouting(Routes);
+
+    render(
+      <MemoryRouter initialEntries={[pathname]}>
+        <SentryRoutes>
+          <Route path="/" element={<div>Home</div>} />
+          <Route path="/clubs/:clubId" element={<div>Club</div>} />
+        </SentryRoutes>
+      </MemoryRouter>,
+    );
+
+    expect(mockStartBrowserTracingPageLoadSpan).toHaveBeenCalledTimes(1);
+    expect(getCurrentScope().getScopeData().transactionName).toEqual('/clubs/:clubId');
+  });
+
+  it('parameterizes a navigation whose pathname has a percent-encoded parameter', () => {
+    const client = createMockBrowserClient();
+    setCurrentClient(client);
+
+    client.addIntegration(reactRouterBrowserTracingIntegration());
+
+    const SentryRoutes = wrapReactRouterRouting(Routes);
+
+    function Home(): React.ReactElement {
+      const navigate = useNavigate();
+      return (
+        <button type="button" onClick={() => navigate('/clubs/%ED%95%9C%EA%B8%80')}>
+          to club
+        </button>
+      );
+    }
+
+    const { getByText } = render(
+      <MemoryRouter initialEntries={['/']}>
+        <SentryRoutes>
+          <Route path="/" element={<Home />} />
+          <Route path="/clubs/:clubId" element={<div>Club</div>} />
+        </SentryRoutes>
+      </MemoryRouter>,
+    );
+
+    fireEvent.click(getByText('to club'));
+
+    expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1);
+    expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), {
+      name: '/clubs/:clubId',
+      attributes: {
+        [SENTRY_SEGMENT_NAME_SOURCE]: 'route',
+        [URL_TEMPLATE]: '/clubs/:clubId',
+        [SENTRY_OP]: 'navigation',
+        [SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter',
+      },
+    });
+  });
+
   it('reactRouterBrowserTracingIntegration() instruments a navigation without passing router hooks', () => {
     const client = createMockBrowserClient();
     setCurrentClient(client);

packages/react/test/reactrouter-compat-utils/utils.test.ts

@@ -185,6 +185,28 @@ describe('reactrouter-compat-utils/utils', () => {
   });
 
   describe('rebuildRoutePathFromAllRoutes', () => {
+    it('should match a percent-encoded pathname against the decoded pathnameBase', () => {
+      const allRoutes: RouteObject[] = [{ path: ':clubId', element: null }];
+
+      // What a browser's `location.pathname` holds for `/한글`
+      const location: Location = { pathname: '/%ED%95%9C%EA%B8%80' };
+
+      const mockMatches: RouteMatch[] = [
+        {
+          route: { path: ':clubId', element: null },
+          params: { clubId: '한글' },
+          pathname: '/한글',
+          pathnameBase: '/한글',
+        },
+      ];
+
+      // The recursive call matches the remaining routes (none) against the stripped pathname
+      mockMatchRoutes.mockImplementation((routes: RouteObject[]) => (routes.length ? mockMatches : []));
+
+      const result = rebuildRoutePathFromAllRoutes(allRoutes, location, mockMatchRoutes as MatchRoutes);
+      expect(result).toBe(':clubId');
+    });
+
     it('should return pathname when it matches stripped path', () => {
       const allRoutes: RouteObject[] = [{ path: '/users', element: null }];
 
@@ -423,6 +445,112 @@ describe('reactrouter-compat-utils/utils', () => {
   });
 
   describe('getNormalizedName', () => {
+    it('should parameterize a percent-encoded pathname whose branch pathname is decoded', () => {
+      const routes: RouteObject[] = [{ path: '/clubs/:clubId', element: null }];
+      // What a browser's `location.pathname` holds for `/clubs/한글`
+      const location: Location = { pathname: '/clubs/%ED%95%9C%EA%B8%80' };
+      const branches: RouteMatch[] = [
+        {
+          route: { path: '/clubs/:clubId', element: null },
+          params: { clubId: '한글' },
+          pathname: '/clubs/한글',
+          pathnameBase: '/clubs/한글',
+        },
+      ];
+
+      const result = getNormalizedName(routes, location, branches, '');
+      expect(result).toEqual(['/clubs/:clubId', 'route']);
+    });
+
+    it('should keep matching when the basename itself is percent-encoded', () => {
+      const routes: RouteObject[] = [{ path: '/clubs/:clubId', element: null }];
+      // React Router strips the basename from the raw pathname, so an encoded basename is passed encoded
+      const basename = '/%ED%95%9C%EA%B5%AD';
+      const location: Location = { pathname: `${basename}/clubs/abc` };
+      const branches: RouteMatch[] = [
+        {
+          route: { path: '/clubs/:clubId', element: null },
+          params: { clubId: 'abc' },
+          pathname: '/clubs/abc',
+          pathnameBase: '/clubs/abc',
+        },
+      ];
+
+      expect(getNormalizedName(routes, location, branches, basename)).toEqual([`${basename}/clubs/:clubId`, 'route']);
+      expect(getNormalizedName(routes, location, branches, basename, true)).toEqual(['/clubs/:clubId', 'route']);
+    });
+
+    it('should not double the slash when the basename ends with one', () => {
+      const routes: RouteObject[] = [{ path: '/clubs/:clubId', element: null }];
+      const branches: RouteMatch[] = [
+        {
+          route: { path: '/clubs/:clubId', element: null },
+          params: { clubId: 'abc' },
+          pathname: '/clubs/abc',
+          pathnameBase: '/clubs/abc',
+        },
+      ];
+
+      // e.g. Vite's `import.meta.env.BASE_URL` always ends with a slash
+      expect(getNormalizedName(routes, { pathname: '/clubs/abc' }, branches, '/')).toEqual(['/clubs/:clubId', 'route']);
+      expect(getNormalizedName(routes, { pathname: '/app/clubs/abc' }, branches, '/app/')).toEqual([
+        '/app/clubs/:clubId',
+        'route',
+      ]);
+      expect(getNormalizedName(routes, { pathname: '/app/clubs/abc' }, branches, '/app/', true)).toEqual([
+        '/clubs/:clubId',
+        'route',
+      ]);
+    });
+
+    it('should keep matching when the router left the branch pathname encoded', () => {
+      // react-router < 6.5 matched against the raw pathname, 6.5 - 6.22 used `decodeURI` (keeps `%40`)
+      const routes: RouteObject[] = [{ path: '/users/:email', element: null }];
+      const location: Location = { pathname: '/users/a%40b.com' };
+      const branches: RouteMatch[] = [
+        {
+          route: { path: '/users/:email', element: null },
+          params: { email: 'a%40b.com' },
+          pathname: '/users/a%40b.com',
+          pathnameBase: '/users/a%40b.com',
+        },
+      ];
+
+      expect(getNormalizedName(routes, location, branches, '')).toEqual(['/users/:email', 'route']);
+    });
+
+    it('should parameterize a pathname whose parameter still looks percent-encoded after decoding', () => {
+      // `%2541` decodes to the literal `%41`; a second decode would turn it into `A`
+      const routes: RouteObject[] = [{ path: '/clubs/:clubId', element: null }];
+      const location: Location = { pathname: '/clubs/%2541' };
+      const branches: RouteMatch[] = [
+        {
+          route: { path: '/clubs/:clubId', element: null },
+          params: { clubId: '%41' },
+          pathname: '/clubs/%41',
+          pathnameBase: '/clubs/%41',
+        },
+      ];
+
+      expect(getNormalizedName(routes, location, branches, '')).toEqual(['/clubs/:clubId', 'route']);
+    });
+
+    it('should parameterize a pathname whose decoded parameter is not valid percent-encoding', () => {
+      // `100%25` decodes to `100%`, which cannot be decoded again
+      const routes: RouteObject[] = [{ path: '/clubs/:clubId', element: null }];
+      const location: Location = { pathname: '/clubs/100%25' };
+      const branches: RouteMatch[] = [
+        {
+          route: { path: '/clubs/:clubId', element: null },
+          params: { clubId: '100%' },
+          pathname: '/clubs/100%',
+          pathnameBase: '/clubs/100%',
+        },
+      ];
+
+      expect(getNormalizedName(routes, location, branches, '')).toEqual(['/clubs/:clubId', 'route']);
+    });
+
     it('should return pathname with url source when no routes provided', () => {
       const routes: RouteObject[] = [];
       const location: Location = { pathname: '/test' };

packages/react/test/reactrouter-descendant-routes.test.tsx

@@ -84,6 +84,81 @@ describe('React Router Descendant Routes', () => {
   });
 
   describe('withSentryReactRouterV6Routing', () => {
+    it('keeps the parent path prefix for a descendant route with a percent-encoded parameter', () => {
+      const client = createMockBrowserClient();
+      setCurrentClient(client);
+
+      client.addIntegration(
+        reactRouterV6BrowserTracingIntegration({
+          useEffect: React.useEffect,
+          useLocation,
+          useNavigationType,
+          createRoutesFromChildren,
+          matchRoutes,
+        }),
+      );
+      const SentryRoutes = withSentryReactRouterV6Routing(Routes);
+
+      const ChildRouter = () => (
+        <SentryRoutes>
+          <Route path=":id" element={<div id="child">Child</div>} />
+        </SentryRoutes>
+      );
+
+      const { container } = render(
+        // What a browser's `location.pathname` holds for `/child/한글`
+        <MemoryRouter initialEntries={['/child/%ED%95%9C%EA%B8%80']}>
+          <SentryRoutes>
+            <Route path="child/*" element={<ChildRouter />} />
+          </SentryRoutes>
+        </MemoryRouter>,
+      );
+
+      expect(container.innerHTML).toContain('Child');
+      expect(mockStartBrowserTracingPageLoadSpan).toHaveBeenCalledTimes(1);
+      expect(getCurrentScope().getScopeData().transactionName).toEqual('/child/:id');
+    });
+
+    it('re-matches the descendant remainder without decoding it twice', () => {
+      const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
+      const client = createMockBrowserClient();
+      setCurrentClient(client);
+
+      client.addIntegration(
+        reactRouterV6BrowserTracingIntegration({
+          useEffect: React.useEffect,
+          useLocation,
+          useNavigationType,
+          createRoutesFromChildren,
+          matchRoutes,
+        }),
+      );
+      const SentryRoutes = withSentryReactRouterV6Routing(Routes);
+
+      const ChildRouter = () => (
+        <SentryRoutes>
+          <Route path=":id" element={<div id="child">Child</div>} />
+        </SentryRoutes>
+      );
+
+      try {
+        const { container } = render(
+          // `100%25` decodes to `100%`, which React Router cannot decode a second time
+          <MemoryRouter initialEntries={['/child/100%25']}>
+            <SentryRoutes>
+              <Route path="child/*" element={<ChildRouter />} />
+            </SentryRoutes>
+          </MemoryRouter>,
+        );
+
+        expect(container.innerHTML).toContain('Child');
+        expect(getCurrentScope().getScopeData().transactionName).toEqual('/child/:id');
+        expect(warnSpy).not.toHaveBeenCalledWith(expect.stringContaining('could not be decoded'));
+      } finally {
+        warnSpy.mockRestore();
+      }
+    });
+
     it('keeps the parent path prefix for descendant routes with non-wildcard nested children - pageload', () => {
       const client = createMockBrowserClient();
       setCurrentClient(client);