diff --git a/kitsune/products/migrations/0006_alter_product_and_topic_images.py b/kitsune/products/migrations/0006_alter_product_and_topic_images.py new file mode 100644 index 00000000000..0a687b39c63 --- /dev/null +++ b/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/" + ), + ), + ] diff --git a/kitsune/products/models.py b/kitsune/products/models.py index 94fc59b8540..2f493f7d262 100644 --- a/kitsune/products/models.py +++ b/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,7 +20,7 @@ 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, @@ -27,7 +28,7 @@ class Product(ModelBase): # 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, diff --git a/kitsune/sumo/fields.py b/kitsune/sumo/fields.py new file mode 100644 index 00000000000..671f80a0010 --- /dev/null +++ b/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, + } + ) diff --git a/kitsune/sumo/form_fields.py b/kitsune/sumo/form_fields.py index 14b8f686c30..58383e808ff 100644 --- a/kitsune/sumo/form_fields.py +++ b/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 diff --git a/kitsune/sumo/tests/test_form_fields.py b/kitsune/sumo/tests/test_form_fields.py index f316bb36b49..1393b4ed3f8 100644 --- a/kitsune/sumo/tests/test_form_fields.py +++ b/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""" + + + + """ + 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""" + + + + """, + ) + self.assertEqual(field.clean(data), data) + content = data.read() + self.assertIn(b'", 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"""""" + ) + + 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)) diff --git a/poetry.lock b/poetry.lock index a1d91256323..5b60f4c9329 100644 --- a/poetry.lock +++ b/poetry.lock @@ -317,6 +317,47 @@ files = [ {file = "cachetools-5.3.2.tar.gz", hash = "sha256:086ee420196f7b2ab9ca2db2520aca326318b68fe5ba8bc4d49cca91add450f2"}, ] +[[package]] +name = "cairocffi" +version = "1.6.1" +description = "cffi-based cairo bindings for Python" +optional = false +python-versions = ">=3.7" +files = [ + {file = "cairocffi-1.6.1-py3-none-any.whl", hash = "sha256:aa78ee52b9069d7475eeac457389b6275aa92111895d78fbaa2202a52dac112e"}, + {file = "cairocffi-1.6.1.tar.gz", hash = "sha256:78e6bbe47357640c453d0be929fa49cd05cce2e1286f3d2a1ca9cbda7efdb8b7"}, +] + +[package.dependencies] +cffi = ">=1.1.0" + +[package.extras] +doc = ["sphinx", "sphinx_rtd_theme"] +test = ["flake8", "isort", "numpy", "pikepdf", "pytest"] +xcb = ["xcffib (>=1.4.0)"] + +[[package]] +name = "cairosvg" +version = "2.7.1" +description = "A Simple SVG Converter based on Cairo" +optional = false +python-versions = ">=3.5" +files = [ + {file = "CairoSVG-2.7.1-py3-none-any.whl", hash = "sha256:8a5222d4e6c3f86f1f7046b63246877a63b49923a1cd202184c3a634ef546b3b"}, + {file = "CairoSVG-2.7.1.tar.gz", hash = "sha256:432531d72347291b9a9ebfb6777026b607563fd8719c46ee742db0aef7271ba0"}, +] + +[package.dependencies] +cairocffi = "*" +cssselect2 = "*" +defusedxml = "*" +pillow = "*" +tinycss2 = "*" + +[package.extras] +doc = ["sphinx", "sphinx-rtd-theme"] +test = ["flake8", "isort", "pytest"] + [[package]] name = "celery" version = "5.2.7" @@ -711,6 +752,25 @@ files = [ {file = "cssselect-1.2.0.tar.gz", hash = "sha256:666b19839cfaddb9ce9d36bfe4c969132c647b92fc9088c4e23f786b30f1b3dc"}, ] +[[package]] +name = "cssselect2" +version = "0.7.0" +description = "CSS selectors for Python ElementTree" +optional = false +python-versions = ">=3.7" +files = [ + {file = "cssselect2-0.7.0-py3-none-any.whl", hash = "sha256:fd23a65bfd444595913f02fc71f6b286c29261e354c41d722ca7a261a49b5969"}, + {file = "cssselect2-0.7.0.tar.gz", hash = "sha256:1ccd984dab89fc68955043aca4e1b03e0cf29cad9880f6e28e3ba7a74b14aa5a"}, +] + +[package.dependencies] +tinycss2 = "*" +webencodings = "*" + +[package.extras] +doc = ["sphinx", "sphinx_rtd_theme"] +test = ["flake8", "isort", "pytest"] + [[package]] name = "cssutils" version = "2.9.0" @@ -764,6 +824,17 @@ files = [ {file = "decorator-5.1.1.tar.gz", hash = "sha256:637996211036b6385ef91435e4fae22989472f9d571faba8927ba8253acbc330"}, ] +[[package]] +name = "defusedxml" +version = "0.7.1" +description = "XML bomb protection for Python stdlib modules" +optional = false +python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*" +files = [ + {file = "defusedxml-0.7.1-py2.py3-none-any.whl", hash = "sha256:a352e7e428770286cc899e2542b6cdaedb2b4953ff269a210103ec58f6198a61"}, + {file = "defusedxml-0.7.1.tar.gz", hash = "sha256:1bb3032db185915b62d7c6209c5a8792be6a32ab2fedacc84e01b52c51aa3e69"}, +] + [[package]] name = "dennis" version = "1.1.0" @@ -2920,8 +2991,6 @@ files = [ {file = "psycopg2-2.9.9-cp310-cp310-win_amd64.whl", hash = "sha256:426f9f29bde126913a20a96ff8ce7d73fd8a216cfb323b1f04da402d452853c3"}, {file = "psycopg2-2.9.9-cp311-cp311-win32.whl", hash = "sha256:ade01303ccf7ae12c356a5e10911c9e1c51136003a9a1d92f7aa9d010fb98372"}, {file = "psycopg2-2.9.9-cp311-cp311-win_amd64.whl", hash = "sha256:121081ea2e76729acfb0673ff33755e8703d45e926e416cb59bae3a86c6a4981"}, - {file = "psycopg2-2.9.9-cp312-cp312-win32.whl", hash = "sha256:d735786acc7dd25815e89cc4ad529a43af779db2e25aa7c626de864127e5a024"}, - {file = "psycopg2-2.9.9-cp312-cp312-win_amd64.whl", hash = "sha256:a7653d00b732afb6fc597e29c50ad28087dcb4fbfb28e86092277a559ae4e693"}, {file = "psycopg2-2.9.9-cp37-cp37m-win32.whl", hash = "sha256:5e0d98cade4f0e0304d7d6f25bbfbc5bd186e07b38eac65379309c4ca3193efa"}, {file = "psycopg2-2.9.9-cp37-cp37m-win_amd64.whl", hash = "sha256:7e2dacf8b009a1c1e843b5213a87f7c544b2b042476ed7755be813eaf4e8347a"}, {file = "psycopg2-2.9.9-cp38-cp38-win32.whl", hash = "sha256:ff432630e510709564c01dafdbe996cb552e0b9f3f065eb89bdce5bd31fabf4c"}, @@ -4794,4 +4863,4 @@ testing = ["coverage (>=5.0.3)", "zope.event", "zope.testing"] [metadata] lock-version = "2.0" python-versions = "^3.11" -content-hash = "e7b143e9570caf624894097043230b440a5b4472d6ce6ac07bee2516c41882a8" +content-hash = "2daf19c72d71fdd098f767b4307d32db2db2fe8b96a2e1cf57ccabb02558da33" diff --git a/pyproject.toml b/pyproject.toml index 7fcf21e747b..4f3a9b6d138 100644 --- a/pyproject.toml +++ b/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"