Merge branch 'fix/backup-url-on-s3' into 'develop'

Standardize base url for uploads

See merge request pleroma/pleroma!3246

lib/pleroma/upload.ex

@@ -31,6 +31,7 @@ defmodule Pleroma.Upload do
 
   """
   alias Ecto.UUID
+  alias Pleroma.Config
   require Logger
 
   @type source ::
@@ -228,4 +229,39 @@ defp url_from_spec(%__MODULE__{name: name}, base_url, {:file, path}) do
   end
 
   defp url_from_spec(_upload, _base_url, {:url, url}), do: url
+
+  def base_url do
+    uploader = Config.get([Pleroma.Upload, :uploader])
+    upload_base_url = Config.get([Pleroma.Upload, :base_url])
+    public_endpoint = Config.get([uploader, :public_endpoint])
+
+    case uploader do
+      Pleroma.Uploaders.Local ->
+        upload_base_url || Pleroma.Web.base_url() <> "/media/"
+
+      Pleroma.Uploaders.S3 ->
+        bucket = Config.get([Pleroma.Uploaders.S3, :bucket])
+
+        bucket_with_namespace =
+          cond do
+            truncated_namespace = Config.get([Pleroma.Uploaders.S3, :truncated_namespace]) ->
+              truncated_namespace
+
+            namespace = Config.get([Pleroma.Uploaders.S3, :bucket_namespace]) ->
+              namespace <> ":" <> bucket
+
+            true ->
+              bucket
+          end
+
+        if public_endpoint do
+          Path.join([public_endpoint, bucket_with_namespace])
+        else
+          Path.join([upload_base_url, bucket_with_namespace])
+        end
+
+      _ ->
+        public_endpoint || upload_base_url
+    end
+  end
 end

lib/pleroma/uploaders/s3.ex

@@ -12,26 +12,10 @@ defmodule Pleroma.Uploaders.S3 do
   # links with less strict filenames
   @impl true
   def get_file(file) do
-    config = Config.get([__MODULE__])
-    bucket = Keyword.fetch!(config, :bucket)
-
-    bucket_with_namespace =
-      cond do
-        truncated_namespace = Keyword.get(config, :truncated_namespace) ->
-          truncated_namespace
-
-        namespace = Keyword.get(config, :bucket_namespace) ->
-          namespace <> ":" <> bucket
-
-        true ->
-          bucket
-      end
-
     {:ok,
      {:url,
       Path.join([
-        Keyword.fetch!(config, :public_endpoint),
-        bucket_with_namespace,
+        Pleroma.Upload.base_url(),
         strict_encode(URI.decode(file))
       ])}}
   end

lib/pleroma/web/media_proxy.ex

@@ -69,7 +69,7 @@ def enabled?, do: Config.get([:media_proxy, :enabled], false)
   #   non-local non-whitelisted URLs through it and be sure that body size constraint is preserved.
   def preview_enabled?, do: enabled?() and !!Config.get([:media_preview_proxy, :enabled])
 
-  def local?(url), do: String.starts_with?(url, Pleroma.Web.base_url())
+  def local?(url), do: String.starts_with?(url, Web.base_url())
 
   def whitelisted?(url) do
     %{host: domain} = URI.parse(url)
@@ -77,17 +77,10 @@ def whitelisted?(url) do
     mediaproxy_whitelist_domains =
       [:media_proxy, :whitelist]
       |> Config.get()
+      |> Kernel.++(["#{Upload.base_url()}"])
       |> Enum.map(&maybe_get_domain_from_url/1)
 
-    whitelist_domains =
-      if base_url = Config.get([Upload, :base_url]) do
-        %{host: base_domain} = URI.parse(base_url)
-        [base_domain | mediaproxy_whitelist_domains]
-      else
-        mediaproxy_whitelist_domains
-      end
-
-    domain in whitelist_domains
+    domain in mediaproxy_whitelist_domains
   end
 
   defp maybe_get_domain_from_url("http" <> _ = url) do

lib/pleroma/web/pleroma_api/views/backup_view.ex

@@ -24,6 +24,6 @@ def render("index.json", %{backups: backups}) do
   end
 
   def download_url(%Backup{file_name: file_name}) do
-    Pleroma.Web.Endpoint.url() <> "/media/backups/" <> file_name
+    Pleroma.Upload.base_url() <> "/backups/" <> file_name
   end
 end

lib/pleroma/web/plugs/uploaded_media.ex

@@ -62,7 +62,7 @@ def call(%{request_path: <<"/", @path, "/", file::binary>>} = conn, opts) do
   def call(conn, _opts), do: conn
 
   defp media_is_banned(%{request_path: path} = _conn, {:static_dir, _}) do
-    MediaProxy.in_banned_urls(Pleroma.Web.base_url() <> path)
+    MediaProxy.in_banned_urls(Pleroma.Upload.base_url() <> path)
   end
 
   defp media_is_banned(_, {:url, url}), do: MediaProxy.in_banned_urls(url)

lib/pleroma/workers/attachments_cleanup_worker.ex

@@ -32,21 +32,15 @@ def perform(%Job{args: %{"op" => "cleanup_attachments", "object" => _object}}),
   defp do_clean({object_ids, attachment_urls}) do
     uploader = Pleroma.Config.get([Pleroma.Upload, :uploader])
 
-    prefix =
-      case Pleroma.Config.get([Pleroma.Upload, :base_url]) do
-        nil -> "media"
-        _ -> ""
-      end
-
     base_url =
       String.trim_trailing(
-        Pleroma.Config.get([Pleroma.Upload, :base_url], Pleroma.Web.base_url()),
+        Pleroma.Upload.base_url(),
         "/"
       )
 
     Enum.each(attachment_urls, fn href ->
       href
-      |> String.trim_leading("#{base_url}/#{prefix}")
+      |> String.trim_leading("#{base_url}")
       |> uploader.delete_file()
     end)
 

test/pleroma/uploaders/s3_test.exs

@@ -11,11 +11,16 @@ defmodule Pleroma.Uploaders.S3Test do
   import Mock
   import ExUnit.CaptureLog
 
-  setup do:
+  setup do
+    clear_config(Pleroma.Upload,
+      uploader: Pleroma.Uploaders.S3
+    )
+
     clear_config(Pleroma.Uploaders.S3,
       bucket: "test_bucket",
       public_endpoint: "https://s3.amazonaws.com"
     )
+  end
 
   describe "get_file/1" do
     test "it returns path to local folder for files" do