support SVG for product and topic images (#5430)

kitsune/products/migrations/0006_alter_product_and_topic_images.py

@@ -0,0 +1,42 @@
+# Generated by Django 4.1.7 on 2023-03-31 16:04
+
+from django.db import migrations
+import kitsune.sumo.fields
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("products", "0001_squashed_0005_auto_20200629_0826"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="product",
+            name="image",
+            field=kitsune.sumo.fields.ImagePlusField(
+                blank=True,
+                help_text="Used on the the home page. Must be 484x244.",
+                max_length=250,
+                null=True,
+                upload_to="uploads/products/",
+            ),
+        ),
+        migrations.AlterField(
+            model_name="product",
+            name="image_alternate",
+            field=kitsune.sumo.fields.ImagePlusField(
+                blank=True,
+                help_text="Used everywhere except the home page. Must be 96x96.",
+                max_length=250,
+                null=True,
+                upload_to="uploads/products/",
+            ),
+        ),
+        migrations.AlterField(
+            model_name="topic",
+            name="image",
+            field=kitsune.sumo.fields.ImagePlusField(
+                blank=True, max_length=250, null=True, upload_to="uploads/topics/"
+            ),
+        ),
+    ]

kitsune/products/models.py

@@ -2,6 +2,7 @@
 from django.db import models
 from django.utils.translation import gettext_lazy as _lazy
 
+from kitsune.sumo.fields import ImagePlusField
 from kitsune.sumo.models import ModelBase
 from kitsune.sumo.urlresolvers import reverse
 from kitsune.sumo.utils import webpack_static
@@ -19,15 +20,15 @@ class Product(ModelBase):
     codename = models.CharField(max_length=255, blank=True, default="")
     slug = models.SlugField()
     description = models.TextField()
