URL encode some well known unsafe characters in RouteResolver

Signed-off-by: Fredrik Adelöw <freben@gmail.com>

.changeset/twelve-days-walk.md

@@ -0,0 +1,5 @@
+---
+'@backstage/core-app-api': minor
+---
+
+URL encode some well known unsafe characters in `RouteResolver` (and therefore `useRouteRef`)

packages/core-app-api/package.json

@@ -50,6 +50,7 @@
     "@types/react": "^16.13.1 || ^17.0.0",
     "history": "^5.0.0",
     "i18next": "^22.4.15",
+    "lodash": "^4.17.21",
     "prop-types": "^15.7.2",
     "react-use": "^17.2.4",
     "zen-observable": "^0.10.0",

packages/core-app-api/src/routing/RouteResolver.beta.test.ts

@@ -364,4 +364,31 @@ describe('RouteResolver', () => {
       /^Cannot route.*with parent.*as it has parameters$/,
     );
   });
+
+  it('should encode some characters in params', () => {
+    const r = new RouteResolver(
+      new Map<RouteRef, string>([
+        [ref2, 'my-parent/:x'],
+        [ref1, 'my-route'],
+      ]),
+      new Map<RouteRef, RouteRef>([[ref1, ref2]]),
+      [
+        {
+          routeRefs: new Set([ref2]),
+          path: 'my-parent/:x',
+          ...rest,
+          children: [
+            MATCH_ALL_ROUTE,
+            { routeRefs: new Set([ref1]), path: 'my-route', ...rest },
+          ],
+        },
+      ],
+      new Map(),
+      '/base',
+    );
+
+    expect(r.resolve(ref2, '/')?.({ x: 'a/#&?b' })).toBe(
+      '/base/my-parent/a%2F%23%26%3Fb',
+    );
+  });
 });

packages/core-app-api/src/routing/RouteResolver.compat.test.ts

@@ -390,4 +390,33 @@ describe.each(['beta', 'stable'])('react-router %s', rrVersion => {
       /^Cannot route.*with parent.*as it has parameters$/,
     );
   });
+
+  it('should encode some characters in params', () => {
+    const { RouteResolver } =
+      require('./RouteResolver') as typeof import('./RouteResolver');
+    const r = new RouteResolver(
+      new Map<RouteRef, string>([
+        [ref2, 'my-parent/:x'],
+        [ref1, 'my-route'],
+      ]),
+      new Map<RouteRef, RouteRef>([[ref1, ref2]]),
+      [
+        {
+          routeRefs: new Set([ref2]),
+          path: 'my-parent/:x',
+          ...rest,
+          children: [
+            MATCH_ALL_ROUTE,
+            { routeRefs: new Set([ref1]), path: 'my-route', ...rest },
+          ],
+        },
+      ],
+      new Map(),
+      '/base',
+    );
+
+    expect(r.resolve(ref2, '/')?.({ x: 'a/#&?b' })).toBe(
+      '/base/my-parent/a%2F%23%26%3Fb',
+    );
+  });
 });

packages/core-app-api/src/routing/RouteResolver.stable.test.ts

@@ -364,4 +364,31 @@ describe('RouteResolver', () => {
       /^Cannot route.*with parent.*as it has parameters$/,
     );
   });
+
+  it('should encode some characters in params', () => {
+    const r = new RouteResolver(
+      new Map<RouteRef, string>([
+        [ref2, 'my-parent/:x'],
+        [ref1, 'my-route'],
+      ]),
+      new Map<RouteRef, RouteRef>([[ref1, ref2]]),
+      [
+        {
+          routeRefs: new Set([ref2]),
+          path: 'my-parent/:x',
+          ...rest,
+          children: [
+            MATCH_ALL_ROUTE,
+            { routeRefs: new Set([ref1]), path: 'my-route', ...rest },
+          ],
+        },
+      ],
+      new Map(),
+      '/base',
+    );
+
+    expect(r.resolve(ref2, '/')?.({ x: 'a/#&?b' })).toBe(
+      '/base/my-parent/a%2F%23%26%3Fb',
+    );
+  });
 });

packages/core-app-api/src/routing/RouteResolver.ts

@@ -31,6 +31,7 @@ import {
   SubRouteRef,
 } from '@backstage/core-plugin-api';
 import { joinPaths } from './helpers';
+import mapValues from 'lodash/mapValues';
 
 /**
  * Resolves the absolute route ref that our target route ref is pointing pointing to, as well
@@ -225,7 +226,23 @@ export class RouteResolver {
       );
 
     const routeFunc: RouteFunc<Params> = (...[params]) => {
-      return joinPaths(basePath, generatePath(targetPath, params));
+      // We selectively encode some some known-dangerous characters in the
+      // params. The reason that we don't perform a blanket `encodeURIComponent`
+      // here is that this encoding was added defensively long after the initial
+      // release of this code. There's likely to be many users of this code that
+      // already encode their parameters knowing that this code didn't do this
+      // for them in the past. Therefore, we are extra careful NOT to include
+      // the percent character in this set, even though that might seem like a
+      // bad idea.
+      const encodedParams =
+        params &&
+        mapValues(params, value => {
+          if (typeof value === 'string') {
+            return value.replaceAll(/[&?#;\/]/g, c => encodeURIComponent(c));
+          }
+          return value;
+        });
+      return joinPaths(basePath, generatePath(targetPath, encodedParams));
     };
     return routeFunc;
   }

yarn.lock

@@ -3955,6 +3955,7 @@ __metadata:
     "@types/zen-observable": ^0.8.0
     history: ^5.0.0
     i18next: ^22.4.15
+    lodash: ^4.17.21
     msw: ^1.0.0
     prop-types: ^15.7.2
     react-router-beta: "npm:react-router@6.0.0-beta.0"