fix(html): percent-encode unsafe characters in symbol page paths

Symbol names that come from string literals (e.g. `obj["a/b"]` or a
property named `'"><img src=x onerror=alert(1)>'`) can contain characters
that are reserved on common filesystems (Windows forbids `<>:"/\|?*`) or
that have special meaning in a URL. Such a name was used verbatim as the
file-name component of the generated page, producing an invalid path that
crashed `deno doc --html` on Windows, and an unescaped link that could 404
or inject markup into the surrounding attribute.

Percent-encode the unsafe characters when building both the written file
name and every link to it, so they stay consistent. Ordinary identifier
names are unaffected (the encoder borrows without allocating).

Author: crowlbot

src/html/mod.rs

@@ -1401,8 +1401,11 @@ pub fn generate(
               Some(short_path),
             );
 
-            let file_name =
-              format!("{}/~/{}.html", short_path.path, symbol_group_ctx.name);
+            let file_name = format!(
+              "{}/~/{}.html",
+              short_path.path,
+              util::sanitize_symbol_path_part(&symbol_group_ctx.name)
+            );
 
             let page_ctx = pages::SymbolPageCtx {
               html_head_ctx,
@@ -1426,8 +1429,11 @@ pub fn generate(
             let redirect =
               serde_json::json!({ "kind": "redirect", "path": href });
 
-            let file_name =
-              format!("{}/~/{}.html", short_path.path, current_symbol);
+            let file_name = format!(
+              "{}/~/{}.html",
+              short_path.path,
+              util::sanitize_symbol_path_part(&current_symbol)
+            );
 
             vec![(file_name, ctx.render("pages/redirect", &redirect))]
           }
@@ -1616,8 +1622,11 @@ where
               continue;
             }
 
-            let file_name =
-              format!("{}/~/{}.json", short_path.path, symbol_group_ctx.name);
+            let file_name = format!(
+              "{}/~/{}.json",
+              short_path.path,
+              util::sanitize_symbol_path_part(&symbol_group_ctx.name)
+            );
             if !emitted_keys.insert(file_name.clone()) {
               continue;
             }
@@ -1667,8 +1676,11 @@ where
               continue;
             }
 
-            let file_name =
-              format!("{}/~/{}.json", short_path.path, current_symbol);
+            let file_name = format!(
+              "{}/~/{}.json",
+              short_path.path,
+              util::sanitize_symbol_path_part(&current_symbol)
+            );
             if !emitted_keys.insert(file_name.clone()) {
               continue;
             }

src/html/util.rs

@@ -402,7 +402,11 @@ pub fn href_path_resolve(
       symbol: target_symbol,
       ..
     } => {
-      format!("{backs}./{}/~/{target_symbol}.html", target_file.path)
+      format!(
+        "{backs}./{}/~/{}.html",
+        target_file.path,
+        sanitize_symbol_path_part(target_symbol)
+      )
     }
     UrlResolveKind::File { file: target_file } => {
       format!("{backs}./{}/index.html", target_file.path)
@@ -955,3 +959,57 @@ pub fn slugify(name: &str) -> String {
     .replace_all(&name.to_lowercase(), "")
     .replace(' ', "-")
 }
+
+/// Whether a character is unsafe inside the file-name component of a symbol
+/// page, either because it is reserved on common filesystems (Windows forbids
+/// `<>:"/\|?*`) or because it has special meaning in a URL path segment (`#`,
+/// `?`, `%`, whitespace, …).
+fn is_symbol_path_unsafe(c: char) -> bool {
+  c.is_control()
+    || c.is_whitespace()
+    || matches!(
+      c,
+      '"'
+        | '#'
+        | '%'
+        | '<'
+        | '>'
+        | '?'
+        | '`'
+        | '{'
+        | '}'
+        | '|'
+        | '\\'
+        | '^'
+        | '*'
+        | ':'
+        | '/'
+    )
+}
+
+/// Percent-encode any character of a symbol name that is unsafe to use as the
+/// file-name component of its generated page. Symbol names are normally plain
+/// identifiers, for which this is a borrow-only no-op; but a symbol whose name
+/// comes from a string literal (e.g. `obj["a/b"]` or `'"><img …>'`) can contain
+/// characters that would otherwise produce an invalid file path or a broken —
+/// and potentially HTML-injecting — link. The same encoding is applied wherever
+/// such a path is built (the written file name and every link to it) so they
+/// stay consistent. See issue #724.
+pub(crate) fn sanitize_symbol_path_part(name: &str) -> Cow<'_, str> {
+  if !name.chars().any(is_symbol_path_unsafe) {
+    return Cow::Borrowed(name);
+  }
+
+  let mut out = String::with_capacity(name.len());
+  let mut buf = [0u8; 4];
+  for c in name.chars() {
+    if is_symbol_path_unsafe(c) {
+      for b in c.encode_utf8(&mut buf).as_bytes() {
+        out.push_str(&format!("%{b:02X}"));
+      }
+    } else {
+      out.push(c);
+    }
+  }
+  Cow::Owned(out)
+}

tests/html_test.rs

