markdown: CSS-escape preview links.

This adds `soupsieve` as an explicit dependency, but intentionally
does not adjust the provision version, as it was already an indirect
dependency.

pyproject.toml

@@ -90,6 +90,7 @@ module = [
     "social_core.*",
     "social_django.*",
     "sourcemap.*",
+    "soupsieve.*",
     "sphinx_rtd_theme.*",
     "talon_core.*",
     "tlds.*",

requirements/common.in

@@ -191,3 +191,6 @@ django-cte
 
 # SCIM integration
 django-scim2
+
+# CSS manipulation
+soupsieve

requirements/dev.txt

@@ -1621,7 +1621,9 @@ social-auth-core[azuread,openidconnect,saml]==4.1.0 \
 soupsieve==2.2.1 \
     --hash=sha256:052774848f448cf19c7e959adf5566904d525f33a3f8b6ba6f6f8f26ec7de0cc \
     --hash=sha256:c2c1c2d44f158cdbddab7824a9af8c4f83c76b1e23e049479aa432feb6c4c23b
-    # via beautifulsoup4
+    # via
+    #   -r requirements/common.in
+    #   beautifulsoup4
 sourcemap==0.2.1 \
     --hash=sha256:be00a90185e7a16b87bbe62a68ffd5e38bc438ef4700806d9b90e44d8027787c \
     --hash=sha256:c448a8c48f9482e522e4582106b0c641a83b5dbc7f13927b178848e3ea20967b

requirements/prod.txt

@@ -1118,7 +1118,9 @@ social-auth-core[azuread,openidconnect,saml]==4.1.0 \
 soupsieve==2.2.1 \
     --hash=sha256:052774848f448cf19c7e959adf5566904d525f33a3f8b6ba6f6f8f26ec7de0cc \
     --hash=sha256:c2c1c2d44f158cdbddab7824a9af8c4f83c76b1e23e049479aa432feb6c4c23b
-    # via beautifulsoup4
+    # via
+    #   -r requirements/common.in
+    #   beautifulsoup4
 sourcemap==0.2.1 \
     --hash=sha256:be00a90185e7a16b87bbe62a68ffd5e38bc438ef4700806d9b90e44d8027787c \
     --hash=sha256:c448a8c48f9482e522e4582106b0c641a83b5dbc7f13927b178848e3ea20967b

zerver/lib/markdown/__init__.py

@@ -43,6 +43,7 @@ import requests
 from django.conf import settings
 from markdown.blockparser import BlockParser
 from markdown.extensions import codehilite, nl2br, sane_lists, tables
+from soupsieve import escape as css_escape
 from tlds import tld_set
 from typing_extensions import TypedDict
 
@@ -714,7 +715,7 @@ class InlineInterestingLinkProcessor(markdown.treeprocessors.Treeprocessor):
 
         img_link = get_camo_url(img_link)
         img = SubElement(container, "a")
-        img.set("style", "background-image: url(" + img_link + ")")
+        img.set("style", "background-image: url(" + css_escape(img_link) + ")")
         img.set("href", link)
         img.set("class", "message_embed_image")
 

zerver/tests/test_link_embed.py

@@ -1,3 +1,4 @@
+import re
 from collections import OrderedDict
 from typing import Any, Optional, Union
 from unittest import mock
@@ -524,7 +525,7 @@ class PreviewTestCase(ZulipTestCase):
 
     @override_settings(CAMO_URI="")
     def test_inline_url_embed_preview(self) -> None:
-        with_preview = '<p><a href="http://test.org/">http://test.org/</a></p>\n<div class="message_embed"><a class="message_embed_image" href="http://test.org/" style="background-image: url(http://ia.media-imdb.com/images/rock.jpg)"></a><div class="data-container"><div class="message_embed_title"><a href="http://test.org/" title="The Rock">The Rock</a></div><div class="message_embed_description">Description text</div></div></div>'
+        with_preview = '<p><a href="http://test.org/">http://test.org/</a></p>\n<div class="message_embed"><a class="message_embed_image" href="http://test.org/" style="background-image: url(http\\:\\/\\/ia\\.media-imdb\\.com\\/images\\/rock\\.jpg)"></a><div class="data-container"><div class="message_embed_title"><a href="http://test.org/" title="The Rock">The Rock</a></div><div class="message_embed_description">Description text</div></div></div>'
         without_preview = '<p><a href="http://test.org/">http://test.org/</a></p>'
         msg = self._send_message_with_test_org_url(sender=self.example_user("hamlet"))
         self.assertEqual(msg.rendered_content, with_preview)
@@ -539,7 +540,9 @@ class PreviewTestCase(ZulipTestCase):
         self.assertEqual(msg.rendered_content, without_preview)
 
     def test_inline_url_embed_preview_with_camo(self) -> None:
-        camo_url = get_camo_url("http://ia.media-imdb.com/images/rock.jpg")
+        camo_url = re.sub(
+            r"([^\w-])", r"\\\1", get_camo_url("http://ia.media-imdb.com/images/rock.jpg")
+        )
         with_preview = (
             '<p><a href="http://test.org/">http://test.org/</a></p>\n<div class="message_embed"><a class="message_embed_image" href="http://test.org/" style="background-image: url('
             + camo_url
@@ -548,6 +551,42 @@ class PreviewTestCase(ZulipTestCase):
         msg = self._send_message_with_test_org_url(sender=self.example_user("hamlet"))
         self.assertEqual(msg.rendered_content, with_preview)
 
+    @responses.activate
+    @override_settings(CAMO_URI="")
+    @override_settings(INLINE_URL_EMBED_PREVIEW=True)
+    def test_link_preview_css_escaping_image(self) -> None:
+        user = self.example_user("hamlet")
+        self.login_user(user)
+        url = "http://test.org/"
+        with mock_queue_publish("zerver.lib.actions.queue_json_publish") as patched:
+            msg_id = self.send_stream_message(user, "Scotland", topic_name="foo", content=url)
+            patched.assert_called_once()
+            queue = patched.call_args[0][0]
+            self.assertEqual(queue, "embed_links")
+            event = patched.call_args[0][1]
+
+        # Swap the URL out for one with characters that need CSS escaping
+        html = re.sub(r"rock\.jpg", "rock).jpg", self.open_graph_html)
+        self.create_mock_response(url, body=html)
+        with self.settings(TEST_SUITE=False, CACHES=TEST_CACHES):
+            with self.assertLogs(level="INFO") as info_logs:
+                FetchLinksEmbedData().consume(event)
+            self.assertTrue(
+                "INFO:root:Time spent on get_link_embed_data for http://test.org/: "
+                in info_logs.output[0]
+            )
+
+        msg = Message.objects.select_related("sender").get(id=msg_id)
+        with_preview = (
+            '<p><a href="http://test.org/">http://test.org/</a></p>\n<div class="message_embed"><a class="message_embed_image" href="http://test.org/" style="background-image: url('
+            + "http\\:\\/\\/ia\\.media-imdb\\.com\\/images\\/rock\\)\\.jpg"
+            + ')"></a><div class="data-container"><div class="message_embed_title"><a href="http://test.org/" title="The Rock">The Rock</a></div><div class="message_embed_description">Description text</div></div></div>'
+        )
+        self.assertEqual(
+            with_preview,
+            msg.rendered_content,
+        )
+
     @override_settings(CAMO_URI="")
     @override_settings(INLINE_URL_EMBED_PREVIEW=True)
     def test_inline_relative_url_embed_preview(self) -> None:
@@ -562,7 +601,7 @@ class PreviewTestCase(ZulipTestCase):
 
     @override_settings(CAMO_URI="")
     def test_inline_url_embed_preview_with_relative_image_url(self) -> None:
-        with_preview_relative = '<p><a href="http://test.org/">http://test.org/</a></p>\n<div class="message_embed"><a class="message_embed_image" href="http://test.org/" style="background-image: url(http://test.org/images/rock.jpg)"></a><div class="data-container"><div class="message_embed_title"><a href="http://test.org/" title="The Rock">The Rock</a></div><div class="message_embed_description">Description text</div></div></div>'
+        with_preview_relative = '<p><a href="http://test.org/">http://test.org/</a></p>\n<div class="message_embed"><a class="message_embed_image" href="http://test.org/" style="background-image: url(http\\:\\/\\/test\\.org\\/images\\/rock\\.jpg)"></a><div class="data-container"><div class="message_embed_title"><a href="http://test.org/" title="The Rock">The Rock</a></div><div class="message_embed_description">Description text</div></div></div>'
         # Try case where the Open Graph image is a relative URL.
         msg = self._send_message_with_test_org_url(
             sender=self.example_user("prospero"), relative_url=True
@@ -749,7 +788,7 @@ class PreviewTestCase(ZulipTestCase):
 
         msg = Message.objects.select_related("sender").get(id=msg_id)
         self.assertIn(data["title"], msg.rendered_content)
-        self.assertIn(data["image"], msg.rendered_content)
+        self.assertIn(re.sub(r"([^\w-])", r"\\\1", data["image"]), msg.rendered_content)
 
     @responses.activate
     @override_settings(INLINE_URL_EMBED_PREVIEW=True)