Merge branch 'media-root' into 'develop'

Make media and static files serving more reliable when reverse proxy...

Closes #947

See merge request funkwhale/funkwhale!959

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(

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")
 
 

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)
+    )

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

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)