Repository: phoenixframework/phoenix (elixir)

Issue:
`Phoenix.Controller.send_download/3` incorrectly escapes spaces in filenames
<!--
### Precheck

* For general discussions and support, use Stack Overflow or the Elixir Forum
* For proposing and discussing new features, start a thread on the Phoenix Core mailing list
* For bugs, do a quick search and make sure the bug has not yet been reported
* Ensure that this issue is related to the Phoenix library and not one of the dependencies listed in mix.exs (Ecto, Plug, etc.)
* All checked? Be nice and have fun!
-->

### Environment

* Elixir version (elixir -v): Erlang/OTP 25, Elixir 1.14.4
* Phoenix version (mix deps): 1.7.2

### Expected behavior

When hitting a controller route that looks like this:


```elixir
def download(conn, _params) do
  send_download(conn, {:binary, "test"}, filename: "name with spaces.txt")
end
```

I would expect the user to be prompted to download a file named `name with spaces.txt`.

### Actual behavior

The user is prompted to download a file named `name+with+spaces.txt`

### Proposed solution

Using RFC-3986 encoding instead of "x-www-form-urlencoded"; i.e. encoding the space character as `%20` instead of `+`. The following does preserve spaces:

```elixir
def download(conn, _params) do
  send_download(conn, {:binary, "test"},
    filename: URI.encode("name with spaces.txt", %URI.char_unreserved?/1),
    encode: false
  )
end
```

I don't believe using `URI.char_unreserved?/1` instead of the default `URI.char_unescaped?/1` is actually necessary for security reasons but it is the smallest breaking change from `URI.encode_www_form/1`.

### Previous discussions

Discussed in #3344 where the `encode: false` option was introduced. The option to use `URI.encode/2` was I believe dismissed in error.

I suspect when testing whether `URI.encode/2` would work the filename accidentally got encoded twice (i.e. `filename="some%2520file.txt"`), which is what happens if you manually `URI.encode` the filename without also disabling the automatic encoding. The browser will decode that - once - into `some%20file.txt`.

Pull request:
Fix filename when sending downloads with non-ascii names
according to https://www.rfc-editor.org/rfc/rfc6266#appendix-D when it is not possible to express a filename with regular encoding, a `filename*` parameter can be added to the header.

This commit adds the `filename*` when the encoded filename does not match the filename.

This has been tested on chrome, safari and firefox.

This closes #5478 

Patch:
diff --git a/lib/phoenix/controller.ex b/lib/phoenix/controller.ex
index 87c51c9b6..ca59e3999 100644
--- a/lib/phoenix/controller.ex
+++ b/lib/phoenix/controller.ex
@@ -1259,16 +1259,22 @@ defmodule Phoenix.Controller do
     disposition_type = get_disposition_type(Keyword.get(opts, :disposition, :attachment))
     warn_if_ajax(conn)
 
+    disposition = ~s[#{disposition_type}; filename="#{encoded_filename}"]
+
+    disposition =
+      if encoded_filename != filename do
+        disposition <> "; filename*=utf-8''#{encoded_filename}"
+      else
+        disposition
+      end
+
     conn
     |> put_resp_content_type(content_type, opts[:charset])
-    |> put_resp_header(
-      "content-disposition",
-      ~s[#{disposition_type}; filename="#{encoded_filename}"]
-    )
+    |> put_resp_header("content-disposition", disposition)
   end
 
   defp encode_filename(filename, false), do: filename
-  defp encode_filename(filename, true), do: URI.encode_www_form(filename)
+  defp encode_filename(filename, true), do: URI.encode(filename)
 
   defp get_disposition_type(:attachment), do: "attachment"
   defp get_disposition_type(:inline), do: "inline"


Test patch:
diff --git a/test/phoenix/controller/controller_test.exs b/test/phoenix/controller/controller_test.exs
index 64db87e55..5d33b4408 100644
--- a/test/phoenix/controller/controller_test.exs
+++ b/test/phoenix/controller/controller_test.exs
@@ -530,13 +530,24 @@ defmodule Phoenix.Controller.ControllerTest do
       conn = send_download(conn(:get, "/"), {:file, @hello_txt}, filename: "hello world.json")
       assert conn.status == 200
       assert get_resp_header(conn, "content-disposition") ==
-             ["attachment; filename=\"hello+world.json\""]
+             ["attachment; filename=\"hello%20world.json\"; filename*=utf-8''hello%20world.json"]
       assert get_resp_header(conn, "content-type") ==
              ["application/json"]
       assert conn.resp_body ==
              "world"
     end
 
+    test "sends file supports UTF-8" do
+      conn = send_download(conn(:get, "/"), {:file, @hello_txt}, filename: "测 试.txt")
+      assert conn.status == 200
+      assert get_resp_header(conn, "content-disposition") ==
+             ["attachment; filename=\"%E6%B5%8B%20%E8%AF%95.txt\"; filename*=utf-8''%E6%B5%8B%20%E8%AF%95.txt"]
+      assert get_resp_header(conn, "content-type") ==
+             ["text/plain"]
+      assert conn.resp_body ==
+             "world"
+    end
+
     test "sends file for download with custom :filename and :encode false" do
       conn = send_download(conn(:get, "/"), {:file, @hello_txt}, filename: "dev's hello world.json", encode: false)
       assert conn.status == 200
@@ -583,10 +594,10 @@ defmodule Phoenix.Controller.ControllerTest do
     end
 
     test "sends binary for download with :filename" do
-      conn = send_download(conn(:get, "/"), {:binary, "world"}, filename: "hello world.json")
+      conn = send_download(conn(:get, "/"), {:binary, "world"}, filename: "hello.json")
       assert conn.status == 200
       assert get_resp_header(conn, "content-disposition") ==
-             ["attachment; filename=\"hello+world.json\""]
+             ["attachment; filename=\"hello.json\""]
       assert get_resp_header(conn, "content-type") ==
              ["application/json"]
       assert conn.resp_body ==