Merge branch 'proxy-error' into 'develop'

make sure the url used by proxy is same as original url

See merge request pleroma/pleroma!1383

2 files changed, 25 insertions, 33 deletions

diff --git a/lib/pleroma/web/media_proxy/media_proxy.ex b/lib/pleroma/web/media_proxy/media_proxy.ex
index cee6d8481..dd8888a02 100644
--- a/lib/pleroma/web/media_proxy/media_proxy.ex
+++ b/lib/pleroma/web/media_proxy/media_proxy.ex
@@ -33,20 +33,7 @@ defmodule Pleroma.Web.MediaProxy do
 
   def encode_url(url) do
     secret = Pleroma.Config.get([Pleroma.Web.Endpoint, :secret_key_base])
-
-    # Must preserve `%2F` for compatibility with S3
-    # https://git.pleroma.social/pleroma/pleroma/issues/580
-    replacement = get_replacement(url, ":2F:")
-
-    # The URL is url-decoded and encoded again to ensure it is correctly encoded and not twice.
-    base64 =
-      url
-      |> String.replace("%2F", replacement)
-      |> URI.decode()
-      |> URI.encode()
-      |> String.replace(replacement, "%2F")
-      |> Base.url_encode64(@base64_opts)
-
+    base64 = Base.url_encode64(url, @base64_opts)
     sig = :crypto.hmac(:sha, secret, base64)
     sig64 = sig |> Base.url_encode64(@base64_opts)
 
@@ -80,12 +67,4 @@ defmodule Pleroma.Web.MediaProxy do
     |> Enum.filter(fn value -> value end)
     |> Path.join()
   end
-
-  defp get_replacement(url, replacement) do
-    if String.contains?(url, replacement) do
-      get_replacement(url, replacement <> replacement)
-    else
-      replacement
-    end
-  end
 end
diff --git a/test/media_proxy_test.exs b/test/media_proxy_test.exs
index b23aeb88b..1d6d170b7 100644
--- a/test/media_proxy_test.exs
+++ b/test/media_proxy_test.exs
@@ -70,14 +70,6 @@ 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])
 
@@ -141,10 +133,31 @@ defmodule Pleroma.MediaProxyTest do
       assert String.starts_with?(encoded, Pleroma.Config.get([:media_proxy, :base_url]))
     end
 
-    # https://git.pleroma.social/pleroma/pleroma/issues/580
-    test "encoding S3 links (must preserve `%2F`)" do
+    # Some sites expect ASCII encoded characters in the URL to be preserved even if
+    # unnecessary.
+    # Issues: https://git.pleroma.social/pleroma/pleroma/issues/580
+    #         https://git.pleroma.social/pleroma/pleroma/issues/1055
+    test "preserve ASCII encoding" do
+      url =
+        "https://pleroma.com/%20/%21/%22/%23/%24/%25/%26/%27/%28/%29/%2A/%2B/%2C/%2D/%2E/%2F/%30/%31/%32/%33/%34/%35/%36/%37/%38/%39/%3A/%3B/%3C/%3D/%3E/%3F/%40/%41/%42/%43/%44/%45/%46/%47/%48/%49/%4A/%4B/%4C/%4D/%4E/%4F/%50/%51/%52/%53/%54/%55/%56/%57/%58/%59/%5A/%5B/%5C/%5D/%5E/%5F/%60/%61/%62/%63/%64/%65/%66/%67/%68/%69/%6A/%6B/%6C/%6D/%6E/%6F/%70/%71/%72/%73/%74/%75/%76/%77/%78/%79/%7A/%7B/%7C/%7D/%7E/%7F/%80/%81/%82/%83/%84/%85/%86/%87/%88/%89/%8A/%8B/%8C/%8D/%8E/%8F/%90/%91/%92/%93/%94/%95/%96/%97/%98/%99/%9A/%9B/%9C/%9D/%9E/%9F/%C2%A0/%A1/%A2/%A3/%A4/%A5/%A6/%A7/%A8/%A9/%AA/%AB/%AC/%C2%AD/%AE/%AF/%B0/%B1/%B2/%B3/%B4/%B5/%B6/%B7/%B8/%B9/%BA/%BB/%BC/%BD/%BE/%BF/%C0/%C1/%C2/%C3/%C4/%C5/%C6/%C7/%C8/%C9/%CA/%CB/%CC/%CD/%CE/%CF/%D0/%D1/%D2/%D3/%D4/%D5/%D6/%D7/%D8/%D9/%DA/%DB/%DC/%DD/%DE/%DF/%E0/%E1/%E2/%E3/%E4/%E5/%E6/%E7/%E8/%E9/%EA/%EB/%EC/%ED/%EE/%EF/%F0/%F1/%F2/%F3/%F4/%F5/%F6/%F7/%F8/%F9/%FA/%FB/%FC/%FD/%FE/%FF"
+
+      encoded = url(url)
+      assert decode_result(encoded) == url
+    end
+
+    # This includes unsafe/reserved characters which are not interpreted as part of the URL
+    # and would otherwise have to be ASCII encoded. It is our role to ensure the proxied URL
+    # is unmodified, so we are testing these characters anyway.
+    test "preserve non-unicode characters per RFC3986" do
       url =
-        "https://s3.amazonaws.com/example/test.png?X-Amz-Credential=your-access-key-id%2F20130721%2Fus-east-1%2Fs3%2Faws4_request"
+        "https://pleroma.com/ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz1234567890-._~:/?#[]@!$&'()*+,;=|^`{}"
+
+      encoded = url(url)
+      assert decode_result(encoded) == url
+    end
+
+    test "preserve unicode characters" do
+      url = "https://ko.wikipedia.org/wiki/위키백과:대문"
 
       encoded = url(url)
       assert decode_result(encoded) == url