Do not escape valid characters in custom CSS (#863)

bookmarks/migrations/0042_userprofile_custom_css_hash.py

@@ -0,0 +1,18 @@
+# Generated by Django 5.1.1 on 2024-09-28 08:03
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("bookmarks", "0041_merge_metadata"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="userprofile",
+            name="custom_css_hash",
+            field=models.CharField(blank=True, max_length=32),
+        ),
+    ]

bookmarks/models.py

@@ -1,4 +1,5 @@
 import binascii
+import hashlib
 import logging
 import os
 from typing import List
@@ -430,6 +431,7 @@ class UserProfile(models.Model):
     display_remove_bookmark_action = models.BooleanField(default=True, null=False)
     permanent_notes = models.BooleanField(default=False, null=False)
     custom_css = models.TextField(blank=True, null=False)
+    custom_css_hash = models.CharField(blank=True, null=False, max_length=32)
     auto_tagging_rules = models.TextField(blank=True, null=False)
     search_preferences = models.JSONField(default=dict, null=False)
     enable_automatic_html_snapshots = models.BooleanField(default=True, null=False)
@@ -439,6 +441,15 @@ class UserProfile(models.Model):
     )
     sticky_pagination = models.BooleanField(default=False, null=False)
 
+    def save(self, *args, **kwargs):
+        if self.custom_css:
+            self.custom_css_hash = hashlib.md5(
+                self.custom_css.encode("utf-8")
+            ).hexdigest()
+        else:
+            self.custom_css_hash = ""
+        super().save(*args, **kwargs)
+
 
 class UserProfileForm(forms.ModelForm):
     class Meta:

bookmarks/templates/bookmarks/head.html

@@ -30,7 +30,7 @@
     <meta name="theme-color" media="(prefers-color-scheme: light)" content="#5856e0">
   {% endif %}
   {% if request.user_profile.custom_css %}
-    <style>{{ request.user_profile.custom_css }}</style>
+    <link href="{% url 'bookmarks:custom_css' %}?hash={{ request.user_profile.custom_css_hash }}" rel="stylesheet" type="text/css"/>
   {% endif %}
   <meta name="turbo-cache-control" content="no-preview">
   {% if not request.global_settings.enable_link_prefetch %}

bookmarks/tests/test_custom_css.py

@@ -1,21 +0,0 @@
-from django.test import TestCase
-from django.urls import reverse
-
-from bookmarks.tests.helpers import BookmarkFactoryMixin
-
-
-class CustomCssTestCase(TestCase, BookmarkFactoryMixin):
-    def setUp(self):
-        self.client.force_login(self.get_or_create_test_user())
-
-    def test_does_not_render_custom_style_tag_by_default(self):
-        response = self.client.get(reverse("bookmarks:index"))
-        self.assertNotContains(response, "<style>")
-
-    def test_renders_custom_style_tag_if_user_has_custom_css(self):
-        profile = self.get_or_create_test_user().profile
-        profile.custom_css = "body { background-color: red; }"
-        profile.save()
-
-        response = self.client.get(reverse("bookmarks:index"))
-        self.assertContains(response, "<style>body { background-color: red; }</style>")

bookmarks/tests/test_custom_css_view.py

@@ -0,0 +1,28 @@
+from django.test import TestCase
+from django.urls import reverse
+
+from bookmarks.tests.helpers import BookmarkFactoryMixin
+
+
+class CustomCssViewTestCase(TestCase, BookmarkFactoryMixin):
+    def setUp(self) -> None:
+        user = self.get_or_create_test_user()
+        self.client.force_login(user)
+
+    def test_with_empty_css(self):
+        response = self.client.get(reverse("bookmarks:custom_css"))
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response["Content-Type"], "text/css")
+        self.assertEqual(response.headers["Cache-Control"], "public, max-age=2592000")
+        self.assertEqual(response.content.decode(), "")
+
+    def test_with_custom_css(self):
+        css = "body { background-color: red; }"
+        self.user.profile.custom_css = css
+        self.user.profile.save()
+
+        response = self.client.get(reverse("bookmarks:custom_css"))
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response["Content-Type"], "text/css")
+        self.assertEqual(response.headers["Cache-Control"], "public, max-age=2592000")
+        self.assertEqual(response.content.decode(), css)

bookmarks/tests/test_layout.py

@@ -2,10 +2,10 @@ from django.test import TestCase
 from django.urls import reverse
 
 from bookmarks.models import GlobalSettings
