url_encoding: Standardize to use encode_hash_component.

Previously we use `hash_util_encode` to encode channel and topic names
to be URL compatible. This uses the more capable `encode_hash_component`
from the recently added `topic_link_utils.py` module. It also moves the
function to `url_encoding.py`

zerver/lib/markdown/__init__.py

@@ -65,7 +65,7 @@ from zerver.lib.thumbnail import (
 from zerver.lib.timeout import unsafe_timeout
 from zerver.lib.timezone import common_timezones
 from zerver.lib.types import LinkifierDict
-from zerver.lib.url_encoding import encode_channel, hash_util_encode
+from zerver.lib.url_encoding import encode_channel, encode_hash_component
 from zerver.lib.url_preview.types import UrlEmbedData, UrlOEmbedData
 from zerver.models import Message, Realm, UserProfile
 from zerver.models.linkifiers import linkifiers_for_realm
@@ -2101,7 +2101,7 @@ class StreamTopicPattern(StreamTopicMessageProcessor):
         el.set("class", "stream-topic")
         el.set("data-stream-id", str(stream_id))
         stream_url = encode_channel(stream_id, stream_name)
-        topic_url = hash_util_encode(topic_name)
+        topic_url = encode_hash_component(topic_name)
         channel_topic_object = ChannelTopicInfo(stream_name, topic_name)
         with_operand = self.get_with_operand(channel_topic_object)
         if with_operand is not None:
@@ -2138,7 +2138,7 @@ class StreamTopicMessagePattern(StreamTopicMessageProcessor):
         el = Element("a")
         el.set("class", "message-link")
         stream_url = encode_channel(stream_id, stream_name)
-        topic_url = hash_util_encode(topic_name)
+        topic_url = encode_hash_component(topic_name)
         link = f"/#narrow/channel/{stream_url}/topic/{topic_url}/near/{message_id}"
         el.set("href", link)
 

zerver/lib/topic_link_util.py

@@ -1,8 +1,8 @@
 # Keep this synchronized with web/src/topic_link_util.ts
 
 import re
-import urllib.parse
 
+from zerver.lib.url_encoding import encode_channel, encode_hash_component
 from zerver.models.messages import Message
 
 invalid_stream_topic_regex = re.compile(r"[`>*&\[\]]|(\$\$)")
@@ -31,19 +31,6 @@ def escape_invalid_stream_topic_characters(text: str) -> str:
     )
 
 
-hash_replacements = {
-    "%": ".",
-    "(": ".28",
-    ")": ".29",
-    ".": ".2E",
-}
-
-
-def encode_hash_component(s: str) -> str:
-    encoded = urllib.parse.quote(s, safe="*")
-    return "".join(hash_replacements.get(c, c) for c in encoded)
-
-
 def get_fallback_markdown_link(
     stream_id: int, stream_name: str, topic_name: str | None = None, message_id: int | None = None
 ) -> str:
@@ -55,7 +42,7 @@ def get_fallback_markdown_link(
     render properly due to special characters in the channel or topic name.
     """
     escape = escape_invalid_stream_topic_characters
-    link = f"#narrow/channel/{stream_id}-{encode_hash_component(stream_name.replace(' ', '-'))}"
+    link = f"#narrow/channel/{encode_channel(stream_id, stream_name)}"
     text = f"#{escape(stream_name)}"
     if topic_name is not None:
         link += f"/topic/{encode_hash_component(topic_name)}"

zerver/lib/url_encoding.py

@@ -1,5 +1,6 @@
+import urllib.parse
 from typing import Any
-from urllib.parse import quote, urlsplit
+from urllib.parse import urlsplit
 
 import re2
 
@@ -7,18 +8,37 @@ from zerver.lib.topic import get_topic_from_message_info
 from zerver.lib.types import UserDisplayRecipient
 from zerver.models import Realm, Stream, UserProfile
 
-
+hash_replacements = {
-def hash_util_encode(string: str) -> str:
+    "%": ".",
-    # Do the same encoding operation as shared internal_url.encodeHashComponent
+    "(": ".28",
-    # on the frontend.
+    ")": ".29",
-    # `safe` has a default value of "/", but we want those encoded, too.
+    ".": ".2E",
-    return quote(string, safe=b"").replace(".", "%2E").replace("%", ".")
+}
 
 
-def encode_channel(channel_id: int, channel_name: str) -> str:
+def encode_hash_component(s: str) -> str:
-    # We encode channel for urls as something like 99-Verona.
+    encoded = urllib.parse.quote(s, safe="*")
+    return "".join(hash_replacements.get(c, c) for c in encoded)
+
+
+def encode_channel(channel_id: int, channel_name: str, with_operator: bool = False) -> str:
+    """
+    This encodes the given `channel_id` and `channel_name`
+    into a recipient slug string that can be used to
+    construct a narrow URL.
+
+    e.g., 9, "Verona" -> "99-Verona"
+
+    The `with_operator` parameter decides whether to append
+    the "channel" operator to the recipient slug or not.
+
+    e.g., "channel/99-Verona"
+    """
     channel_name = channel_name.replace(" ", "-")
-    return str(channel_id) + "-" + hash_util_encode(channel_name)
+    encoded_channel = str(channel_id) + "-" + encode_hash_component(channel_name)
+    if with_operator:
+        return f"channel/{encoded_channel}"
+    return encoded_channel
 
 
 def personal_narrow_url(*, realm: Realm, sender: UserProfile) -> str:
@@ -45,9 +65,7 @@ def stream_narrow_url(realm: Realm, stream: Stream) -> str:
 
 def topic_narrow_url(*, realm: Realm, stream: Stream, topic_name: str) -> str:
     base_url = f"{realm.url}/#narrow/channel/"
-    return (
+    return f"{base_url}{encode_channel(stream.id, stream.name)}/topic/{encode_hash_component(topic_name)}"
-        f"{base_url}{encode_channel(stream.id, stream.name)}/topic/{hash_util_encode(topic_name)}"
-    )
 
 
 def message_link_url(
@@ -80,7 +98,7 @@ def stream_message_url(
     stream_id = message["stream_id"]
     stream_name = message["display_recipient"]
     topic_name = get_topic_from_message_info(message)
-    encoded_topic_name = hash_util_encode(topic_name)
+    encoded_topic_name = encode_hash_component(topic_name)
     encoded_stream = encode_channel(stream_id, stream_name)
 
     parts = [

zerver/tests/test_url_encoding.py

@@ -0,0 +1,11 @@
+from zerver.lib.test_classes import ZulipTestCase
+from zerver.lib.url_encoding import encode_channel
+
+
+class URLEncodeTest(ZulipTestCase):
+    def test_encode_channel(self) -> None:
+        # We have more tests for this function in `test_topic_link_utils.py`
+        self.assertEqual(encode_channel(9, "Verona"), "9-Verona")
+        self.assertEqual(encode_channel(123, "General"), "123-General")
+        self.assertEqual(encode_channel(7, "random_channel"), "7-random_channel")
+        self.assertEqual(encode_channel(9, "Verona", with_operator=True), "channel/9-Verona")