util: Rename clean_user_content_links to postprocess_content.

Signed-off-by: Anders Kaseorg <anders@zulip.com>

tools/linter_lib/custom_check.py

@@ -139,7 +139,7 @@ js_rules = RuleList(
         {"pattern": r"\+.*\$t\(.+\)", "description": "Do not concatenate i18n strings"},
         {
             "pattern": "[.]html[(]",
-            "exclude_pattern": r"""\.html\(("|'|render_|\w+_html|html|message\.content|util\.clean_user_content_links|rendered_|$|\)|error_html|widget_elem|\$error|\$\("<p>"\))""",
+            "exclude_pattern": r"""\.html\(("|'|render_|\w+_html|html|message\.content|postprocess_content|rendered_|$|\)|error_html|widget_elem|\$error|\$\("<p>"\))""",
             "exclude": {
                 "web/src/portico",
                 "web/src/lightbox.ts",

web/src/compose.js

@@ -21,6 +21,7 @@ import * as markdown from "./markdown";
 import * as message_events from "./message_events";
 import * as onboarding_steps from "./onboarding_steps";
 import * as people from "./people";
+import {postprocess_content} from "./postprocess_content";
 import * as rendered_markdown from "./rendered_markdown";
 import * as scheduled_messages from "./scheduled_messages";
 import * as sent_messages from "./sent_messages";
@@ -350,7 +351,7 @@ export function render_and_show_preview($preview_spinner, $preview_content_box,
             rendered_preview_html = rendered_content;
         }
 
-        $preview_content_box.html(util.clean_user_content_links(rendered_preview_html));
+        $preview_content_box.html(postprocess_content(rendered_preview_html));
         rendered_markdown.update_elements($preview_content_box);
     }
 

web/src/info_overlay.ts

@@ -13,11 +13,11 @@ import * as keydown_util from "./keydown_util";
 import * as markdown from "./markdown";
 import * as overlays from "./overlays";
 import {page_params} from "./page_params";
+import {postprocess_content} from "./postprocess_content";
 import * as rendered_markdown from "./rendered_markdown";
 import * as scroll_util from "./scroll_util";
 import {current_user} from "./state_data";
 import {user_settings} from "./user_settings";
-import * as util from "./util";
 
 // Make it explicit that our toggler is undefined until
 // set_up_toggler is called.
@@ -269,7 +269,7 @@ export function set_up_toggler(): void {
                 raw_content: row.markdown,
                 ...markdown.render(row.markdown),
             };
-            row.output_html = util.clean_user_content_links(message.content);
+            row.output_html = postprocess_content(message.content);
         }
     }
 

web/src/postprocess_content.ts

