Add ability to bypass url decode/parse in Pleroma.HTTP, fix encode in Pleroma.Upload

lib/pleroma/http.ex

@@ -133,32 +133,51 @@ defmodule Pleroma.HTTP do
   defp default_middleware,
     do: [Tesla.Middleware.FollowRedirects, Pleroma.Tesla.Middleware.EncodeUrl]
 
-  def encode_url(url) when is_binary(url) do
-    URI.parse(url)
-    |> then(fn parsed ->
-      path = encode_path(parsed.path)
-      query = encode_query(parsed.query)
+  # We don't always want to decode the path first, like is the case in
+  # Pleroma.Upload.url_from_spec/3.
+  def encode_url(url, opts \\ []) when is_binary(url) and is_list(opts) do
+    bypass_parse = Keyword.get(opts, :bypass_parse, false)
+    bypass_decode = Keyword.get(opts, :bypass_decode, false)
 
-      %{parsed | path: path, query: query}
-    end)
-    |> URI.to_string()
+    cond do
+      bypass_parse ->
+        encode_path(url, bypass_decode)
+
+      true ->
+        URI.parse(url)
+        |> then(fn parsed ->
+          path = encode_path(parsed.path, bypass_decode)
+          query = encode_query(parsed.query)
+
+          %{parsed | path: path, query: query}
+        end)
+        |> URI.to_string()
+    end
   end
 
-  defp encode_path(nil), do: nil
+  defp encode_path(nil, _bypass_decode), do: nil
 
   # URI.encode/2 deliberately does not encode all chars that are forbidden
   # in the path component of a URI. It only encodes chars that are forbidden
   # in the whole URI. A predicate in the 2nd argument is used to fix that here.
   # URI.encode/2 uses the predicate function to determine whether each byte
   # (in an integer representation) should be encoded or not.
-  defp encode_path(path) when is_binary(path) do
+  defp encode_path(path, bypass_decode) when is_binary(path) do
+    path =
+      cond do
+        bypass_decode ->
+          path
+
+        true ->
+          URI.decode(path)
+      end
+
     path
-    |> URI.decode()
     |> URI.encode(fn byte ->
       URI.char_unreserved?(byte) || Enum.any?(
         Pleroma.Constants.uri_path_allowed_reserved_chars, fn char ->
           char == byte end)
-    end)
+      end)
   end
 
   defp encode_query(nil), do: nil

lib/pleroma/upload.ex

@@ -34,6 +34,7 @@ defmodule Pleroma.Upload do
 
   """
   alias Ecto.UUID
+  alias Pleroma.HTTP
   alias Pleroma.Maps
   alias Pleroma.Web.ActivityPub.Utils
   require Logger
@@ -230,11 +231,17 @@ defmodule Pleroma.Upload do
     tmp_path
   end
 
+  # Encoding the whole path here is fine since the path is in a
+  # UUID/<file name> form.
+  # The file at this point isn't %-encoded, so the path shouldn't
+  # be decoded first like Pleroma.HTTP.encode_url/1 does.
   defp url_from_spec(%__MODULE__{name: name}, base_url, {:file, path}) do
+    encode_opts = [bypass_decode: true, bypass_parse: true]
+
     path =
-      URI.encode(path, &char_unescaped?/1) <>
+      HTTP.encode_url(path, encode_opts) <>
         if Pleroma.Config.get([__MODULE__, :link_name], false) do
-          "?name=#{URI.encode(name, &char_unescaped?/1)}"
+          "?name=#{URI.encode_query(name)}"
         else
           ""
         end

test/pleroma/upload_test.exs

@@ -242,6 +242,21 @@ defmodule Pleroma.UploadTest do
       assert Path.basename(attachment_url["href"]) ==
                "%3A%3F%23%5B%5D%40%21%24%26%5C%27%28%29%2A%2B%2C%3B%3D.jpg"
     end
+
+    test "double %-encodes filename" do
+      File.cp!("test/fixtures/image.jpg", "test/fixtures/image_tmp.jpg")
+
+      file = %Plug.Upload{
+        content_type: "image/jpeg",
+        path: Path.absname("test/fixtures/image_tmp.jpg"),
+        filename: "file with %20.jpg"
+      }
+
+      {:ok, data} = Upload.store(file)
+      [attachment_url | _] = data["url"]
+
+      assert Path.basename(attachment_url["href"]) == "file%20with%20%2520.jpg"
+    end
   end
 
   describe "Setting a custom base_url for uploaded media" do