Merge branch 'feat/do-links-verify' into feat/desktop-offline

components/calendar/calendar-app.tsx

@@ -78,7 +78,6 @@ import { cn, generateUUID } from "@/lib/utils";
 import type { Calendar, CalendarEvent, CalendarParticipant, CalendarRights, CalendarTask } from "@/lib/jmap/types";
 import { ShareCollectionDialog } from "@/components/settings/share-collection-dialog";
 import { ConfirmDialog } from "@/components/ui/confirm-dialog";
-import { Dialog } from "@/components/ui/dialog";
 import { useConfirmDialog } from "@/hooks/use-confirm-dialog";
 import { SchedulingDeniedError } from "@/lib/jmap/scheduling-error";
 import { CreateCalendarModal } from "@/components/calendar/create-calendar-modal";
@@ -166,7 +165,6 @@ export function CalendarApp({ linkSegments: routeSegments }: CalendarAppProps =
   useDocumentTitle(tSidebar("calendar"));
   useDesktopWindowName(tSidebar("calendar"), { withAccount: true });
   const skeletonLabels = useSkeletonLabels();
-  const tWebcalAction = useTranslations("calendar.webcal_action");
   const { demoMode } = useConfig();
   // Subscriptions fetch the feed through /api/fetch-ical, which the demo has no
   // mail server for and the Lite static build does not have at all.
@@ -250,7 +248,6 @@ export function CalendarApp({ linkSegments: routeSegments }: CalendarAppProps =
   const searchInputRef = useRef<HTMLInputElement>(null);
   const [showSubscriptionModal, setShowSubscriptionModal] = useState(false);
   const [pendingSubscription, setPendingSubscription] = useState<{ url: string; name: string } | null>(null);
-  const [showWebcalActionChoice, setShowWebcalActionChoice] = useState(false);
   const [pendingWebcalAccountChoice, setPendingWebcalAccountChoice] = useState<ParsedWebcal | null>(null);
   const [isProtocolAccountSwitching, setIsProtocolAccountSwitching] = useState(false);
   const [editingSubscription, setEditingSubscription] = useState<string | null>(null);
@@ -354,10 +351,10 @@ export function CalendarApp({ linkSegments: routeSegments }: CalendarAppProps =
     } else if (client && !calendarEnabled) {
       // Calendar disabled by admin policy - send the user back to mail.
       router.push("/");
-    } else if (client && !supportsCalendar && !pendingWebcalAccountChoice && !isProtocolAccountSwitching && !pendingSubscription && !showWebcalActionChoice && !hasPendingWebcal()) {
+    } else if (client && !supportsCalendar && !pendingWebcalAccountChoice && !isProtocolAccountSwitching && !pendingSubscription && !hasPendingWebcal()) {
       router.push("/");
     }
-  }, [initialCheckDone, isAuthenticated, authLoading, client, calendarEnabled, supportsCalendar, pendingWebcalAccountChoice, isProtocolAccountSwitching, pendingSubscription, showWebcalActionChoice, router]);
+  }, [initialCheckDone, isAuthenticated, authLoading, client, calendarEnabled, supportsCalendar, pendingWebcalAccountChoice, isProtocolAccountSwitching, pendingSubscription, router]);
 
   useEffect(() => {
     if (error) {
@@ -373,6 +370,17 @@ export function CalendarApp({ linkSegments: routeSegments }: CalendarAppProps =
     });
   }, []);
 