@@ -0,0 +1,88 @@
+import {$t} from "./i18n";
+
+let inertDocument: Document | undefined;
+
+export function postprocess_content(html: string): string {
+    if (inertDocument === undefined) {
+        inertDocument = new DOMParser().parseFromString("", "text/html");
+    }
+    const template = inertDocument.createElement("template");
+    template.innerHTML = html;
+
+    for (const elt of template.content.querySelectorAll("a")) {
+        // Ensure that all external links have target="_blank"
+        // rel="opener noreferrer".  This ensures that external links
+        // never replace the Zulip web app while also protecting
+        // against reverse tabnapping attacks, without relying on the
+        // correctness of how Zulip's Markdown processor generates links.
+        //
+        // Fragment links, which we intend to only open within the
+        // Zulip web app using our hashchange system, do not require
+        // these attributes.
+        const href = elt.getAttribute("href");
+        if (href === null) {
+            continue;
+        }
+        let url;
+        try {
+            url = new URL(href, window.location.href);
+        } catch {
+            elt.removeAttribute("href");
+            elt.removeAttribute("title");
+            continue;
+        }
+
+        // eslint-disable-next-line no-script-url
+        if (["data:", "javascript:", "vbscript:"].includes(url.protocol)) {
+            // Remove unsafe links completely.
+            elt.removeAttribute("href");
+            elt.removeAttribute("title");
+            continue;
+        }
+
+        // We detect URLs that are just fragments by comparing the URL
+        // against a new URL generated using only the hash.
+        if (url.hash === "" || url.href !== new URL(url.hash, window.location.href).href) {
+            elt.setAttribute("target", "_blank");
+            elt.setAttribute("rel", "noopener noreferrer");
+        } else {
+            elt.removeAttribute("target");
+        }
+
+        if (elt.parentElement?.classList.contains("message_inline_image")) {
+            // For inline images we want to handle the tooltips explicitly, and disable
+            // the browser's built in handling of the title attribute.
+            const title = elt.getAttribute("title");
+            if (title !== null) {
+                elt.setAttribute("aria-label", title);
+                elt.removeAttribute("title");
+            }
+        } else {
+            // For non-image user uploads, the following block ensures that the title
+            // attribute always displays the filename as a security measure.
+            let title: string;
+            let legacy_title: string;
+            if (
+                url.origin === window.location.origin &&
+                url.pathname.startsWith("/user_uploads/")
+            ) {
+                // We add the word "download" to make clear what will
+                // happen when clicking the file.  This is particularly
+                // important in the desktop app, where hovering a URL does
+                // not display the URL like it does in the web app.
+                title = legacy_title = $t(
+                    {defaultMessage: "Download {filename}"},
+                    {filename: url.pathname.slice(url.pathname.lastIndexOf("/") + 1)},
+                );
+            } else {
+                title = url.toString();
+                legacy_title = href;
+            }
+            elt.setAttribute(
+                "title",
+                ["", legacy_title].includes(elt.title) ? title : `${title}\n${elt.title}`,
+            );
+        }
+    }
+    return template.innerHTML;
+}

web/src/stream_edit.js

@@ -20,6 +20,7 @@ import {$t, $t_html} from "./i18n";
 import * as keydown_util from "./keydown_util";
 import * as narrow_state from "./narrow_state";
 import * as popovers from "./popovers";
+import {postprocess_content} from "./postprocess_content";
 import * as scroll_util from "./scroll_util";
 import * as settings_components from "./settings_components";
 import * as settings_config from "./settings_config";
@@ -38,7 +39,6 @@ import * as sub_store from "./sub_store";
 import * as ui_report from "./ui_report";
 import * as user_groups from "./user_groups";
 import {user_settings} from "./user_settings";
