diff --git a/web/src/components/community/PostBody.test.tsx b/web/src/components/community/PostBody.test.tsx
new file mode 100644
index 000000000..8091ccf08
--- /dev/null
+++ b/web/src/components/community/PostBody.test.tsx
@@ -0,0 +1,93 @@
+// The links PostBody draws around a comment. The URL rule itself is
+// lib/community/markdown.test.ts.
+//
+// Until 26 October every link opened a new tab, so a footnote opened the
+// whole page again to show one line of it. And footnote ids come from the
+// footnote's label: two comments on one page that both wrote [^1] both had
+// id="user-content-fn-1" and id="footnote-label", and the second comment's
+// footnote link landed on the first comment's note.
+
+import { render } from "@testing-library/react";
+import { describe, expect, it } from "vitest";
+import PostBody from "./PostBody";
+
+const PAGE = "https://patternflow.work/community/p/abc";
+const FOOTNOTE = "Measured at 5 V.[^1]\n\n[^1]: With the 3 A bank.";
+
+const link = (root: HTMLElement, name: string) => {
+ const found = [...root.querySelectorAll("a")].find((a) => a.textContent === name);
+ if (!found) throw new Error(`no link "${name}" in ${root.innerHTML}`);
+ return found;
+};
+
+/** The element a same-page link jumps to, looked up the way the browser does. */
+const landing = (a: HTMLAnchorElement) =>
+ document.getElementById(decodeURIComponent(a.getAttribute("href")!.slice(1)));
+
+describe("PostBody links", () => {
+ it("opens a link off the site in a new tab", () => {
+ const { container } = render();
+ const a = link(container, "datasheet");
+ expect(a.getAttribute("target")).toBe("_blank");
+ expect(a.getAttribute("rel")).toBe("noopener noreferrer nofollow");
+ });
+
+ it("keeps a link to another page of the site in this tab", () => {
+ const { container } = render();
+ const a = link(container, "my deck");
+ expect(a.getAttribute("href")).toBe("/community/d/abc");
+ expect(a.hasAttribute("target")).toBe(false);
+ });
+
+ it("keeps a footnote in this tab and lands on the note and back", () => {
+ const { container } = render();
+ const ref = link(container, "1");
+ expect(ref.hasAttribute("target")).toBe(false);
+ expect(landing(ref)?.textContent).toContain("With the 3 A bank.");
+
+ const back = link(container, "↩");
+ expect(back.hasAttribute("target")).toBe(false);
+ expect(back.getAttribute("aria-label")).toBe("Back to reference 1");
+ expect(landing(back)).toBe(ref);
+ });
+
+ it("gives two comments with the same footnote their own ids", () => {
+ const { container } = render(
+ <>
+
+
+ ,
+ );
+ const ids = [...document.querySelectorAll("[id]")].map((el) => el.id);
+ expect(ids.length).toBeGreaterThan(0);
+ expect(new Set(ids).size).toBe(ids.length);
+
+ // Each comment's footnote link lands inside that comment, and the
+ // reference is described by that comment's own "Footnotes" heading.
+ for (const body of container.querySelectorAll(":scope > div")) {
+ const ref = link(body as HTMLElement, "1");
+ expect(body.contains(landing(ref))).toBe(true);
+ const label = document.getElementById(ref.getAttribute("aria-describedby")!);
+ expect(label?.textContent).toBe("Footnotes");
+ expect(body.contains(label)).toBe(true);
+ }
+ });
+
+ // micromark writes a backslash in a link as %5C before safeHref sees it, so
+ // these never reached the rule. markdown.test.ts holds the rule to them
+ // anyway, in case that ever changes.
+ it.each(["[x](/\\evil.example)", "[x](\\\\evil.example)", "[x](\\/evil.example)"])(
+ "puts %j on this site",
+ (text) => {
+ const { container } = render();
+ const href = container.querySelector("a")!.getAttribute("href")!;
+ expect(new URL(href, PAGE).origin).toBe(new URL(PAGE).origin);
+ },
+ );
+
+ it("writes a javascript: link as plain text", () => {
+ const { container } = render();
+ expect(container.querySelector("a")).toBeNull();
+ expect(container.textContent).toBe("click");
+ });
+});
diff --git a/web/src/components/community/PostBody.tsx b/web/src/components/community/PostBody.tsx
index b8704e5f5..b066596fc 100644
--- a/web/src/components/community/PostBody.tsx
+++ b/web/src/components/community/PostBody.tsx
@@ -1,8 +1,9 @@
"use client";
+import { useId } from "react";
import ReactMarkdown from "react-markdown";
import remarkGfm from "remark-gfm";
-import { safeHref } from "@/lib/community/markdown";
+import { opensHere, safeHref } from "@/lib/community/markdown";
import styles from "./Community.module.css";
// What a comment is written in. The workshop's threads were too, until it came
@@ -25,8 +26,9 @@ import styles from "./Community.module.css";
// a picture had was a workshop attachment, sniffed by magic bytes and
// served same-origin, and it went with the workshop.
//
-// 3. http(s) links only, and never same-tab. safeHref drops anything else, so
-// a `javascript:` or `data:` href renders as inert text.
+// 3. http(s) links only. safeHref drops anything else, so a `javascript:` or
+// `data:` href renders as inert text. A link off the site opens a new tab.
+// A footnote, an anchor or a path on this site stays in this one.
/** Elements react-markdown may produce. Anything not here is dropped, so a new
* remark plugin cannot quietly widen what a post can render. */
@@ -43,6 +45,11 @@ const ALLOWED = new Set([
]);
export default function PostBody({ text }: { text: string }) {
+ // A footnote's ids come from its label, so two comments on one page that
+ // both write [^1] would both hold id="user-content-fn-1", and the second
+ // one's link would land on the first one's note. Each body gets its own
+ // prefix. useId gives the server and the browser the same one.
+ const prefix = `user-content-${useId()}-`;
return (
{
+ a: ({
+ href,
+ children,
+ id,
+ "aria-label": label,
+ "aria-describedby": describedBy,
+ }) => {
const safe = safeHref(href);
// Not a link we will follow: render what they typed, as text.
if (!safe) return <>{children};
+ if (opensHere(safe)) {
+ // A footnote and its way back carry an id and a label. The
+ // "Footnotes" heading that describes them takes the prefix below.
+ return (
+
+ {children}
+
+ );
+ }
return (
);
},
+ // The one heading with an id is the footnotes' own, which
+ // clobberPrefix leaves as plain "footnote-label".
+ h2: ({ id, className, children }) => (
+
+ {children}
+
+ ),
pre: ({ children }) => {children},
code: ({ children, className }) =>
// react-markdown hands fenced code a language class and inline code
diff --git a/web/src/lib/community/markdown.test.ts b/web/src/lib/community/markdown.test.ts
new file mode 100644
index 000000000..7d9d7d97a
--- /dev/null
+++ b/web/src/lib/community/markdown.test.ts
@@ -0,0 +1,83 @@
+// safeHref is the rule for every link a member writes, and its header said a
+// smoke test held it to that. No test called it. Each case below is also put
+// through the URL parser a browser uses, so a kept link is shown to land
+// where the rule says it does, not only to match a string.
+
+import { describe, expect, it } from "vitest";
+import { opensHere, safeHref } from "./markdown";
+
+const PAGE = "https://patternflow.work/community/p/abc";
+const SITE = new URL(PAGE).origin;
+
+describe("safeHref", () => {
+ it.each([
+ ["javascript:", "javascript:alert(1)"],
+ ["a capital scheme", "JavaScript:alert(1)"],
+ ["a tab inside the scheme", "java\tscript:alert(1)"],
+ ["a NUL inside the scheme", "java\u0000script:alert(1)"],
+ ["a leading space", " javascript:alert(1)"],
+ ["data:", "data:text/html,hi"],
+ ["vbscript:", "vbscript:msgbox(1)"],
+ ["file:", "file:///etc/passwd"],
+ ["mailto:", "mailto:ada@example.com"],
+ ["a protocol-relative link", "//evil.example"],
+ ["a tab between the slashes", "/\t/evil.example"],
+ ["slash backslash", "/\\evil.example"],
+ ["two backslashes", "\\\\evil.example"],
+ ["backslash slash", "\\/evil.example"],
+ ["nothing", ""],
+ ])("refuses %s", (_, raw) => {
+ expect(safeHref(raw)).toBeNull();
+ });
+
+ it.each([
+ "https://patternflow.work/community",
+ "HTTP://example.com/x",
+ "/community/d/abc",
+ "#user-content-fn-1",
+ "?sort=new",
+ "example.com/x",
+ "notes/a:b",
+ ])("keeps %j, and the browser takes it to http(s)", (raw) => {
+ expect(safeHref(raw)).toBe(raw);
+ const landed = new URL(raw, PAGE);
+ expect(["http:", "https:"]).toContain(landed.protocol);
+ // No scheme of its own means it stays on the page's site.
+ if (!/^https?:/i.test(raw)) expect(landed.origin).toBe(SITE);
+ });
+
+ it("refuses what is not a string", () => {
+ expect(safeHref(undefined)).toBeNull();
+ expect(safeHref(null)).toBeNull();
+ });
+
+ // Why the three backslash cases are refused: a browser reads each of them
+ // as //evil.example, which is another site on the page's scheme.
+ it.each(["/\\evil.example", "\\\\evil.example", "\\/evil.example"])(
+ "%j is another site to a browser",
+ (raw) => {
+ expect(new URL(raw, PAGE).href).toBe("https://evil.example/");
+ },
+ );
+});
+
+describe("opensHere", () => {
+ it.each(["#user-content-fn-1", "/community/d/abc", "?sort=new"])(
+ "keeps %j in this tab",
+ (href) => {
+ expect(opensHere(href)).toBe(true);
+ },
+ );
+
+ it.each([
+ "https://example.com/",
+ "https://patternflow.work/community",
+ // A browser puts this on the page's own site, but whoever wrote it meant
+ // the site of that name.
+ "example.com/x",
+ "//evil.example",
+ "/\\evil.example",
+ ])("opens %j in a new tab", (href) => {
+ expect(opensHere(href)).toBe(false);
+ });
+});
diff --git a/web/src/lib/community/markdown.ts b/web/src/lib/community/markdown.ts
index 9e35bf9ea..0944c9434 100644
--- a/web/src/lib/community/markdown.ts
+++ b/web/src/lib/community/markdown.ts
@@ -2,13 +2,19 @@
//
// react-markdown has its own urlTransform and it is not bad, but "the library
// probably strips that" is not a thing to find out from a bug report. This is
-// the rule, it is asserted in the smoke test, and PostBody calls it.
+// the rule, markdown.test.ts holds it to each case, and PostBody calls it.
/** Characters a browser skips while parsing a scheme. A NUL or a tab inside
* `java\tscript:` is the classic way past a naive startsWith check, so they
* come out before anything is compared. */
const IGNORED = /[\u0000-\u0020]/g;
+/** A browser reads a backslash in an http(s) URL as a slash, so `/\host`,
+ * `\\host` and `\/host` all go to `//host`. The rule reads it the same way. */
+const BACKSLASH = /\\/g;
+
+const probeOf = (raw: string) => raw.replace(IGNORED, "").replace(BACKSLASH, "/").toLowerCase();
+
/**
* The href to actually render, or null to render the link as plain text.
*
@@ -21,7 +27,7 @@ const IGNORED = /[\u0000-\u0020]/g;
export function safeHref(raw: string | undefined | null): string | null {
if (typeof raw !== "string") return null;
- const probe = raw.replace(IGNORED, "").toLowerCase();
+ const probe = probeOf(raw);
if (probe.length === 0) return null;
if (probe.startsWith("//")) return null;
@@ -39,3 +45,16 @@ export function safeHref(raw: string | undefined | null): string | null {
const scheme = probe.slice(0, colon);
return scheme === "http" || scheme === "https" ? raw : null;
}
+
+/**
+ * Whether a link safeHref kept opens in this tab: an anchor, a query, or a
+ * path from the root of this site. A footnote is `#user-content-…`, and
+ * opening the whole page again in a new tab to show it is what happened
+ * until 26 October. Everything else opens a new tab, a bare `example.com/x`
+ * too: a browser puts it on this site, but whoever wrote it meant that one.
+ */
+export function opensHere(safe: string): boolean {
+ const probe = probeOf(safe);
+ if (probe.startsWith("//")) return false;
+ return probe.startsWith("/") || probe.startsWith("#") || probe.startsWith("?");
+}