-from bookmarks.tests.helpers import BookmarkFactoryMixin
+from bookmarks.tests.helpers import BookmarkFactoryMixin, HtmlTestMixin
 
 
-class LayoutTestCase(TestCase, BookmarkFactoryMixin):
+class LayoutTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin):
 
     def setUp(self) -> None:
         user = self.get_or_create_test_user()
@@ -63,3 +63,38 @@ class LayoutTestCase(TestCase, BookmarkFactoryMixin):
             html,
             count=0,
         )
+
+    def test_does_not_link_custom_css_when_empty(self):
+        response = self.client.get(reverse("bookmarks:index"))
+        html = response.content.decode()
+        soup = self.make_soup(html)
+
+        link = soup.select_one("link[rel='stylesheet'][href*='custom_css']")
+        self.assertIsNone(link)
+
+    def test_does_link_custom_css_when_not_empty(self):
+        profile = self.get_or_create_test_user().profile
+        profile.custom_css = "body { background-color: red; }"
+        profile.save()
+
+        response = self.client.get(reverse("bookmarks:index"))
+        html = response.content.decode()
+        soup = self.make_soup(html)
+
+        link = soup.select_one("link[rel='stylesheet'][href*='custom_css']")
+        self.assertIsNotNone(link)
+
+    def test_custom_css_link_href(self):
+        profile = self.get_or_create_test_user().profile
+        profile.custom_css = "body { background-color: red; }"
+        profile.save()
+
+        response = self.client.get(reverse("bookmarks:index"))
+        html = response.content.decode()
+        soup = self.make_soup(html)
+
+        link = soup.select_one("link[rel='stylesheet'][href*='custom_css']")
+        expected_url = (
+            reverse("bookmarks:custom_css") + f"?hash={profile.custom_css_hash}"
+        )
+        self.assertEqual(link["href"], expected_url)

bookmarks/tests/test_settings_general_view.py

@@ -1,3 +1,4 @@
+import hashlib
 import random
 from unittest.mock import patch, Mock
 
@@ -217,6 +218,31 @@ class SettingsGeneralViewTestCase(TestCase, BookmarkFactoryMixin):
         self.assertEqual(self.user.profile.theme, UserProfile.THEME_AUTO)
         self.assertSuccessMessage(html, "Profile updated", count=0)
 
+    def test_update_profile_updates_custom_css_hash(self):
+        form_data = self.create_profile_form_data(
+            {
+                "custom_css": "body { background-color: #000; }",
+            }
+        )
+        self.client.post(reverse("bookmarks:settings.update"), form_data, follow=True)
+        self.user.profile.refresh_from_db()
+
+        expected_hash = hashlib.md5(form_data["custom_css"].encode("utf-8")).hexdigest()
+        self.assertEqual(expected_hash, self.user.profile.custom_css_hash)
+
+        form_data["custom_css"] = "body { background-color: #fff; }"
+        self.client.post(reverse("bookmarks:settings.update"), form_data, follow=True)
+        self.user.profile.refresh_from_db()
+
+        expected_hash = hashlib.md5(form_data["custom_css"].encode("utf-8")).hexdigest()
+        self.assertEqual(expected_hash, self.user.profile.custom_css_hash)
+
+        form_data["custom_css"] = ""
+        self.client.post(reverse("bookmarks:settings.update"), form_data, follow=True)
+        self.user.profile.refresh_from_db()
+
+        self.assertEqual("", self.user.profile.custom_css_hash)
+
     def test_enable_favicons_should_schedule_icon_update(self):
         with patch.object(
             tasks, "schedule_bookmarks_without_favicons"

bookmarks/urls.py

@@ -65,4 +65,6 @@ urlpatterns = [
     path("health", views.health, name="health"),
     # Manifest
     path("manifest.json", views.manifest, name="manifest"),
+    # Custom CSS
+    path("custom_css", views.custom_css, name="custom_css"),
 ]

bookmarks/views/__init__.py

@@ -4,4 +4,5 @@ from .settings import *
 from .toasts import *
 from .health import health
 from .manifest import manifest
+from .custom_css import custom_css
 from .root import root

bookmarks/views/custom_css.py

@@ -0,0 +1,10 @@
+from django.http import HttpResponse
+
+custom_css_cache_max_age = 2592000  # 30 days
+
+
+def custom_css(request):
+    css = request.user_profile.custom_css
+    response = HttpResponse(css, content_type="text/css")
+    response["Cache-Control"] = f"public, max-age={custom_css_cache_max_age}"
+    return response