search: Remove highlighting in search typeahead.

Context: https://chat.zulip.org/#narrow/channel/101-design/topic/search.20typeahead.20highlighting/near/2188093

web/src/composebox_typeahead.ts

@@ -56,7 +56,7 @@ const MAX_LOOKBACK_FOR_TYPEAHEAD_COMPLETION = 60 + 6 + 20;
 //
 // So if you are not using trusted input, you MUST use a
 // highlighter that escapes (i.e. one that calls
-// typeahead_helper.highlight_with_escaping).
+// Handlebars.Utils.escapeExpression).
 
 // ---------------- TYPE DECLARATIONS ----------------
 // There are many types of suggestions that can show

web/src/filter.ts

@@ -1,4 +1,3 @@
-import Handlebars from "handlebars/runtime.js";
 import _ from "lodash";
 import assert from "minimalistic-assert";
 
@@ -261,7 +260,7 @@ export function create_user_pill_context(user: User): UserPillItem {
 
     return {
         id: user.user_id,
-        display_value: new Handlebars.SafeString(user.full_name),
+        display_value: user.full_name,
         has_image: true,
         img_src: avatar_url,
         should_add_guest_user_indicator: people.should_add_guest_user_indicator(user.user_id),

web/src/search_suggestion.ts

@@ -20,7 +20,7 @@ import * as util from "./util.ts";
 
 export type UserPillItem = {
     id: number;
-    display_value: Handlebars.SafeString;
+    display_value: string;
     has_image: boolean;
     img_src: string;
     should_add_guest_user_indicator: boolean;
@@ -49,23 +49,12 @@ function channel_matches_query(channel_name: string, q: string): boolean {
     return common.phrase_match(q, channel_name);
 }
 
-function make_person_highlighter(query: string): (person: User) => string {
-    const highlight_query = typeahead_helper.make_query_highlighter(query);
-
-    return function (person: User): string {
-        return highlight_query(person.full_name);
-    };
-}
-
-function highlight_person(person: User, highlighter: (person: User) => string): UserPillItem {
-    const avatar_url = people.small_avatar_url_for_person(person);
-    const highlighted_name = highlighter(person);
-
+function user_pill_item(person: User): UserPillItem {
     return {
         id: person.user_id,
-        display_value: new Handlebars.SafeString(highlighted_name),
+        display_value: person.full_name,
         has_image: true,
-        img_src: avatar_url,
+        img_src: people.small_avatar_url_for_person(person),
         should_add_guest_user_indicator: people.should_add_guest_user_indicator(person.user_id),
     };
 }
@@ -164,14 +153,11 @@ function get_channel_suggestions(last: NarrowTerm, terms: NarrowTerm[]): Suggest
 
     channels = typeahead_helper.sorter(query, channels, (x) => x);
 
-    const regex = typeahead_helper.build_highlight_regex(query);
-    const highlight_query = typeahead_helper.highlight_with_escaping_and_regex;
-
     return channels.map((channel_name) => {
         const prefix = "channel";
-        const highlighted_channel = highlight_query(regex, channel_name);
         const verb = last.negated ? "exclude " : "";
-        const description_html = verb + prefix + " " + highlighted_channel;
+        const description_html =
+            verb + prefix + " " + Handlebars.Utils.escapeExpression(channel_name);
         const channel = stream_data.get_sub_by_name(channel_name);
         assert(channel !== undefined);
         const term = {
@@ -278,8 +264,6 @@ function get_group_suggestions(last: NarrowTerm, terms: NarrowTerm[]): Suggestio
 
     const prefix = Filter.operator_to_prefix("dm", negated);
 
-    const person_highlighter = make_person_highlighter(last_part);
-
     return persons.map((person) => {
         const term = {
             operator: "dm",
@@ -292,10 +276,7 @@ function get_group_suggestions(last: NarrowTerm, terms: NarrowTerm[]): Suggestio
             terms = [{operator: "is", operand: "dm"}, term];
         }
 
-        const all_user_pill_contexts = [
-            ...user_pill_contexts,
-            highlight_person(person, person_highlighter),
-        ];
+        const all_user_pill_contexts = [...user_pill_contexts, user_pill_item(person)];
 
         return {
             description_html: prefix,
@@ -346,8 +327,6 @@ function get_person_suggestions(
         last = {operator: "dm", operand: "", negated: false};
     }
 
-    const query = last.operand;
-
     // Be especially strict about the less common "from" operator.
     if (autocomplete_operator === "from" && last.operator !== "from") {
         return [];
@@ -383,8 +362,6 @@ function get_person_suggestions(
 
     const prefix = Filter.operator_to_prefix(autocomplete_operator, last.negated);
 
-    const person_highlighter = make_person_highlighter(query);
-
     return persons.map((person) => {
         const terms: NarrowTerm[] = [
             {
@@ -410,7 +387,7 @@ function get_person_suggestions(
             is_people: true,
             users: [
                 {
-                    user_pill_context: highlight_person(person, person_highlighter),
+                    user_pill_context: user_pill_item(person),
                 },
             ],
         };
@@ -1103,9 +1080,7 @@ export function get_search_result(
         suggestion_line = [
             {
                 search_string: last.operand,
-                description_html: `search for <strong>${Handlebars.Utils.escapeExpression(
-                    last.operand,
-                )}</strong>`,
+                description_html: `search for ${Handlebars.Utils.escapeExpression(last.operand)}`,
                 is_people: false,
             },
         ];

web/src/typeahead_helper.ts

@@ -1,4 +1,3 @@
-import Handlebars from "handlebars/runtime.js";
 import _ from "lodash";
 import assert from "minimalistic-assert";
 
@@ -48,48 +47,6 @@ export type CombinedPillContainer = InputPillContainer<CombinedPill>;
 export type GroupSettingPill = UserGroupPill | UserPill;
 export type GroupSettingPillContainer = InputPillContainer<GroupSettingPill>;
 
-export function build_highlight_regex(query: string): RegExp {
-    const regex = new RegExp("(" + _.escapeRegExp(query) + ")", "ig");
-    return regex;
-}
-
-export function highlight_with_escaping_and_regex(regex: RegExp, item: string): string {
-    // if regex is empty return entire item escaped
-    if (regex.source === "()") {
-        return Handlebars.Utils.escapeExpression(item);
-    }
-
-    // We need to assemble this manually (as opposed to doing 'join') because we need to
-    // (1) escape all the pieces and (2) the regex is case-insensitive, and we need
-    // to know the case of the content we're replacing (you can't just use a bolded
-    // version of 'query')
-
-    const pieces = item.split(regex).filter(Boolean);
-    let result = "";
-
-    for (const [i, piece] of pieces.entries()) {
-        if (regex.test(piece) && (i === 0 || pieces[i - 1]!.endsWith(" "))) {
-            // only highlight if the matching part is a word prefix, ie
-            // if it is the 1st piece or if there was a space before it
-            result += "<strong>" + Handlebars.Utils.escapeExpression(piece) + "</strong>";
-        } else {
-            result += Handlebars.Utils.escapeExpression(piece);
-        }
-    }
-
-    return result;
-}
-
-export function make_query_highlighter(query: string): (phrase: string) => string {
-    query = query.toLowerCase();
-
-    const regex = build_highlight_regex(query);
-
-    return function (phrase) {
-        return highlight_with_escaping_and_regex(regex, phrase);
-    };
-}
-
 type StreamData = {
     invite_only: boolean;
     is_web_public: boolean;

web/tests/search.test.cjs

@@ -96,7 +96,7 @@ run_test("initialize", ({override, override_rewire, mock_template}) => {
                     [
                         "stream:Verona",
                         {
-                            description_html: "Stream <strong>Ver</strong>ona",
+                            description_html: "Stream Verona",
                             search_string: "stream:Verona",
                         },
                     ],
@@ -121,7 +121,7 @@ run_test("initialize", ({override, override_rewire, mock_template}) => {
             let expected_value = `<div class="search_list_item">\n    <span>Search for ver</span>\n</div>\n`;
             assert.equal(opts.highlighter_html(source[0]), expected_value);
 
-            expected_value = `<div class="search_list_item">\n    <span>Stream <strong>Ver</strong>ona</span>\n</div>\n`;
+            expected_value = `<div class="search_list_item">\n    <span>Stream Verona</span>\n</div>\n`;
             assert.equal(opts.highlighter_html(source[1]), expected_value);
 
             /* Test sorter */
@@ -140,7 +140,7 @@ run_test("initialize", ({override, override_rewire, mock_template}) => {
                             users: [
                                 {
                                     user_pill_context: {
-                                        display_value: "<strong>Zo</strong>e",
+                                        display_value: "Zoe",
                                         has_image: true,
                                         id: 7,
                                         img_src:
@@ -159,7 +159,7 @@ run_test("initialize", ({override, override_rewire, mock_template}) => {
                             users: [
                                 {
                                     user_pill_context: {
-                                        display_value: "<strong>Zo</strong>e",
+                                        display_value: "Zoe",
                                         has_image: true,
                                         id: 7,
                                         img_src:
@@ -178,7 +178,7 @@ run_test("initialize", ({override, override_rewire, mock_template}) => {
                             users: [
                                 {
                                     user_pill_context: {
-                                        display_value: "<strong>Zo</strong>e",
+                                        display_value: "Zoe",
                                         has_image: true,
                                         id: 7,
                                         img_src:
@@ -209,13 +209,13 @@ run_test("initialize", ({override, override_rewire, mock_template}) => {
             let expected_value = `<div class="search_list_item">\n    <span>Search for zo</span>\n</div>\n`;
             assert.equal(opts.highlighter_html(source[0]), expected_value);
 
-            expected_value = `<div class="search_list_item">\n    <span>sent by</span>\n        <span class="pill-container">\n            <div class='pill ' tabindex=0>\n    <img class="pill-image" src="https://secure.gravatar.com/avatar/0f030c97ab51312c7bbffd3966198ced?d&#x3D;identicon&amp;version&#x3D;1" />\n    <div class="pill-image-border"></div>\n    <span class="pill-label">\n        <span class="pill-value">\n            &lt;strong&gt;Zo&lt;/strong&gt;e\n        </span></span>\n    <div class="exit">\n        <a role="button" class="zulip-icon zulip-icon-close pill-close-button"></a>\n    </div>\n</div>\n        </span>\n</div>\n`;
+            expected_value = `<div class="search_list_item">\n    <span>sent by</span>\n        <span class="pill-container">\n            <div class='pill ' tabindex=0>\n    <img class="pill-image" src="https://secure.gravatar.com/avatar/0f030c97ab51312c7bbffd3966198ced?d&#x3D;identicon&amp;version&#x3D;1" />\n    <div class="pill-image-border"></div>\n    <span class="pill-label">\n        <span class="pill-value">\n            Zoe\n        </span></span>\n    <div class="exit">\n        <a role="button" class="zulip-icon zulip-icon-close pill-close-button"></a>\n    </div>\n</div>\n        </span>\n</div>\n`;
             assert.equal(opts.highlighter_html(source[1]), expected_value);
 
-            expected_value = `<div class="search_list_item">\n    <span>direct messages with</span>\n        <span class="pill-container">\n            <div class='pill ' tabindex=0>\n    <img class="pill-image" src="https://secure.gravatar.com/avatar/0f030c97ab51312c7bbffd3966198ced?d&#x3D;identicon&amp;version&#x3D;1" />\n    <div class="pill-image-border"></div>\n    <span class="pill-label">\n        <span class="pill-value">\n            &lt;strong&gt;Zo&lt;/strong&gt;e\n        </span></span>\n    <div class="exit">\n        <a role="button" class="zulip-icon zulip-icon-close pill-close-button"></a>\n    </div>\n</div>\n        </span>\n</div>\n`;
+            expected_value = `<div class="search_list_item">\n    <span>direct messages with</span>\n        <span class="pill-container">\n            <div class='pill ' tabindex=0>\n    <img class="pill-image" src="https://secure.gravatar.com/avatar/0f030c97ab51312c7bbffd3966198ced?d&#x3D;identicon&amp;version&#x3D;1" />\n    <div class="pill-image-border"></div>\n    <span class="pill-label">\n        <span class="pill-value">\n            Zoe\n        </span></span>\n    <div class="exit">\n        <a role="button" class="zulip-icon zulip-icon-close pill-close-button"></a>\n    </div>\n</div>\n        </span>\n</div>\n`;
             assert.equal(opts.highlighter_html(source[2]), expected_value);
 
-            expected_value = `<div class="search_list_item">\n    <span>group direct messages including</span>\n        <span class="pill-container">\n            <div class='pill ' tabindex=0>\n    <img class="pill-image" src="https://secure.gravatar.com/avatar/0f030c97ab51312c7bbffd3966198ced?d&#x3D;identicon&amp;version&#x3D;1" />\n    <div class="pill-image-border"></div>\n    <span class="pill-label">\n        <span class="pill-value">\n            &lt;strong&gt;Zo&lt;/strong&gt;e\n        </span></span>\n    <div class="exit">\n        <a role="button" class="zulip-icon zulip-icon-close pill-close-button"></a>\n    </div>\n</div>\n        </span>\n</div>\n`;
+            expected_value = `<div class="search_list_item">\n    <span>group direct messages including</span>\n        <span class="pill-container">\n            <div class='pill ' tabindex=0>\n    <img class="pill-image" src="https://secure.gravatar.com/avatar/0f030c97ab51312c7bbffd3966198ced?d&#x3D;identicon&amp;version&#x3D;1" />\n    <div class="pill-image-border"></div>\n    <span class="pill-label">\n        <span class="pill-value">\n            Zoe\n        </span></span>\n    <div class="exit">\n        <a role="button" class="zulip-icon zulip-icon-close pill-close-button"></a>\n    </div>\n</div>\n        </span>\n</div>\n`;
             assert.equal(opts.highlighter_html(source[3]), expected_value);
 
             /* Test sorter */

web/tests/search_suggestion.test.cjs

@@ -705,7 +705,7 @@ test("topic_suggestions", ({override, mock_template}) => {
     function describe(q) {
         return suggestions.lookup_table.get(q).description_html;
     }
-    assert.equal(describe("te"), "Search for <strong>te</strong>");
+    assert.equal(describe("te"), "Search for te");
     assert.equal(describe(`channel:${office_id} topic:team`), "Channel office > team");
 
     suggestions = get_suggestions(`topic:staplers channel:${office_id}`);
@@ -818,6 +818,18 @@ test("whitespace_glitch", ({override, mock_template}) => {
     assert.deepEqual(suggestions.strings, expected);
 });
 
+test("xss_channel_name", () => {
+    const stream_id = new_stream_id();
+    stream_data.add_sub({stream_id, name: "<em> Italics </em>", subscribed: true});
+
+    const query = "channel:ita";
+    const suggestions = get_suggestions(query);
+    assert.deepEqual(
+        suggestions.lookup_table.get(`channel:${stream_id}`).description_html,
+        "Channel &lt;em&gt; Italics &lt;/em&gt;",
+    );
+});
+
 test("channel_completion", ({override}) => {
     const office_stream_id = new_stream_id();
     stream_data.add_sub({stream_id: office_stream_id, name: "office", subscribed: true});
@@ -944,7 +956,7 @@ test("people_suggestions", ({override, mock_template}) => {
     test_describe("sender:ted@zulip.com", "Sent by");
     test_describe("dm-including:ted@zulip.com", "Direct messages including");
 
-    let expectedString = "<strong>Te</strong>d Smith";
+    let expectedString = "Ted Smith";
 
     function test_full_name(q, full_name_html) {
         return suggestions.lookup_table.get(q).description_html.includes(full_name_html);

web/tests/typeahead_helper.test.cjs

@@ -925,36 +925,6 @@ test("test compare directly for direct message", () => {
     assert.equal(th.compare_people_for_relevance(zman_item, all_obj_item), -1);
 });
 
-test("highlight_with_escaping", () => {
-    function highlight(query, item) {
-        return th.make_query_highlighter(query)(item);
-    }
-
-    let item = "Denmark";
-    let query = "Den";
-    let expected = "<strong>Den</strong>mark";
-    let result = highlight(query, item);
-    assert.equal(result, expected);
-
-    item = "w3IrD_naMe";
-    query = "w3IrD_naMe";
-    expected = "<strong>w3IrD_naMe</strong>";
-    result = highlight(query, item);
-    assert.equal(result, expected);
-
-    item = "development help";
-    query = "development h";
-    expected = "<strong>development h</strong>elp";
-    result = highlight(query, item);
-    assert.equal(result, expected);
-
-    item = "Prefix notprefix prefix";
-    query = "pre";
-    expected = "<strong>Pre</strong>fix notprefix <strong>pre</strong>fix";
-    result = highlight(query, item);
-    assert.equal(result, expected);
-});
-
 test("render_person when emails hidden", ({mock_template, override}) => {
     // Test render_person with regular person, under hidden email visibility case
     override(realm, "custom_profile_field_types", {