Show branch names decoded in the branch selector

Closes c639643a8e

src/lib/router.ts

@@ -425,16 +425,6 @@ export function extractBaseUrl(hostAndPort: string): BaseUrl {
 // encoded shape never reaches link building or anything user-visible. Only
 // identifiers are decoded: revision and path segments are deliberately kept
 // encoded, because branch and tag lookups are keyed on the encoded name.
-// Decoding throws on a stray `%` in a repo alias, which is operator-supplied
-// and unvalidated, so an undecodable segment is passed through untouched.
-function decodeIdentifier(segment: string): string {
-  try {
-    return decodeURIComponent(segment);
-  } catch {
-    return segment;
-  }
-}
-
 function urlToRoute(url: URL): Route | null {
   // Normalize a trailing slash (`/guides/protocol/` → `/guides/protocol`) so
   // both the strict marketing resolver and the lenient repo resolver agree.
@@ -454,13 +444,17 @@ function urlToRoute(url: URL): Route | null {
         if (id === "users") {
           const did = segments.shift();
           if (did) {
-            return { resource: "users", baseUrl, did: decodeIdentifier(did) };
+            return {
+              resource: "users",
+              baseUrl,
+              did: utils.safeDecodeURIComponent(did),
+            };
           }
           return null;
         } else if (id) {
           return resolveRepoRoute(
             baseUrl,
-            decodeIdentifier(id),
+            utils.safeDecodeURIComponent(id),
             segments,
             url.search,
           );

src/lib/utils.ts

@@ -305,6 +305,17 @@ export function formatObjectId(id: string): string {
   return id.substring(0, 7);
 }
 
+// Percent-decoding throws on a stray `%`, which branch names, tags and repo
+// aliases are all free to contain, so a segment that is not a well-formed
+// encoding is returned untouched rather than taking down whatever renders it.
+export function safeDecodeURIComponent(segment: string): string {
+  try {
+    return decodeURIComponent(segment);
+  } catch {
+    return segment;
+  }
+}
+
 export function formatQualifiedRefname(
   refname: string,
   peer: string | undefined,

src/views/repos/History.svelte

@@ -10,7 +10,11 @@
 
   import config from "@app/lib/config";
   import { HttpdClient } from "@http-client";
-  import { baseUrlToString, formatQualifiedRefname } from "@app/lib/utils";
+  import {
+    baseUrlToString,
+    formatQualifiedRefname,
+    safeDecodeURIComponent,
+  } from "@app/lib/utils";
   import { groupCommits } from "@app/lib/commit";
 
   import Button from "@app/components/Button.svelte";
@@ -37,8 +41,13 @@
   export let nodeId: string;
   export let nodeAvatarUrl: string | undefined;
 
+  // The revision carries the encoded branch name after an in-app navigation,
+  // and this refname reaches the user as part of the clone popover's archive
+  // command.
   $: currentRefname = formatQualifiedRefname(
-    revision || repo.payloads["xyz.radicle.project"].data.defaultBranch,
+    revision
+      ? safeDecodeURIComponent(revision)
+      : repo.payloads["xyz.radicle.project"].data.defaultBranch,
     peer,
   );
 

src/views/repos/Source.svelte

@@ -4,7 +4,10 @@
 
   import { HttpdClient } from "@http-client";
 
-  import { formatQualifiedRefname } from "@app/lib/utils";
+  import {
+    formatQualifiedRefname,
+    safeDecodeURIComponent,
+  } from "@app/lib/utils";
 
   import BlobComponent from "./Source/Blob.svelte";
   import Button from "@app/components/Button.svelte";
@@ -48,8 +51,13 @@
     });
   };
 
+  // The revision carries the encoded branch name after an in-app navigation,
+  // and this refname reaches the user as part of the clone popover's archive
+  // command.
   $: currentRefname = formatQualifiedRefname(
-    revision || repo.payloads["xyz.radicle.project"].data.defaultBranch,
+    revision
+      ? safeDecodeURIComponent(revision)
+      : repo.payloads["xyz.radicle.project"].data.defaultBranch,
     peer,
   );
 

src/views/repos/Source/PeerBranchSelector.svelte

@@ -12,6 +12,7 @@
     getBranchesFromRefs,
     getTagsFromRefs,
     gravatarURL,
+    safeDecodeURIComponent,
   } from "@app/lib/utils";
 
   import Badge from "@app/components/Badge.svelte";
@@ -207,6 +208,14 @@
   // rather than claiming there are none.
   $: tagsTabDisabled = !hasTags && peers !== undefined;
 
+  // Links to a branch encode its name so a slash cannot be mistaken for a path
+  // separator, which leaves the encoded form in the route after an in-app
+  // navigation. Only the label is decoded: the comparisons above match against
+  // both forms and rely on the route value as it stands.
+  $: selectedBranchLabel = selectedBranch
+    ? safeDecodeURIComponent(selectedBranch)
+    : undefined;
+
   $: isSelectedBranchCanonical = (() => {
     if (onCanonical) return true;
     if (!selectedBranch || peer) return false;
@@ -368,7 +377,7 @@
       {:else if selectedBranch}
         <Icon name="branch" />
         <span class="txt-overflow">
-          {selectedBranch}
+          {selectedBranchLabel}
         </span>
         {#if isSelectedBranchCanonical}
           <Badge title="Canonical branch" variant="foreground-emphasized">

tests/e2e/repo.spec.ts

@@ -438,6 +438,59 @@ test("peer and branch switching", async ({ page }) => {
   }
 });
 
+// Links to a canonical branch encode its name so a slash cannot be mistaken
+// for a path separator, which leaves the encoded form in the route after an
+// in-app navigation. The label has to show the branch name, not the encoding.
+// Only the canonical list encodes: the peer list covered above passes branch
+// names through untouched, so the branch has to be canonical to reproduce it.
+test("canonical branch with a slash in the name", async ({ page, peer }) => {
+  const { rid, repoFolder } = await createRepo(peer, {
+    name: "slash-branch",
+  });
+
+  // A branch is only resolved by delegate quorum when a canonical-ref rule
+  // matches it. Without this the branch is listed under the peer instead,
+  // where names are never encoded.
+  await peer.rad(
+    [
+      "id",
+      "update",
+      "--title",
+      "Add canonical ref rule",
+      "--description",
+      "",
+      "--payload",
+      "xyz.radicle.crefs",
+      "rules",
+      JSON.stringify({
+        "refs/heads/feature/*": {
+          allow: [`did:key:${peer.nodeId}`],
+          threshold: 1,
+        },
+      }),
+      "--no-confirm",
+    ],
+    { cwd: repoFolder },
+  );
+
+  await peer.git(["checkout", "-b", "feature/branch"], { cwd: repoFolder });
+  await peer.git(
+    ["commit", "--allow-empty", "--message", "branch off the default"],
+    { cwd: repoFolder },
+  );
+  await peer.git(["push", "rad", "feature/branch"], { cwd: repoFolder });
+
+  await page.goto(peer.ridUrl(rid));
+  const trigger = page.locator('[title="Change branch"]:visible').first();
+  await trigger.click();
+  await page.getByRole("button", { name: /feature\/branch Canonical/ }).click();
+
+  await expect(trigger).toHaveText(/feature\/branch/);
+  await expect(trigger).not.toHaveText(/%2F/);
+  // The URL keeps the encoded form; only what the user reads is decoded.
+  await expect(page).toHaveURL(/tree\/feature%2Fbranch/);
+});
+
 test("only one modal can be open at a time", async ({ page }) => {
   await page.goto(sourceBrowsingUrl);
 

tests/unit/utils.test.ts

@@ -352,3 +352,26 @@ describe("repoSegment", () => {
     expect(utils.repoSegment(repo)).toEqual(rid);
   });
 });
+
+describe("safeDecodeURIComponent", () => {
+  test("decodes an encoded branch name", () => {
+    expect(utils.safeDecodeURIComponent("releaseprep%2Fv27.1")).toEqual(
+      "releaseprep/v27.1",
+    );
+  });
+
+  test("leaves an already decoded name untouched", () => {
+    expect(utils.safeDecodeURIComponent("releaseprep/v27.1")).toEqual(
+      "releaseprep/v27.1",
+    );
+  });
+
+  // Branch names, tags and repo aliases may all contain a bare `%`, which is
+  // not a well-formed encoding and would otherwise throw.
+  test("returns an undecodable segment unchanged", () => {
+    expect(utils.safeDecodeURIComponent("100%")).toEqual("100%");
+    expect(utils.safeDecodeURIComponent("wip/50%-done")).toEqual(
+      "wip/50%-done",
+    );
+  });
+});