diff --git a/src/core/migrations/0046_alter_sound_file_alter_soundevent_file.py b/src/core/migrations/0046_alter_sound_file_alter_soundevent_file.py new file mode 100644 index 00000000..6673f04b --- /dev/null +++ b/src/core/migrations/0046_alter_sound_file_alter_soundevent_file.py @@ -0,0 +1,24 @@ +# Generated by Django 6.1.1 on 2026-09-17 14:57 + +import core.models +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("core", "0045_alter_artwork_options_alter_exhibit_options_and_more"), + ] + + operations = [ + migrations.AlterField( + model_name="sound", + name="file", + field=models.FileField(upload_to=core.models.sound_file_path), + ), + migrations.AlterField( + model_name="soundevent", + name="file", + field=models.FileField(upload_to=core.models.sound_file_path), + ), + ] diff --git a/src/core/models.py b/src/core/models.py index e0d3be34..3399d0d1 100644 --- a/src/core/models.py +++ b/src/core/models.py @@ -1,4 +1,5 @@ import logging +from uuid import uuid4 import pghistory from django.core.files.base import ContentFile as CF @@ -61,9 +62,26 @@ class SoundExtensions(models.TextChoices): WAV = "wav", "WAV" +def sound_file_path(instance, filename): + """Upload path: sounds/. + + Sound filenames come straight from the user, and names carrying spaces, + accents or reserved characters have caused "file not found" errors once + S3 and Django disagree about how to escape them. The original name is + still recorded on Sound.file_name_original. + + A uuid is used rather than the / layout that objects and markers + use, because the primary key does not exist yet when the upload path is + generated for a new row. + """ + ext = filename.rsplit(".", 1)[-1].lower() if "." in filename else "" + name = uuid4().hex + return f"sounds/{name}.{ext}" if ext else f"sounds/{name}" + + @pghistory.track() class Sound(ContentMixin, models.Model): - file = models.FileField(upload_to="sounds/") + file = models.FileField(upload_to=sound_file_path) title = models.CharField(max_length=50, blank=False) author = models.CharField(max_length=60, blank=False) owner = models.ForeignKey( diff --git a/src/core/tests/test_sound_upload_path.py b/src/core/tests/test_sound_upload_path.py new file mode 100644 index 00000000..5bb316d2 --- /dev/null +++ b/src/core/tests/test_sound_upload_path.py @@ -0,0 +1,64 @@ +import re + +from django.core.files.uploadedfile import SimpleUploadedFile +from django.test import TestCase +from django.urls import reverse + +from core.models import Sound, sound_file_path +from users.models import User + +EXAMPLE_SOUND_PATH = "collection/sounds/happy-message-ping.mp3" +STORED_NAME = re.compile(r"^sounds/[0-9a-f]{32}\.mp3$") + + +class TestSoundUploadPath(TestCase): + """Sound filenames come from the user; they must not reach storage as-is.""" + + def setUp(self): + self.username = "testuser" + self.password = "testpassword" + User.objects.create_user(username=self.username, password=self.password) + self.client.login(username=self.username, password=self.password) + + def upload(self, filename): + with open(EXAMPLE_SOUND_PATH, "rb") as handle: + payload = handle.read() + return self.client.post( + reverse("sound-upload"), + { + "file": SimpleUploadedFile( + filename, payload, content_type="audio/mpeg" + ), + "title": filename, + "author": "Test Author", + }, + ) + + def test_hostile_filename_never_reaches_storage(self): + self.upload("Música #1 (versão final?) 100%.mp3") + sound = Sound.objects.get(title="Música #1 (versão final?) 100%.mp3") + + assert STORED_NAME.match(sound.file.name), sound.file.name + + def test_original_filename_is_still_recorded(self): + self.upload("Música #1 (versão final?) 100%.mp3") + sound = Sound.objects.get(title="Música #1 (versão final?) 100%.mp3") + + assert sound.file_name_original.endswith(".mp3") + assert sound.file_extension == "mp3" + + def test_two_uploads_of_the_same_name_do_not_collide(self): + self.upload("same-name.mp3") + self.upload("same-name.mp3") + names = {sound.file.name for sound in Sound.objects.all()} + + assert len(names) == Sound.objects.count() + + def test_path_helper_lowercases_and_keeps_the_extension(self): + assert sound_file_path(None, "Whatever.MP3").endswith(".mp3") + assert STORED_NAME.match(sound_file_path(None, "a b/c?.mp3")) + + def test_path_helper_tolerates_a_name_without_extension(self): + generated = sound_file_path(None, "noextension") + + assert re.match(r"^sounds/[0-9a-f]{32}$", generated), generated