diff --git a/api/config/settings/common.py b/api/config/settings/common.py index 7c03b89dc..a4c53c01a 100644 --- a/api/config/settings/common.py +++ b/api/config/settings/common.py @@ -354,7 +354,7 @@ CRISPY_TEMPLATE_PACK = "bootstrap3" STATIC_ROOT = env("STATIC_ROOT", default=str(ROOT_DIR("staticfiles"))) # See: https://docs.djangoproject.com/en/dev/ref/settings/#static-url -STATIC_URL = env("STATIC_URL", default="/staticfiles/") +STATIC_URL = env("STATIC_URL", default=FUNKWHALE_URL + "/staticfiles/") DEFAULT_FILE_STORAGE = "funkwhale_api.common.storage.ASCIIFileSystemStorage" PROXY_MEDIA = env.bool("PROXY_MEDIA", default=True) @@ -391,7 +391,7 @@ STATICFILES_FINDERS = ( MEDIA_ROOT = env("MEDIA_ROOT", default=str(APPS_DIR("media"))) # See: https://docs.djangoproject.com/en/dev/ref/settings/#media-url -MEDIA_URL = env("MEDIA_URL", default="/media/") +MEDIA_URL = env("MEDIA_URL", default=FUNKWHALE_URL + "/media/") FILE_UPLOAD_PERMISSIONS = 0o644 ATTACHMENTS_UNATTACHED_PRUNE_DELAY = env.int( diff --git a/api/funkwhale_api/music/views.py b/api/funkwhale_api/music/views.py index 2260d9268..eaeb8a951 100644 --- a/api/funkwhale_api/music/views.py +++ b/api/funkwhale_api/music/views.py @@ -308,6 +308,16 @@ class TrackViewSet(common_views.SkipFilterForGetObject, viewsets.ReadOnlyModelVi ) +def strip_absolute_media_url(path): + if ( + settings.MEDIA_URL.startswith("http://") + or settings.MEDIA_URL.startswith("https://") + and path.startswith(settings.MEDIA_URL) + ): + path = path.replace(settings.MEDIA_URL, "/media/", 1) + return path + + def get_file_path(audio_file): serve_path = settings.MUSIC_DIRECTORY_SERVE_PATH prefix = settings.MUSIC_DIRECTORY_PATH @@ -324,6 +334,7 @@ def get_file_path(audio_file): "MUSIC_DIRECTORY_PATH to serve in-place imported files" ) path = "/music" + audio_file.replace(prefix, "", 1) + path = strip_absolute_media_url(path) if path.startswith("http://") or path.startswith("https://"): protocol, remainder = path.split("://", 1) hostname, r_path = remainder.split("/", 1) @@ -344,6 +355,7 @@ def get_file_path(audio_file): "MUSIC_DIRECTORY_PATH to serve in-place imported files" ) path = audio_file.replace(prefix, serve_path, 1) + path = strip_absolute_media_url(path) return path.encode("utf-8") diff --git a/api/tests/music/test_api.py b/api/tests/music/test_api.py index 99130c9d9..201f54bd7 100644 --- a/api/tests/music/test_api.py +++ b/api/tests/music/test_api.py @@ -3,6 +3,7 @@ import os import pytest from django.urls import reverse +from funkwhale_api.music import views DATA_DIR = os.path.dirname(os.path.abspath(__file__)) @@ -47,4 +48,6 @@ def test_upload_url_is_accessible_to_authenticated_users( response = logged_in_api_client.get(url) assert response.status_code == 200 - assert response["X-Accel-Redirect"] == "/_protected{}".format(upload.audio_file.url) + assert response["X-Accel-Redirect"] == "/_protected{}".format( + views.strip_absolute_media_url(upload.audio_file.url) + ) diff --git a/api/tests/music/test_views.py b/api/tests/music/test_views.py index f0cc68f41..67e513179 100644 --- a/api/tests/music/test_views.py +++ b/api/tests/music/test_views.py @@ -180,7 +180,8 @@ def test_can_serve_upload_as_remote_library( assert response.status_code == 200 assert response["X-Accel-Redirect"] == "{}{}".format( - settings.PROTECT_FILES_PATH, upload.audio_file.url + settings.PROTECT_FILES_PATH, + views.strip_absolute_media_url(upload.audio_file.url), ) @@ -330,7 +331,8 @@ def test_can_proxy_remote_track(factories, settings, api_client, r_mock, prefere assert response.status_code == 200 assert response["X-Accel-Redirect"] == "{}{}".format( - settings.PROTECT_FILES_PATH, upload.audio_file.url + settings.PROTECT_FILES_PATH, + views.strip_absolute_media_url(upload.audio_file.url), ) assert upload.audio_file.read() == b"test" @@ -1043,3 +1045,20 @@ def test_track_list_exclude_channels(params, expected, factories, logged_in_api_ assert response.status_code == 200 assert response.data["count"] == expected + + +@pytest.mark.parametrize( + "media_url, input, expected", + [ + ("https://domain/media/", "https://domain/media/file.mp3", "/media/file.mp3"), + ( + "https://domain/media/", + "https://otherdomain/media/file.mp3", + "https://otherdomain/media/file.mp3", + ), + ("https://domain/media/", "/media/file.mp3", "/media/file.mp3"), + ], +) +def test_strip_absolute_media_url(media_url, input, expected, settings): + settings.MEDIA_URL = media_url + assert views.strip_absolute_media_url(input) == expected diff --git a/changes/changelog.d/media-proxy.enhancement b/changes/changelog.d/media-proxy.enhancement new file mode 100644 index 000000000..e9fae2b5e --- /dev/null +++ b/changes/changelog.d/media-proxy.enhancement @@ -0,0 +1 @@ +Make media and static files serving more reliable when reverse proxy X_FORWARDED_* headers are incorrect (#947)