@@ -352,7 +352,7 @@ async fn html_doc_files_multiple() {
       "./~/Enum2.html",
       "./~/Foo.bar.html",
       "./~/Foo.html",
-      "./~/Foo.prototype.\"><img src=x onerror=alert(1)>.html",
+      "./~/Foo.prototype.%22%3E%3Cimg%20src=x%20onerror=alert(1)%3E.html",
       "./~/Foo.prototype.[Symbol.iterator].html",
       "./~/Foo.prototype.foo.html",
       "./~/Foo.prototype.getter.html",
@@ -697,6 +697,70 @@ async fn parse_file(path: &std::path::Path) -> ParseOutput {
   parse_source(&content).await
 }
 
+// Regression test for https://github.com/denoland/deno_doc/issues/724:
+// a member whose name comes from a string literal can contain characters that
+// are invalid in a file name (Windows reserves `<>:"/\|?*`) or that break a
+// URL. The generated page path must be filesystem- and URL-safe, and the link
+// to it must use the same encoded form so it still resolves.
+#[tokio::test]
+async fn html_symbol_name_unsafe_chars_in_path() {
+  let source = r#"
+export class Foo {
+  "a/b<c>": number = 0;
+}
+"#;
+
+  let ctx = GenerateCtx::create_basic(
+    GenerateOptions {
+      package_name: None,
+      main_entrypoint: None,
+      href_resolver: Arc::new(EmptyResolver),
+      usage_composer: Some(Arc::new(EmptyResolver)),
+      rewrite_map: None,
+      category_docs: None,
+      disable_search: false,
+      symbol_redirect_map: None,
+      default_symbol_map: None,
+      markdown_renderer: comrak::create_renderer(None, None, None),
+      markdown_stripper: Arc::new(comrak::strip),
+      head_inject: None,
+      id_prefix: None,
+      diff_only: false,
+    },
+    parse_source(source).await,
+    None,
+  )
+  .unwrap();
+
+  let files = generate(ctx).unwrap();
+
+  // No generated file path may contain characters that are reserved on common
+  // filesystems or that need escaping in a URL.
+  for name in files.keys() {
+    assert!(
+      !name.contains(['"', '<', '>', '|', '?', '*', '\\', ' ']),
+      "generated file path is not filesystem/URL safe: {name}"
+    );
+  }
+
+  // The member still gets a page, under a percent-encoded name
+  // (`a/b<c>` -> `a%2Fb%3Cc%3E`).
+  assert!(
+    files
+      .keys()
+      .any(|k| k.ends_with("/~/Foo.prototype.a%2Fb%3Cc%3E.html")),
+    "expected a percent-encoded page for the member, got: {:?}",
+    files.keys().collect::<Vec<_>>()
+  );
+
+  // No rendered page may contain the raw (unencoded) link, which would both
+  // 404 and inject markup into the surrounding attribute.
+  assert!(
+    files.values().all(|content| !content.contains("a/b<c>")),
+    "a rendered page contains an unsafe, unencoded symbol link"
+  );
+}
+
 #[tokio::test]
 async fn diff_kind_change() {
   let test_dir = std::env::current_dir()

tests/snapshots/html_test__html_doc_files_multiple-23.snap

@@ -555,7 +555,7 @@ Properties</h2></div><div class="space-y-8"><div class="anchorable docEntry" id=
   </defs>
 </svg>
 </a>
-<a class="font-bold font-lg link" href="..&#x2F;.&#x2F;.&#x2F;~&#x2F;Foo.prototype.&quot;&gt;&lt;img src=x onerror=alert(1)&gt;.html">"&gt;&lt;img src=x onerror=alert(1)&gt;</a><span class="font-medium text-stone-500 dark:text-stone-200"><span>: <span class="td-kw">number</span></span></span>
+<a class="font-bold font-lg link" href="..&#x2F;.&#x2F;.&#x2F;~&#x2F;Foo.prototype.%22%3E%3Cimg%20src=x%20onerror=alert(1)%3E.html">"&gt;&lt;img src=x onerror=alert(1)&gt;</a><span class="font-medium text-stone-500 dark:text-stone-200"><span>: <span class="td-kw">number</span></span></span>
       </code>
     </div></div></div>
 <div class="anchorable docEntry" id="property_foo">

tests/snapshots/html_test__html_doc_files_multiple-73.snap

@@ -216,7 +216,7 @@ expression: files
                         {
                           "name_prefix": null,
                           "name": "\"><img src=x onerror=alert(1)>",
-                          "name_href": "../././~/Foo.prototype.\"><img src=x onerror=alert(1)>.html",
+                          "name_href": "../././~/Foo.prototype.%22%3E%3Cimg%20src=x%20onerror=alert(1)%3E.html",
                           "content": "<span>: <span class=\"td-kw\">number</span></span>",
                           "anchor": {
                             "id": "property_&quot;&gt;&lt;img-src=x-onerror=alert(1)&gt;"
@@ -7841,7 +7841,7 @@ expression: files
         },
         {
           "name": "\"><img src=x onerror=alert(1)>",
-          "href": "../././~/Foo.prototype.\"><img src=x onerror=alert(1)>.html"
+          "href": "../././~/Foo.prototype.%22%3E%3Cimg%20src=x%20onerror=alert(1)%3E.html"
         }
       ]
     },

tests/snapshots/html_test__symbol_group.snap

@@ -212,7 +212,7 @@ expression: search_index
       "name": "Foo.prototype.\"><img src=x onerror=alert(1)>",
       "file": ".",
       "doc": "",
-      "url": "././~/Foo.prototype.\"><img src=x onerror=alert(1)>.html",
+      "url": "././~/Foo.prototype.%22%3E%3Cimg%20src=x%20onerror=alert(1)%3E.html",
       "deprecated": false
     },
     {