-    image = models.ImageField(
+    image = ImagePlusField(
         upload_to=settings.PRODUCT_IMAGE_PATH,
         null=True,
         blank=True,
         max_length=settings.MAX_FILEPATH_LENGTH,
         # no l10n in admin
         help_text="Used on the the home page. Must be 484x244.",
     )
-    image_alternate = models.ImageField(
+    image_alternate = ImagePlusField(
         upload_to=settings.PRODUCT_IMAGE_PATH,
         null=True,
         blank=True,
@@ -86,7 +87,7 @@ class Topic(ModelBase):
     # We don't use a SlugField here because it isn't unique by itself.
     slug = models.CharField(max_length=255, db_index=True)
     description = models.TextField()
-    image = models.ImageField(
+    image = ImagePlusField(
         upload_to=settings.TOPIC_IMAGE_PATH,
         null=True,
         blank=True,

kitsune/sumo/fields.py

@@ -0,0 +1,17 @@
+from django.db import models
+
+from kitsune.sumo import form_fields
+
+
+class ImagePlusField(models.ImageField):
+    """
+    Same as models.ImageField but with support for SVG images as well.
+    """
+
+    def formfield(self, **kwargs):
+        return super().formfield(
+            **{
+                "form_class": form_fields.ImagePlusField,
+                **kwargs,
+            }
+        )

kitsune/sumo/form_fields.py

@@ -1,3 +1,6 @@
+from pathlib import Path
+
+from cairosvg import svg2svg
 from django import forms
 from django.contrib.auth.models import User
 from django.core import validators
@@ -79,3 +82,64 @@ def to_python(self, value):
                     raise forms.ValidationError(msg.format(username=username))
 
         return users
+
+
+class ImagePlusField(forms.ImageField):
+    """
+    Same as django.forms.ImageField but with support for SVG images as well.
+    """
+
+    default_validators = [
+        validators.FileExtensionValidator(
+            allowed_extensions=validators.get_available_image_extensions() + ["svg"]
+        )
+    ]
+
+    def to_python(self, data):
+        """
+        Check that the file-upload field data contains an image that
+        Pillow supports or a valid SVG image.
+        """
+        try:
+            return super().to_python(data)
+        except ValidationError as verr:
+            if (getattr(verr, "code", None) != "invalid_image") or (
+                Path(data.name).suffix.lower() != ".svg"
+            ):
+                raise
+
+        def scrub(svg_as_bytes):
+            """
+            Accepts an SVG file as bytes and returns a safe version of that
+            SVG file as bytes.
+            """
+            try:
+                return svg2svg(bytestring=svg_as_bytes)
+            except Exception as exc:
+                # CairoSVG doesn't recognize it as an SVG image.
+                msg = _("Invalid or unsupported SVG image: {reason}")
+                raise ValidationError(
+                    msg.format(reason=str(exc)),
+                    code="invalid_svg_image",
+                ) from exc
+
+        if hasattr(data, "read"):
+            # This is typically an instance of a sub-class of UploadedFile,
+            # which shouldn't be closed, otherwise it will be deleted.
+            data.seek(0)
+            try:
+                scrubbed = scrub(data.read())
+            finally:
+                # The read pointer is expected to point to the start of the file.
+                data.seek(0)
+            try:
+                # Over-write the image with its scrubbed version.
+                data.truncate()
+                data.write(scrubbed)
+            finally:
+                # The read pointer is expected to point to the start of the file.
+                data.seek(0)
+        else:
+            data["content"] = scrub(data["content"])
+
+        return data

kitsune/sumo/tests/test_form_fields.py

@@ -1,6 +1,7 @@
 from django.core.exceptions import ValidationError
+from django.core.files.uploadedfile import SimpleUploadedFile, TemporaryUploadedFile
 
-from kitsune.sumo.form_fields import TypedMultipleChoiceField
+from kitsune.sumo.form_fields import ImagePlusField, TypedMultipleChoiceField
 from kitsune.sumo.tests import TestCase
 
 
@@ -68,3 +69,79 @@ def test_coerce_only(self):
         """No validation error raised in this case."""
         f = TypedMultipleChoiceField(choices=[(1, "+1")], coerce=int, coerce_only=True)
         self.assertEqual([], f.clean(["2"]))
+
+
+class ImagePlusFieldTestCases(TestCase):
+    """
+    Test cases for kitsune.sumo.form_fields.ImagePlusField, which accepts SVG images
+    as well as the images accepted by django.forms.ImageField.
+    """
+
+    def get_uploaded_file(self, name, kind="simple", content=None):
+        if content is None:
+            content = b"""
+                <svg xmlns="http://www.w3.org/2000/svg" width="100" height="100">
+                    <rect x="10" y="10" width="80" height="80" fill="blue" />
+                </svg>
+            """
+        content_type = "image/svg+xml"
+
+        if kind == "simple":
+            return SimpleUploadedFile(name, content, content_type)
+
+        data = TemporaryUploadedFile(name, content_type, len(content), None)
+        data.open("wb").write(content)
+        return data
+
+    def test_svg_image_with_temp_file(self):
+        """Test for the case when the uploaded file is a named temporary file instance."""
+        field = ImagePlusField()
+        data = self.get_uploaded_file("stuff.svg", "temp")
+        self.assertEqual(field.clean(data), data)
+
+    def test_svg_image_with_in_memory_file(self):
+        """Test for the case when the uploaded file is an in-memory file instance."""
+        field = ImagePlusField()
+        data = self.get_uploaded_file("stuff.svg")
+        self.assertEqual(field.clean(data), data)
+
+    def test_svg_image_with_unsafe_file(self):
+        """Test for the case when the uploaded file is unsafe."""
+        field = ImagePlusField()
+        data = self.get_uploaded_file(
+            "stuff.svg",
+            content=b"""
+            <svg xmlns="http://www.w3.org/2000/svg" width="100" height="100">
+                <script>alert('This is an unsafe SVG file!');</script>
+                <rect x="10" y="10" width="80" height="80" fill="blue" />
+            </svg>""",
+        )
+        self.assertEqual(field.clean(data), data)
+        content = data.read()
+        self.assertIn(b'<svg xmlns="http://www.w3.org/2000/svg"', content)
+        self.assertNotIn(b"<script>", content)
+
+    def test_svg_image_without_proper_extension(self):
+        """SVG images without an "svg" extension should be considered invalid."""
+        field = ImagePlusField()
+        data = self.get_uploaded_file("stuff")
+
+        with self.assertRaises(ValidationError) as arm:
+            field.clean(data)
+
+        self.assertTrue(hasattr(arm.exception, "code"))
+        self.assertEqual(arm.exception.code, "invalid_image")
+
+    def test_invalid_svg_image(self):
+        """Invalid SVG images should raise a validation error."""
+        field = ImagePlusField()
+        data = self.get_uploaded_file(
+            "stuff.svg", content=b"""<svg xmlns="http://www.w3.org/2000/svg"></svg>"""
+        )
+
+        with self.assertRaises(ValidationError) as arm:
+            field.clean(data)
+
+        self.assertTrue(hasattr(arm.exception, "code"))
+        self.assertEqual(arm.exception.code, "invalid_svg_image")
+        self.assertIn("The SVG size is undefined", str(arm.exception))

pyproject.toml

@@ -88,6 +88,7 @@ psycopg2 = "^2.9.9"
 mkdocs = "^1.5.3"
 mkdocs-material = "^9.5.3"
 dockerflow = "^2022.8.0"
+cairosvg = "^2.7.1"
 
 [tool.poetry.group.dev.dependencies]
 ipdb = "^0.13.11"