-import * as util from "./util";
 
 export function setup_subscriptions_tab_hash(tab_key_value) {
     if ($("#subscription_overlay .right").hasClass("show")) {
@@ -132,7 +132,7 @@ export function update_stream_description(sub) {
     const $edit_container = stream_settings_containers.get_edit_container(sub);
     $edit_container.find("input.description").val(sub.description);
     const html = render_stream_description({
-        rendered_description: util.clean_user_content_links(sub.rendered_description),
+        rendered_description: postprocess_content(sub.rendered_description),
     });
     $edit_container.find(".stream-description").html(html);
 }

web/src/stream_settings_ui.js

@@ -22,6 +22,7 @@ import * as message_live_update from "./message_live_update";
 import * as message_view_header from "./message_view_header";
 import * as narrow_state from "./narrow_state";
 import * as overlays from "./overlays";
+import {postprocess_content} from "./postprocess_content";
 import * as resize from "./resize";
 import * as scroll_util from "./scroll_util";
 import * as search_util from "./search_util";
@@ -39,7 +40,6 @@ import * as stream_settings_components from "./stream_settings_components";
 import * as stream_settings_data from "./stream_settings_data";
 import * as stream_ui_updates from "./stream_ui_updates";
 import * as sub_store from "./sub_store";
-import * as util from "./util";
 
 export function is_sub_already_present(sub) {
     return stream_ui_updates.row_for_stream_id(sub.stream_id).length > 0;
@@ -133,7 +133,7 @@ export function update_stream_description(sub, description, rendered_description
 
     // Update stream row
     const $sub_row = stream_ui_updates.row_for_stream_id(sub.stream_id);
-    $sub_row.find(".description").html(util.clean_user_content_links(sub.rendered_description));
+    $sub_row.find(".description").html(postprocess_content(sub.rendered_description));
 
     // Update stream settings
     stream_edit.update_stream_description(sub);

web/src/templates.js

@@ -2,7 +2,7 @@ import Handlebars from "handlebars/runtime";
 
 import * as common from "./common";
 import {default_html_elements, intl} from "./i18n";
-import * as util from "./util";
+import {postprocess_content} from "./postprocess_content";
 
 // Below, we register Zulip-specific extensions to the Handlebars API.
 //
@@ -112,7 +112,7 @@ Handlebars.registerHelper("tr", function (options) {
 
 Handlebars.registerHelper(
     "rendered_markdown",
-    (content) => new Handlebars.SafeString(util.clean_user_content_links(content)),
+    (content) => new Handlebars.SafeString(postprocess_content(content)),
 );
 
 Handlebars.registerHelper("numberFormat", (number) => number.toLocaleString());

web/src/util.ts

@@ -1,7 +1,6 @@
 import _ from "lodash";
 
 import * as blueslip from "./blueslip";
-import {$t} from "./i18n";
 import type {MatchedMessage, Message, RawMessage} from "./message_store";
 import type {UpdateMessageEvent} from "./types";
 import {user_settings} from "./user_settings";
@@ -292,93 +291,6 @@ export function canonicalize_stream_synonyms(text: string): string {
     return text;
 }
 
-let inertDocument: Document | undefined;
-
-export function clean_user_content_links(html: string): string {
-    if (inertDocument === undefined) {
-        inertDocument = new DOMParser().parseFromString("", "text/html");
-    }
-    const template = inertDocument.createElement("template");
-    template.innerHTML = html;
-
-    for (const elt of template.content.querySelectorAll("a")) {
-        // Ensure that all external links have target="_blank"
-        // rel="opener noreferrer".  This ensures that external links
-        // never replace the Zulip web app while also protecting
-        // against reverse tabnapping attacks, without relying on the
-        // correctness of how Zulip's Markdown processor generates links.
-        //
-        // Fragment links, which we intend to only open within the
-        // Zulip web app using our hashchange system, do not require
-        // these attributes.
-        const href = elt.getAttribute("href");
-        if (href === null) {
-            continue;
-        }
-        let url;
-        try {
-            url = new URL(href, window.location.href);
-        } catch {
-            elt.removeAttribute("href");
-            elt.removeAttribute("title");
-            continue;
-        }
-
-        // eslint-disable-next-line no-script-url
-        if (["data:", "javascript:", "vbscript:"].includes(url.protocol)) {
-            // Remove unsafe links completely.
-            elt.removeAttribute("href");
-            elt.removeAttribute("title");
-            continue;
-        }
-
-        // We detect URLs that are just fragments by comparing the URL
-        // against a new URL generated using only the hash.
-        if (url.hash === "" || url.href !== new URL(url.hash, window.location.href).href) {
-            elt.setAttribute("target", "_blank");
-            elt.setAttribute("rel", "noopener noreferrer");
-        } else {
-            elt.removeAttribute("target");
-        }
-
-        if (elt.parentElement?.classList.contains("message_inline_image")) {
-            // For inline images we want to handle the tooltips explicitly, and disable
-            // the browser's built in handling of the title attribute.
-            const title = elt.getAttribute("title");
-            if (title !== null) {
-                elt.setAttribute("aria-label", title);
-                elt.removeAttribute("title");
-            }
-        } else {
-            // For non-image user uploads, the following block ensures that the title
-            // attribute always displays the filename as a security measure.
-            let title: string;
-            let legacy_title: string;
-            if (
-                url.origin === window.location.origin &&
-                url.pathname.startsWith("/user_uploads/")
-            ) {
-                // We add the word "download" to make clear what will
-                // happen when clicking the file.  This is particularly
-                // important in the desktop app, where hovering a URL does
-                // not display the URL like it does in the web app.
-                title = legacy_title = $t(
-                    {defaultMessage: "Download {filename}"},
-                    {filename: url.pathname.slice(url.pathname.lastIndexOf("/") + 1)},
-                );
-            } else {
-                title = url.toString();
-                legacy_title = href;
-            }
-            elt.setAttribute(
-                "title",
-                ["", legacy_title].includes(elt.title) ? title : `${title}\n${elt.title}`,
-            );
-        }
-    }
-    return template.innerHTML;
-}
-
 export function filter_by_word_prefix_match<T>(
     items: T[],
     search_term: string,

web/tests/postprocess_content.test.js

@@ -0,0 +1,33 @@
+"use strict";
+
+const {strict: assert} = require("assert");
+
+const {zrequire} = require("./lib/namespace");
+const {run_test} = require("./lib/test");
+
+const {postprocess_content} = zrequire("postprocess_content");
+
+run_test("postprocess_content", () => {
+    assert.equal(
+        postprocess_content(
+            '<a href="http://example.com">good</a> ' +
+                '<a href="http://zulip.zulipdev.com/user_uploads/w/ha/tever/file.png">upload</a> ' +
+                '<a href="http://localhost:NNNN">invalid</a> ' +
+                '<a href="javascript:alert(1)">unsafe</a> ' +
+                '<a href="/#fragment" target="_blank">fragment</a>' +
+                '<div class="message_inline_image">' +
+                '<a href="http://zulip.zulipdev.com/user_uploads/w/ha/tever/inline.png" title="inline image">upload</a> ' +
+                '<a role="button">button</a> ' +
+                "</div>",
+        ),
+        '<a href="http://example.com" target="_blank" rel="noopener noreferrer" title="http://example.com/">good</a> ' +
+            '<a href="http://zulip.zulipdev.com/user_uploads/w/ha/tever/file.png" target="_blank" rel="noopener noreferrer" title="translated: Download file.png">upload</a> ' +
+            "<a>invalid</a> " +
+            "<a>unsafe</a> " +
+            '<a href="/#fragment" title="http://zulip.zulipdev.com/#fragment">fragment</a>' +
+            '<div class="message_inline_image">' +
+            '<a href="http://zulip.zulipdev.com/user_uploads/w/ha/tever/inline.png" target="_blank" rel="noopener noreferrer" aria-label="inline image">upload</a> ' +
+            '<a role="button">button</a> ' +
+            "</div>",
+    );
+});

web/tests/util.test.js

@@ -302,31 +302,6 @@ run_test("move_array_elements_to_front", () => {
     assert.deepEqual(emails_actual, emails_expected);
 });
 
-run_test("clean_user_content_links", () => {
-    assert.equal(
-        util.clean_user_content_links(
-            '<a href="http://example.com">good</a> ' +
-                '<a href="http://zulip.zulipdev.com/user_uploads/w/ha/tever/file.png">upload</a> ' +
-                '<a href="http://localhost:NNNN">invalid</a> ' +
-                '<a href="javascript:alert(1)">unsafe</a> ' +
-                '<a href="/#fragment" target="_blank">fragment</a>' +
-                '<div class="message_inline_image">' +
-                '<a href="http://zulip.zulipdev.com/user_uploads/w/ha/tever/inline.png" title="inline image">upload</a> ' +
-                '<a role="button">button</a> ' +
-                "</div>",
-        ),
-        '<a href="http://example.com" target="_blank" rel="noopener noreferrer" title="http://example.com/">good</a> ' +
-            '<a href="http://zulip.zulipdev.com/user_uploads/w/ha/tever/file.png" target="_blank" rel="noopener noreferrer" title="translated: Download file.png">upload</a> ' +
-            "<a>invalid</a> " +
-            "<a>unsafe</a> " +
-            '<a href="/#fragment" title="http://zulip.zulipdev.com/#fragment">fragment</a>' +
-            '<div class="message_inline_image">' +
-            '<a href="http://zulip.zulipdev.com/user_uploads/w/ha/tever/inline.png" target="_blank" rel="noopener noreferrer" aria-label="inline image">upload</a> ' +
-            '<a role="button">button</a> ' +
-            "</div>",
-    );
-});
-
 run_test("filter_by_word_prefix_match", () => {
     const strings = ["stream-hyphen_underscore/slash", "three word stream"];
     const values = [0, 1];