Merge branch 'fix/media-proxy-url-encoded' into 'develop'

Media proxy: fix url encoding

See merge request pleroma/pleroma!521

3 files changed, 51 insertions, 2 deletions

diff --git a/lib/pleroma/web/media_proxy/controller.ex b/lib/pleroma/web/media_proxy/controller.ex
index d0b92d0c1..f496fc936 100644
--- a/lib/pleroma/web/media_proxy/controller.ex
+++ b/lib/pleroma/web/media_proxy/controller.ex
@@ -24,7 +24,12 @@ defmodule Pleroma.Web.MediaProxy.MediaProxyController do
   end
 
   def filename_matches(has_filename, path, url) do
-    filename = MediaProxy.filename(url)
+    filename =
+      url
+      |> MediaProxy.filename()
+      |> URI.decode()
+
+    path = URI.decode(path)
 
     cond do
       has_filename && filename && Path.basename(path) != filename -> {:wrong_filename, filename}
diff --git a/lib/pleroma/web/media_proxy/media_proxy.ex b/lib/pleroma/web/media_proxy/media_proxy.ex
index 28aacb0b1..902ab1b77 100644
--- a/lib/pleroma/web/media_proxy/media_proxy.ex
+++ b/lib/pleroma/web/media_proxy/media_proxy.ex
@@ -14,7 +14,14 @@ defmodule Pleroma.Web.MediaProxy do
       url
     else
       secret = Application.get_env(:pleroma, Pleroma.Web.Endpoint)[:secret_key_base]
-      base64 = Base.url_encode64(url, @base64_opts)
+
+      # The URL is url-decoded and encoded again to ensure it is correctly encoded and not twice.
+      base64 =
+        url
+        |> URI.decode()
+        |> URI.encode()
+        |> Base.url_encode64(@base64_opts)
+
       sig = :crypto.hmac(:sha, secret, base64)
       sig64 = sig |> Base.url_encode64(@base64_opts)
 
diff --git a/test/media_proxy_test.exs b/test/media_proxy_test.exs
index d71f9f13a..cb455ca79 100644
--- a/test/media_proxy_test.exs
+++ b/test/media_proxy_test.exs
@@ -1,6 +1,7 @@
 defmodule Pleroma.MediaProxyTest do
   use ExUnit.Case
   import Pleroma.Web.MediaProxy
+  alias Pleroma.Web.MediaProxy.MediaProxyController
 
   describe "when enabled" do
     setup do
@@ -65,6 +66,14 @@ defmodule Pleroma.MediaProxyTest do
       assert decode_result(encoded) == url
     end
 
+    test "ensures urls are url-encoded" do
+      assert decode_result(url("https://pleroma.social/Hello world.jpg")) ==
+               "https://pleroma.social/Hello%20world.jpg"
+
+      assert decode_result(url("https://pleroma.social/Hello%20world.jpg")) ==
+               "https://pleroma.social/Hello%20world.jpg"
+    end
+
     test "validates signature" do
       secret_key_base = Pleroma.Config.get([Pleroma.Web.Endpoint, :secret_key_base])
 
@@ -83,6 +92,34 @@ defmodule Pleroma.MediaProxyTest do
       assert decode_url(sig, base64) == {:error, :invalid_signature}
     end
 
+    test "filename_matches matches url encoded paths" do
+      assert MediaProxyController.filename_matches(
+               true,
+               "/Hello%20world.jpg",
+               "http://pleroma.social/Hello world.jpg"
+             ) == :ok
+
+      assert MediaProxyController.filename_matches(
+               true,
+               "/Hello%20world.jpg",
+               "http://pleroma.social/Hello%20world.jpg"
+             ) == :ok
+    end
+
+    test "filename_matches matches non-url encoded paths" do
+      assert MediaProxyController.filename_matches(
+               true,
+               "/Hello world.jpg",
+               "http://pleroma.social/Hello%20world.jpg"
+             ) == :ok
+
+      assert MediaProxyController.filename_matches(
+               true,
+               "/Hello world.jpg",
+               "http://pleroma.social/Hello world.jpg"
+             ) == :ok
+    end
+
     test "uses the configured base_url" do
       base_url = Pleroma.Config.get([:media_proxy, :base_url])