Merge branch '847-ascii-rename-s3' into 'master'

Fix #847: Use ASCII filename before upload to S3 to avoid playback issues

See merge request funkwhale/funkwhale!778

api/config/settings/common.py

@@ -323,7 +323,7 @@ if AWS_ACCESS_KEY_ID:
     AWS_S3_REGION_NAME = env("AWS_S3_REGION_NAME", default=None)
     AWS_S3_SIGNATURE_VERSION = "s3v4"
     AWS_LOCATION = env("AWS_LOCATION", default="")
-    DEFAULT_FILE_STORAGE = "storages.backends.s3boto3.S3Boto3Storage"
+    DEFAULT_FILE_STORAGE = "funkwhale_api.common.storage.ASCIIS3Boto3Storage"
 
 # See: https://docs.djangoproject.com/en/dev/ref/contrib/staticfiles/#std:setting-STATICFILES_DIRS
 STATICFILES_DIRS = (str(APPS_DIR.path("static")),)

api/funkwhale_api/common/storage.py

@@ -1,13 +1,21 @@
-import unicodedata
+import slugify
 
 from django.core.files.storage import FileSystemStorage
+from storages.backends.s3boto3 import S3Boto3Storage
 
 
-class ASCIIFileSystemStorage(FileSystemStorage):
+def asciionly(name):
     """
     Convert unicode characters in name to ASCII characters.
     """
+    return slugify.slugify(name, ok=slugify.SLUG_OK + ".", only_ascii=True)
 
+
+class ASCIIFileSystemStorage(FileSystemStorage):
     def get_valid_name(self, name):
-        name = unicodedata.normalize("NFKD", name).encode("ascii", "ignore")
-        return super().get_valid_name(name)
+        return super().get_valid_name(asciionly(name))
+
+
+class ASCIIS3Boto3Storage(S3Boto3Storage):
+    def get_valid_name(self, name):
+        return super().get_valid_name(asciionly(name))

api/requirements/base.txt

@@ -69,3 +69,4 @@ autobahn>=19.3.3
 django-oauth-toolkit==1.2
 django-storages==1.7.1
 boto3<3
+unicode-slugify

api/tests/common/test_storages.py

@@ -0,0 +1,28 @@
+import pytest
+
+from funkwhale_api.common import storage
+
+
+@pytest.mark.parametrize(
+    "filename, expected",
+    [("집으로 가는 길.mp3", "jibeuro-ganeun-gil.mp3"), ("éàe*$i$.ogg", "eaei.ogg")],
+)
+def test_asciionly(filename, expected):
+    assert storage.asciionly(filename) == expected
+
+
+@pytest.mark.parametrize(
+    "storage_class, parent_class",
+    [
+        (storage.ASCIIFileSystemStorage, storage.FileSystemStorage),
+        (storage.ASCIIS3Boto3Storage, storage.S3Boto3Storage),
+    ],
+)
+def test_ascii_storage_call_asciionly(storage_class, parent_class, mocker):
+    """Cf #847"""
+    asciionly = mocker.patch.object(storage, "asciionly")
+    parent_get_valid_filename = mocker.patch.object(parent_class, "get_valid_name")
+    st = storage_class()
+    assert st.get_valid_name("test") == parent_get_valid_filename.return_value
+    asciionly.assert_called_once_with("test")
+    parent_get_valid_filename.assert_called_once_with(asciionly.return_value)

changes/changelog.d/847.bugfix

@@ -0,0 +1 @@
+Use ASCII filename before upload to S3 to avoid playback issues (#847)