+  // A webcal: link asks to subscribe, so that dialog opens with the address and name filled
+  // in and is the one question. Where subscribing isn't offered (Lite, demo) it's the import.
+  const openWebcalDialog = useCallback((pending: ParsedWebcal) => {
+    setPendingSubscription({
+      url: pending.subscriptionUrl,
+      name: pending.suggestedName,
+    });
+    if (canSubscribe) setShowSubscriptionModal(true);
+    else setShowImportModal(true);
+  }, [canSubscribe]);
+
   const openWebcalForAccount = useCallback(async (pending: ParsedWebcal, accountId: string) => {
     setIsProtocolAccountSwitching(true);
     try {
@@ -380,15 +388,11 @@ export function CalendarApp({ linkSegments: routeSegments }: CalendarAppProps =
         await switchAccount(accountId);
       }
       setPendingWebcalAccountChoice(null);
-      setPendingSubscription({
-        url: pending.subscriptionUrl,
-        name: pending.suggestedName,
-      });
-      setShowWebcalActionChoice(true);
+      openWebcalDialog(pending);
     } finally {
       setIsProtocolAccountSwitching(false);
     }
-  }, [switchAccount]);
+  }, [switchAccount, openWebcalDialog]);
 
   const handleWebcalProtocolRequest = useCallback((pending: ParsedWebcal) => {
     const protocolAccounts = getWebcalProtocolAccounts();
@@ -407,27 +411,8 @@ export function CalendarApp({ linkSegments: routeSegments }: CalendarAppProps =
       return;
     }
 
-    setPendingSubscription({
-      url: pending.subscriptionUrl,
-      name: pending.suggestedName,
-    });
-    setShowWebcalActionChoice(true);
-  }, [activeAccountId, getWebcalProtocolAccounts, openWebcalForAccount, supportsCalendar]);
-
-  const closeWebcalActionChoice = useCallback(() => {
-    setShowWebcalActionChoice(false);
-    setPendingSubscription(null);
-  }, []);
-
-  const handleImportWebcal = useCallback(() => {
-    setShowWebcalActionChoice(false);
-    setShowImportModal(true);
-  }, []);
-
-  const handleSubscribeWebcal = useCallback(() => {
-    setShowWebcalActionChoice(false);
-    setShowSubscriptionModal(true);
-  }, []);
+    openWebcalDialog(pending);
+  }, [activeAccountId, getWebcalProtocolAccounts, openWebcalForAccount, openWebcalDialog, supportsCalendar]);
 
   useEffect(() => {
     if (!isAuthenticated || !client) return;
@@ -1943,33 +1928,6 @@ export function CalendarApp({ linkSegments: routeSegments }: CalendarAppProps =
     />
   ) : null;
 
-  const renderWebcalActionChoice = () => showWebcalActionChoice && pendingSubscription ? (
-    <Dialog
-      isOpen
-      onClose={closeWebcalActionChoice}
-      title={tWebcalAction("title")}
-      description={tWebcalAction("description", { name: pendingSubscription.name })}
-      footer={<Button variant="outline" onClick={closeWebcalActionChoice}>{tWebcalAction("cancel")}</Button>}
-    >
-        <div className="pt-4 space-y-3">
-          <Button variant="outline" className="w-full justify-start h-auto py-3" onClick={handleImportWebcal}>
-            <span className="text-start">
-              <span className="block font-medium">{tWebcalAction("import_title")}</span>
-              <span className="block text-xs text-muted-foreground mt-0.5">{tWebcalAction("import_description")}</span>
-            </span>
-          </Button>
-          {!IS_LITE && (
-            <Button variant="outline" className="w-full justify-start h-auto py-3" onClick={handleSubscribeWebcal}>
-              <span className="text-start">
-                <span className="block font-medium">{tWebcalAction("subscribe_title")}</span>
-                <span className="block text-xs text-muted-foreground mt-0.5">{tWebcalAction("subscribe_description")}</span>
-              </span>
-            </Button>
-          )}
-        </div>
-    </Dialog>
-  ) : null;
-
   // Until the session says what the account has, the page is drawn as the
   // skeleton the server sent (components/boot-skeleton): the same markup, so
   // nothing moves while the sign-in is restored or the redirects decide.
@@ -2729,7 +2687,6 @@ export function CalendarApp({ linkSegments: routeSegments }: CalendarAppProps =
 
       <SidebarAppsModal isOpen={showAppsModal} onClose={closeAppsModal} />
       {renderWebcalAccountPicker()}
-      {renderWebcalActionChoice()}
       <RecurrenceScopeDialog
         isOpen={!!pendingScopeAction}
         actionType={pendingScopeAction?.type || "edit"}

integration/tests/26-do-links.spec.ts

@@ -0,0 +1,161 @@
+import { test, expect, type Browser, type Page } from '@playwright/test';
+import { accountSwitcher, neutralizeDevOverlay } from './helpers/app';
+import { ENT, ENT_PASSWORD, contextOptions, jmapAs, requireEntEnv, shot } from './helpers/ent';
+
+/**
+ * The browser webmail as the mail and calendar app of the browser: Settings registers
+ * navigator.registerProtocolHandler for mailto: and webcal:, and the address the browser
+ * then opens fills the composer or the subscription.
+ *
+ * Chromium can't accept its own permission prompt from a test, so a stand-in records what the
+ * page registers, and the spec opens that address the way the browser would, %s replaced by the
+ * encoded link. A second context checks the real call is accepted without an error.
+ *
+ *   BUILD=1 bash desktop-offline-rig/start-prod.sh <worktree> 3241 <jmap-url> links-1.0
+ *   bash desktop-offline-rig/run-spec.sh <worktree> 3241 <jmap-url> 1.0 integration/tests/26-do-links.spec.ts
+ */
+
+test.describe.configure({ mode: 'serial' });
+test.setTimeout(180_000);
+
+const RUN = `lkb${Date.now().toString(36)}`;
+const composer = (page: Page) => page.locator('[data-testid="email-composer"]').first();
+
+/** Records navigator.registerProtocolHandler instead of asking Chromium. */
+function recordRegistrations() {
+  const calls: [string, string][] = [];
+  (window as unknown as { __registered: [string, string][] }).__registered = calls;
+  Object.defineProperty(Navigator.prototype, 'registerProtocolHandler', {
+    configurable: true,
+    value(scheme: string, url: string) {
+      calls.push([scheme, url]);
+    },
+  });
+}
+
+async function signedIn(browser: Browser, record: boolean): Promise<Page> {
+  const context = await browser.newContext(contextOptions());
+  if (record) await context.addInitScript(recordRegistrations);
+  const page = await context.newPage();
+  await neutralizeDevOverlay(page);
+  await page.goto('/');
+  await page.fill('#username', ENT.alice);
+  await page.fill('#password', ENT_PASSWORD);
+  await page.click('button[type="submit"]');
+  await accountSwitcher(page).waitFor({ state: 'visible', timeout: 60_000 });
+  return page;
+}
+
+async function openDefaultApps(page: Page): Promise<void> {
+  await page.goto('/settings/general');
+  await expect(page.getByRole('heading', { name: 'Default apps' })).toBeVisible({ timeout: 60_000 });
+}
+
+/**
+ * The tab the composer or calendar ended up in. With "Always open a new tab" a handler tab that
+ * has history of its own hands the link to a fresh tab and closes, so it may not be this one.
+ */
+async function tabWith(page: Page, selector: string): Promise<Page> {
+  let found: Page | undefined;
+  await expect.poll(async () => {
+    for (const candidate of page.context().pages()) {
+      if (!candidate.isClosed() && await candidate.locator(selector).first().isVisible().catch(() => false)) {
+        found = candidate;
+        return true;
+      }
+    }
+    return false;
+  }, { timeout: 60_000 }).toBe(true);
+  return found!;
+}
+
+/** The address the browser opens for a link, as Chromium fills in %s. */
+const handlerAddress = (template: string, link: string) => template.replace('%s', encodeURIComponent(link));
+
+let mailtoTemplate = '';
+let webcalTemplate = '';
+
+test.beforeAll(requireEntEnv);
+
+test.afterAll(async () => {
+  // Only this run's: a draft a composer may have saved.
+  const client = await jmapAs(ENT.alice);
+  const found = await client.request([['Email/query', { accountId: client.accountId, filter: { subject: RUN }, limit: 50 }, 'q']]);
+  const ids = (found.methodResponses[0][1] as { ids: string[] }).ids;
+  if (ids.length) await client.request([['Email/set', { accountId: client.accountId, destroy: ids }, 'd']]);
+});
+
+test('Settings registers both handlers with this app\'s own address', async ({ browser }) => {
+  const page = await signedIn(browser, true);
+  await openDefaultApps(page);
+  await page.getByRole('button', { name: /for email links/ }).click();
+  await expect(page.getByText('Email handler registration requested')).toBeVisible();
+  await page.getByRole('button', { name: /for calendar links/ }).click();
+  await expect(page.getByText('Calendar handler registration requested')).toBeVisible();
+  const origin = new URL(page.url()).origin;
+  const calls = await page.evaluate(() => (window as unknown as { __registered: [string, string][] }).__registered);
+  expect(calls).toEqual([
+    ['mailto', `${origin}/protocol/mailto?url=%s`],
+    ['webcal', `${origin}/protocol/webcal?url=%s`],
+  ]);
+  [, mailtoTemplate] = calls[0];
+  [, webcalTemplate] = calls[1];
+  await shot(page, 'links-browser-default-apps');
+  await page.context().close();
+});
+
+test('the real browser call takes the address without an error', async ({ browser }) => {
+  const page = await signedIn(browser, false);
+  await openDefaultApps(page);
+  await page.getByRole('button', { name: /for email links/ }).click();
+  await expect(page.getByText('Email handler registration requested')).toBeVisible();
+  await expect(page.getByText('Protocol handler registration failed')).toHaveCount(0);
+  await page.context().close();
+});
+
+test('a mail link opened by the browser fills the composer: every field, UTF-8 and line breaks', async ({ browser }) => {
+  const page = await signedIn(browser, false);
+  // The browser opens the handler in a tab of its own.
+  const tab = await page.context().newPage();
+  const subject = `${RUN} Grüße 😀`;
+  const link = `mailto:J%C3%BCrgen%20%3Cjuergen%40example.org%3E,second%40example.org?cc=cc%40example.org&bcc=bcc%40example.org&subject=${encodeURIComponent(subject)}&body=${encodeURIComponent('Zeile 1\r\nZeile 2\r\n€ 10 & more')}`;
+  await tab.goto(handlerAddress(mailtoTemplate, link));
+  const shown = await tabWith(page, '[data-testid="email-composer"]');
+  expect(shown).not.toBe(page);
+  await expect(shown.getByTestId('composer-subject')).toHaveValue(subject);
+  const to = await shown.locator('[data-testid="composer-to"] [data-recipient-chip]').allTextContents();
+  expect(to.join(' ')).toContain('Jürgen');
+  expect(to.join(' ')).toContain('second@example.org');
+  await expect(composer(shown)).toContainText('cc@example.org');
+  await expect(composer(shown)).toContainText('bcc@example.org');
+  const body = composer(shown).locator('[contenteditable="true"], textarea').last();
+  await expect(body).toContainText('Zeile 1');
+  await expect(body).toContainText('Zeile 2');
+  await expect(body).toContainText('€ 10 & more');
+  await shot(shown, 'links-browser-mailto');
+  await page.context().close();
+});
+
+test('a link with a stray percent sign still opens the composer', async ({ browser }) => {
+  const page = await signedIn(browser, false);
+  const tab = await page.context().newPage();
+  await tab.goto(handlerAddress(mailtoTemplate, `mailto:someone@example.org?subject=${RUN}%2050% off`));
+  const shown = await tabWith(page, '[data-testid="composer-subject"]');
+  await expect(shown.getByTestId('composer-subject')).toHaveValue(`${RUN} 50% off`);
+  await page.context().close();
+});
+
+test('a calendar link opened by the browser goes to the subscription, filled in', async ({ browser }) => {
+  const page = await signedIn(browser, false);
+  const tab = await page.context().newPage();
+  await tab.goto(handlerAddress(webcalTemplate, `webcal://feeds.example.org/${RUN}.ics`));
+  const shown = await tabWith(page, '[role="dialog"]');
+  await expect(shown).toHaveURL(/\/calendar/);
+  const dialog = shown.getByRole('dialog', { name: 'Subscribe to a calendar' });
+  await expect(dialog).toBeVisible({ timeout: 60_000 });
+  await expect(shown.getByRole('dialog', { name: 'Open calendar link' })).toHaveCount(0);
+  await expect(dialog.getByRole('textbox').first()).toHaveValue(`https://feeds.example.org/${RUN}.ics`);
+  await expect(dialog.getByRole('textbox').nth(1)).toHaveValue(RUN);
+  await shot(shown, 'links-browser-webcal');
+  await page.context().close();
+});

lib/__tests__/protocol-handlers.test.ts

@@ -97,6 +97,26 @@ describe("protocol handlers", () => {
       });
     });
 
+    it("keeps a link with a stray percent sign, decoding what can be decoded", () => {
+      // Links written by hand or by older programs: Outlook and Thunderbird open these.
+      expect(parseMailto("mailto:alice@example.com?subject=50% off&body=Caf%C3%A9 %ZZ %C3")).toMatchObject({
+        to: ["alice@example.com"],
+        subject: "50% off",
+        body: "Café %ZZ �",
+      });
+      expect(parseMailto("mailto:bob%40example.com,100%@example.com")?.to).toEqual(["bob@example.com", "100%@example.com"]);
+    });
+
+    it("decodes UTF-8 and CRLF line breaks in every field", () => {
+      expect(parseMailto("mailto:J%C3%BCrgen%20%3Cj%40example.com%3E?cc=a%40example.com,b%40example.com&subject=Gr%C3%BC%C3%9Fe%20%F0%9F%98%80&body=Zeile%201%0D%0AZeile%202%0D%0A%0D%0A%E2%82%AC%2610")).toEqual({
+        to: ["Jürgen <j@example.com>"],
+        cc: ["a@example.com", "b@example.com"],
+        bcc: [],
+        subject: "Grüße 😀",
+        body: "Zeile 1\nZeile 2\n\n€&10",
+      });
+    });
+
     it("removes control characters and caps recipients", () => {
       const recipients = Array.from({ length: 250 }, (_, index) => `user${index}@example.com`).join(",");
       const parsed = parseMailto(`mailto:${recipients}?subject=Hi%0ABcc:evil@example.com`);

lib/protocol-handlers/mailto.ts

@@ -78,25 +78,29 @@ function getQueryValue(searchParams: QueryParam[], key: string): string {
   return values.join(",");
 }
 
-function decodePathname(pathname: string): string | null {
+/**
+ * Percent-decoding that never gives up on a link. Links written by hand or by older
+ * programs carry a stray "%" ("50% off") or a cut-off UTF-8 sequence; Outlook and
+ * Thunderbird open those, so each run of valid escapes is decoded and anything else
+ * stays as it is. Bytes that aren't UTF-8 become U+FFFD. RFC 6068 uses
+ * percent-encoding, not form encoding: a literal '+' stays a '+'.
+ */
+function decodeLenient(value: string): string {
   try {
-    return decodeURIComponent(pathname || "");
-  } catch {
-    return null;
-  }
-}
-
-function decodeQueryPart(value: string): string | null {
-  try {
-    // RFC 6068 uses percent-encoding for mailto query fields; unlike form
-    // encoding, a literal '+' is part of the value and must not become space.
     return decodeURIComponent(value);
   } catch {
-    return null;
+    return value.replace(/(?:%[0-9a-f]{2})+/gi, (run) => {
+      try {
+        return decodeURIComponent(run);
+      } catch {
+        const bytes = Uint8Array.from(run.slice(1).split("%"), (hex) => parseInt(hex, 16));
+        return new TextDecoder().decode(bytes);
+      }
+    });
   }
 }
 
-function parseQuery(query: string): QueryParam[] | null {
+function parseQuery(query: string): QueryParam[] {
   if (!query) return [];
 
   const params: QueryParam[] = [];
@@ -105,10 +109,7 @@ function parseQuery(query: string): QueryParam[] | null {
     const separatorIndex = part.indexOf("=");
     const rawKey = separatorIndex >= 0 ? part.slice(0, separatorIndex) : part;
     const rawValue = separatorIndex >= 0 ? part.slice(separatorIndex + 1) : "";
-    const key = decodeQueryPart(rawKey);
-    const value = decodeQueryPart(rawValue);
-    if (key === null || value === null) return null;
-    params.push({ key, value });
+    params.push({ key: decodeLenient(rawKey), value: decodeLenient(rawValue) });
   }
 
   return params;
@@ -122,10 +123,8 @@ export function parseMailto(raw: string): ParsedMailto | null {
   const rawPathname = queryIndex >= 0 ? addressAndQuery.slice(0, queryIndex) : addressAndQuery;
   const rawQuery = queryIndex >= 0 ? addressAndQuery.slice(queryIndex + 1) : "";
 
-  const decodedPathname = decodePathname(rawPathname);
-  if (decodedPathname === null) return null;
+  const decodedPathname = decodeLenient(rawPathname);
   const searchParams = parseQuery(rawQuery);
-  if (searchParams === null) return null;
 
   const to = [
     ...splitRecipients(decodedPathname),