From 2523108b6a90549bce96161d230a82a0567d2dcd Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Mon, 10 Jun 2019 14:33:46 +0200
Subject: [PATCH] Fix #847: Use ASCII filename before upload to S3 to avoid
 playback issues

---
 api/config/settings/common.py       |  2 +-
 api/funkwhale_api/common/storage.py | 16 ++++++++++++----
 api/requirements/base.txt           |  1 +
 api/tests/common/test_storages.py   | 28 ++++++++++++++++++++++++++++
 changes/changelog.d/847.bugfix      |  1 +
 5 files changed, 43 insertions(+), 5 deletions(-)
 create mode 100644 api/tests/common/test_storages.py
 create mode 100644 changes/changelog.d/847.bugfix

diff --git a/api/config/settings/common.py b/api/config/settings/common.py
index 5d6536686..ab060b448 100644
--- a/api/config/settings/common.py
+++ b/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")),)
diff --git a/api/funkwhale_api/common/storage.py b/api/funkwhale_api/common/storage.py
index c5651693f..07d6a7cc3 100644
--- a/api/funkwhale_api/common/storage.py
+++ b/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))
diff --git a/api/requirements/base.txt b/api/requirements/base.txt
index c72c20374..8372adc47 100644
--- a/api/requirements/base.txt
+++ b/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
diff --git a/api/tests/common/test_storages.py b/api/tests/common/test_storages.py
new file mode 100644
index 000000000..febe5df70
--- /dev/null
+++ b/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)
diff --git a/changes/changelog.d/847.bugfix b/changes/changelog.d/847.bugfix
new file mode 100644
index 000000000..d15646721
--- /dev/null
+++ b/changes/changelog.d/847.bugfix
@@ -0,0 +1 @@
+Use ASCII filename before upload to S3 to avoid playback issues (#847)