[DONE] 댓글의 각주와 이 사이트 안의 링크가 같은 탭에서 열리고, 댓글마다 제 각주로 간다

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(<PostBody text="[datasheet](https://example.com/hub75.pdf)" />);
+    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(<PostBody text="Same as [my deck](/community/d/abc)." />);
+    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(<PostBody text={FOOTNOTE} />);
+    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(
+      <>
+        <PostBody text={FOOTNOTE} />
+        <PostBody text={"Mine too.[^1]\n\n[^1]: A 2 A bank browns out."} />
+      </>,
+    );
+    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(<PostBody text={text} />);
+      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(<PostBody text="[click](javascript:alert(1))" />);
+    expect(container.querySelector("a")).toBeNull();
+    expect(container.textContent).toBe("click");
+  });
+});

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 (
     <div className={styles.postBody}>
       <ReactMarkdown
@@ -52,11 +59,33 @@ export default function PostBody({ text }: { text: string }) {
         // write [text](url).
         allowedElements={[...ALLOWED]}
         unwrapDisallowed
+        remarkRehypeOptions={{ clobberPrefix: prefix }}
         components={{
-          a: ({ href, children }) => {
+          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 (
+                <a
+                  href={safe}
+                  id={id}
+                  aria-label={label}
+                  aria-describedby={describedBy && prefix + describedBy}
+                  className={styles.inlineLink}
+                >
+                  {children}
+                </a>
+              );
+            }
             return (
               <a
                 href={safe}
@@ -88,6 +117,13 @@ export default function PostBody({ text }: { text: string }) {
               </a>
             );
           },
+          // The one heading with an id is the footnotes' own, which
+          // clobberPrefix leaves as plain "footnote-label".
+          h2: ({ id, className, children }) => (
+            <h2 id={id && prefix + id} className={className}>
+              {children}
+            </h2>
+          ),
           pre: ({ children }) => <pre className={styles.codeFence}>{children}</pre>,
           code: ({ children, className }) =>
             // react-markdown hands fenced code a language class and inline code

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,<b>hi</b>"],
+    ["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);
+  });
+});

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("?");
+}