diff --git a/api/funkwhale_api/common/utils.py b/api/funkwhale_api/common/utils.py
index 57bcba932f89008be946ea62e429659c45a12426..7763e9b7f810090018fb9845a7fb390df812c0d9 100644
--- a/api/funkwhale_api/common/utils.py
+++ b/api/funkwhale_api/common/utils.py
@@ -113,6 +113,9 @@ def chunk_queryset(source_qs, chunk_size):
def join_url(start, end):
+ if end.startswith("http://") or end.startswith("https://"):
+ # alread a full URL, joining makes no sense
+ return end
if start.endswith("/") and end.startswith("/"):
return start + end[1:]
diff --git a/api/funkwhale_api/music/metadata.py b/api/funkwhale_api/music/metadata.py
index 387b8ffe7de425ecca879999e39d671d619caff8..55044dbc7f98e88d2decc1c756a7f0488bcfdac9 100644
--- a/api/funkwhale_api/music/metadata.py
+++ b/api/funkwhale_api/music/metadata.py
@@ -481,14 +481,26 @@ class PermissiveDateField(serializers.CharField):
return None
+class MBIDField(serializers.UUIDField):
+ def __init__(self, *args, **kwargs):
+ kwargs.setdefault("allow_null", True)
+ kwargs.setdefault("required", False)
+ super().__init__(*args, **kwargs)
+
+ def to_internal_value(self, v):
+ if v in ["", None]:
+ return None
+ return super().to_internal_value(v)
+
+
class ArtistSerializer(serializers.Serializer):
name = serializers.CharField()
- mbid = serializers.UUIDField(required=False, allow_null=True)
+ mbid = MBIDField()
class AlbumSerializer(serializers.Serializer):
title = serializers.CharField()
- mbid = serializers.UUIDField(required=False, allow_null=True)
+ mbid = MBIDField()
release_date = PermissiveDateField(required=False, allow_null=True)
@@ -512,16 +524,35 @@ class PositionField(serializers.CharField):
class TrackMetadataSerializer(serializers.Serializer):
title = serializers.CharField()
- position = PositionField(allow_null=True, required=False)
- disc_number = PositionField(allow_null=True, required=False)
- copyright = serializers.CharField(allow_null=True, required=False)
- license = serializers.CharField(allow_null=True, required=False)
- mbid = serializers.UUIDField(allow_null=True, required=False)
+ position = PositionField(allow_blank=True, allow_null=True, required=False)
+ disc_number = PositionField(allow_blank=True, allow_null=True, required=False)
+ copyright = serializers.CharField(allow_blank=True, allow_null=True, required=False)
+ license = serializers.CharField(allow_blank=True, allow_null=True, required=False)
+ mbid = MBIDField()
album = AlbumField()
artists = ArtistField()
cover_data = CoverDataField()
+ remove_blank_null_fields = [
+ "copyright",
+ "license",
+ "position",
+ "disc_number",
+ "mbid",
+ ]
+
+ def validate(self, validated_data):
+ validated_data = super().validate(validated_data)
+ for field in self.remove_blank_null_fields:
+ try:
+ v = validated_data[field]
+ except KeyError:
+ continue
+ if v in ["", None]:
+ validated_data.pop(field)
+ return validated_data
+
class FakeMetadata(Mapping):
def __init__(self, data, picture=None):
diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py
index 309eb1266dd5fd8db1446ff8981a89de4a1fecf9..cf6a0b7ba310cbbe7354a20072de3a6f352333e4 100644
--- a/api/funkwhale_api/music/models.py
+++ b/api/funkwhale_api/music/models.py
@@ -870,7 +870,14 @@ class UploadVersion(models.Model):
@property
def filename(self):
- return self.upload.filename
+ try:
+ return (
+ self.upload.track.full_name
+ + "."
+ + utils.MIMETYPE_TO_EXTENSION[self.mimetype]
+ )
+ except KeyError:
+ return self.upload.filename
@property
def audio_file_path(self):
diff --git a/api/funkwhale_api/music/views.py b/api/funkwhale_api/music/views.py
index 391a4b333fe45628626383c4479d4a952c9df4b5..4f8f35ced2b2e7a0f6319b568fad511fb48dd711 100644
--- a/api/funkwhale_api/music/views.py
+++ b/api/funkwhale_api/music/views.py
@@ -285,6 +285,11 @@ def should_transcode(upload, format, max_bitrate=None):
return format_need_transcoding or bitrate_need_transcoding
+def get_content_disposition(filename):
+ filename = "filename*=UTF-8''{}".format(urllib.parse.quote(filename))
+ return "attachment; {}".format(filename)
+
+
def handle_serve(upload, user, format=None, max_bitrate=None, proxy_media=True):
f = upload
# we update the accessed_date
@@ -342,8 +347,7 @@ def handle_serve(upload, user, format=None, max_bitrate=None, proxy_media=True):
mapping = {"nginx": "X-Accel-Redirect", "apache2": "X-Sendfile"}
file_header = mapping[settings.REVERSE_PROXY_TYPE]
response[file_header] = file_path
- filename = "filename*=UTF-8''{}".format(urllib.parse.quote(filename))
- response["Content-Disposition"] = "attachment; {}".format(filename)
+ response["Content-Disposition"] = get_content_disposition(filename)
if mt:
response["Content-Type"] = mt
diff --git a/api/tests/common/test_utils.py b/api/tests/common/test_utils.py
index ea64ed9d2834f8d97aee7425fb223548ffab1b52..74a3d0bca61526e1cf7b5e87772a18a17a3a77aa 100644
--- a/api/tests/common/test_utils.py
+++ b/api/tests/common/test_utils.py
@@ -85,3 +85,17 @@ def test_get_updated_fields(conf, mock_args, data, expected, mocker):
obj = mocker.Mock(**mock_args)
assert utils.get_updated_fields(conf, data, obj) == expected
+
+
+@pytest.mark.parametrize(
+ "start, end, expected",
+ [
+ ("https://domain", "/api", "https://domain/api"),
+ ("https://domain/", "/api", "https://domain/api"),
+ ("https://domain", "api", "https://domain/api"),
+ ("https://domain", "https://api", "https://api"),
+ ("https://domain", "http://api", "http://api"),
+ ],
+)
+def test_join_url(start, end, expected):
+ assert utils.join_url(start, end) == expected
diff --git a/api/tests/music/test_metadata.py b/api/tests/music/test_metadata.py
index 1656ece49701159c4215e82c95d384b052925d35..52c4e302685b7bf0aa178676f321c29bf551e1a8 100644
--- a/api/tests/music/test_metadata.py
+++ b/api/tests/music/test_metadata.py
@@ -539,6 +539,33 @@ def test_serializer_album_artist_missing():
assert serializer.validated_data == expected
+@pytest.mark.parametrize(
+ "field_name", ["copyright", "license", "mbid", "position", "disc_number"]
+)
+def test_serializer_empty_fields(field_name):
+ data = {
+ "title": "Track Title",
+ "artist": "Track Artist",
+ "album": "Track Album",
+ # empty copyright/license field shouldn't fail, cf #850
+ field_name: "",
+ }
+ expected = {
+ "title": "Track Title",
+ "artists": [{"name": "Track Artist", "mbid": None}],
+ "album": {
+ "title": "Track Album",
+ "mbid": None,
+ "release_date": None,
+ "artists": [],
+ },
+ "cover_data": None,
+ }
+ serializer = metadata.TrackMetadataSerializer(data=metadata.FakeMetadata(data))
+ assert serializer.is_valid(raise_exception=True) is True
+ assert serializer.validated_data == expected
+
+
def test_artist_field_featuring():
data = {
"artist": "Santana feat. Chris Cornell",
diff --git a/api/tests/music/test_views.py b/api/tests/music/test_views.py
index 102b5a790a82d8d6fc174df72b414d0bf64033bf..25845e738b228b1b1e969c04070f2de134d454eb 100644
--- a/api/tests/music/test_views.py
+++ b/api/tests/music/test_views.py
@@ -1,6 +1,7 @@
import io
import magic
import os
+import urllib.parse
import pytest
from django.urls import reverse
@@ -412,7 +413,7 @@ def test_handle_serve_create_mp3_version(factories, now):
user = factories["users.User"]()
upload = factories["music.Upload"](bitrate=42)
response = views.handle_serve(upload, user, format="mp3")
-
+ expected_filename = upload.track.full_name + ".mp3"
version = upload.versions.latest("id")
assert version.mimetype == "audio/mpeg"
@@ -421,7 +422,9 @@ def test_handle_serve_create_mp3_version(factories, now):
assert version.audio_file_path.endswith(".mp3")
assert version.size == version.audio_file.size
assert magic.from_buffer(version.audio_file.read(), mime=True) == "audio/mpeg"
-
+ assert response["Content-Disposition"] == "attachment; filename*=UTF-8''{}".format(
+ urllib.parse.quote(expected_filename)
+ )
assert response.status_code == 200
diff --git a/changes/changelog.d/848.bugfix b/changes/changelog.d/848.bugfix
new file mode 100644
index 0000000000000000000000000000000000000000..478a8d42cc1fc6dbcf217a0bdb1721146c6744a4
--- /dev/null
+++ b/changes/changelog.d/848.bugfix
@@ -0,0 +1 @@
+Fixed invalid file extension for transcoded tracks (#848)
diff --git a/changes/changelog.d/850.bugfix b/changes/changelog.d/850.bugfix
new file mode 100644
index 0000000000000000000000000000000000000000..0e26ce773fcfc55a4c1f07708b58b6d6ebea5735
--- /dev/null
+++ b/changes/changelog.d/850.bugfix
@@ -0,0 +1 @@
+Ensure empty but optional fields in file metadata don't error during import (#850)
diff --git a/changes/changelog.d/851.bugfix b/changes/changelog.d/851.bugfix
new file mode 100644
index 0000000000000000000000000000000000000000..e6866b3e1f99b5c47ff6d764394da4ef501e1a7c
--- /dev/null
+++ b/changes/changelog.d/851.bugfix
@@ -0,0 +1 @@
+Fixed wrong og:image url when using S3 storage (#851)