diff --git a/src/html/mod.rs b/src/html/mod.rs index 76620d033..356a0c261 100644 --- a/src/html/mod.rs +++ b/src/html/mod.rs @@ -96,6 +96,13 @@ fn setup_hbs() -> Result, anyhow::Error> { handlebars_helper!(print: |a: Json| println!("{a:#?}")); reg.register_helper("print", Box::new(print)); + // Escape a value for use in plain text contexts (such as ``), where + // only `&`, `<` and `>` need escaping. The registry-wide escaper is the + // stricter `encode_safe`, which also escapes characters like `/` that are + // harmless in text content, turning e.g. `I/O` into `I/O`. + handlebars_helper!(escape_text: |a: str| html_escape::encode_text(a).into_owned()); + reg.register_helper("escape_text", Box::new(escape_text)); + reg.register_template_string( ToCCtx::TEMPLATE, include_str!("./templates/toc.hbs"), diff --git a/src/html/templates/pages/html_head.hbs b/src/html/templates/pages/html_head.hbs index 1be0a5c12..f1360b4eb 100644 --- a/src/html/templates/pages/html_head.hbs +++ b/src/html/templates/pages/html_head.hbs @@ -1,7 +1,7 @@ <!DOCTYPE html> <html> <head> - <title>{{title}} + {{{escape_text title}}} diff --git a/tests/html_test.rs b/tests/html_test.rs index 0b90708bf..71aa826fd 100644 --- a/tests/html_test.rs +++ b/tests/html_test.rs @@ -1097,3 +1097,126 @@ async fn html_output_is_valid() { assert_generated_html_is_valid(&files); } + +/// The page `` should only escape characters that are unsafe in text +/// content (`&`, `<`, `>`). Characters like `/` are harmless and must not be +/// over-escaped into entities like `/` (regression test for `I/O` +/// rendering as `I/O`). +#[tokio::test] +async fn title_does_not_over_escape_slash() { + let source = r#" +/** A simple function. */ +export function hello(): string { + return "hello"; +} +"#; + + let doc_nodes_by_url = parse_source(source).await; + + let specifier = ModuleSpecifier::parse("file:///mod.ts").unwrap(); + + let ctx = GenerateCtx::create_basic( + GenerateOptions { + // A scoped package name naturally contains a `/`. + package_name: Some("@deno/cool".to_string()), + main_entrypoint: Some(specifier), + 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, + }, + doc_nodes_by_url, + None, + ) + .unwrap(); + + let files = generate(ctx).unwrap(); + let index_html = files.get("./index.html").unwrap(); + + // Scope the check to the `<title>` element itself: `/` is still expected + // elsewhere on the page (e.g. in attribute/URL values escaped by the + // registry-wide escaper), but the title must read `@deno/cool`, not + // `@deno/cool`. + let title_start = index_html.find("<title>").expect("title tag"); + let title_end = index_html.find("").expect("title close tag"); + let title = &index_html[title_start..title_end]; + + assert_eq!( + title, "@deno/cool documentation", + "title should contain an unescaped `/`" + ); + assert!( + !title.contains("/"), + "title should not over-escape `/` into `/`" + ); +} + +/// Symbol names containing HTML-special characters must still be escaped in +/// the `<title>` so they cannot break out of the element. +#[tokio::test] +async fn title_escapes_html_special_chars() { + let source = r#" +/** A class with a dangerous property name. */ +export class Foo { + "<script>" = 1; +} +"#; + + let doc_nodes_by_url = parse_source(source).await; + + let specifier = ModuleSpecifier::parse("file:///mod.ts").unwrap(); + + let ctx = GenerateCtx::create_basic( + GenerateOptions { + package_name: None, + main_entrypoint: Some(specifier), + 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, + }, + doc_nodes_by_url, + None, + ) + .unwrap(); + + let files = generate(ctx).unwrap(); + + // Find the generated page whose `<title>` is built from the dangerous + // property name, regardless of the exact file name. + let title_page = files + .iter() + .filter(|(name, _)| name.ends_with(".html")) + .map(|(_, content)| content) + .find(|content| { + content + .lines() + .any(|line| line.contains("<title>") && line.contains("script")) + }) + .expect("a page whose title references the property should exist"); + + assert!( + !title_page.contains("<title>Foo.prototype.\"<script>\""), + "raw `<script>` must not appear unescaped in the title" + ); + assert!( + title_page.contains("<script>"), + "`<` and `>` in the title must be escaped" + ); +} diff --git a/tests/snapshots/html_test__html_doc_files_multiple-24.snap b/tests/snapshots/html_test__html_doc_files_multiple-24.snap index 4fb37b548..58a4419ec 100644 --- a/tests/snapshots/html_test__html_doc_files_multiple-24.snap +++ b/tests/snapshots/html_test__html_doc_files_multiple-24.snap @@ -5,7 +5,7 @@ expression: files.get(file_name).unwrap() <!DOCTYPE html> <html> <head> - <title>Foo.prototype."><img src=x onerror=alert(1)> - default - documentation + Foo.prototype."><img src=x onerror=alert(1)> - default - documentation