From 5a73cae2be8e9b490ed4a610347998f1120740f0 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Tue, 12 Mar 2019 09:10:19 +0300 Subject: [PATCH 1/5] WIP: Stop mangling filenames --- lib/pleroma/plugs/uploaded_media.ex | 10 ++++++++++ lib/pleroma/upload.ex | 10 ++++------ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/lib/pleroma/plugs/uploaded_media.ex b/lib/pleroma/plugs/uploaded_media.ex index 13aa8641a..15f447ded 100644 --- a/lib/pleroma/plugs/uploaded_media.ex +++ b/lib/pleroma/plugs/uploaded_media.ex @@ -24,6 +24,16 @@ def init(_opts) do end def call(%{request_path: <<"/", @path, "/", file::binary>>} = conn, opts) do + conn = + case fetch_query_params(conn) do + %{query_params: %{"name" => name}} = conn -> + conn + |> put_resp_header("Content-Disposition", "filename=\"#{name}\"") + + conn -> + conn + end + config = Pleroma.Config.get([Pleroma.Upload]) with uploader <- Keyword.fetch!(config, :uploader), diff --git a/lib/pleroma/upload.ex b/lib/pleroma/upload.ex index 1a97e9fde..ae461d434 100644 --- a/lib/pleroma/upload.ex +++ b/lib/pleroma/upload.ex @@ -70,7 +70,7 @@ def store(upload, opts \\ []) do %{ "type" => "Link", "mediaType" => upload.content_type, - "href" => url_from_spec(opts.base_url, url_spec) + "href" => url_from_spec(opts.base_url, url_spec, upload.name) } ], "name" => Map.get(opts, :description) || upload.name @@ -219,14 +219,12 @@ defp tempfile_for_image(data) do tmp_path end - defp url_from_spec(base_url, {:file, path}) do - path = - path - |> URI.encode(&char_unescaped?/1) + defp url_from_spec(base_url, {:file, path}, name) do + path = URI.encode(path, &char_unescaped?/1) <> "?name=#{URI.encode(name, &char_unescaped?/1)}" [base_url, "media", path] |> Path.join() end - defp url_from_spec(_base_url, {:url, url}), do: url + defp url_from_spec(_base_url, {:url, url}, _name), do: url end From 92a69bddce10da92a6a418f08c134ebdd6217ca4 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Tue, 12 Mar 2019 09:21:13 +0300 Subject: [PATCH 2/5] escape quotation marks in Content-Disposition header --- lib/pleroma/plugs/uploaded_media.ex | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/pleroma/plugs/uploaded_media.ex b/lib/pleroma/plugs/uploaded_media.ex index 15f447ded..bc913f408 100644 --- a/lib/pleroma/plugs/uploaded_media.ex +++ b/lib/pleroma/plugs/uploaded_media.ex @@ -27,6 +27,8 @@ def call(%{request_path: <<"/", @path, "/", file::binary>>} = conn, opts) do conn = case fetch_query_params(conn) do %{query_params: %{"name" => name}} = conn -> + name = String.replace(name, "\"", "\\\"") + conn |> put_resp_header("Content-Disposition", "filename=\"#{name}\"") From faf238c1b0b4d814ce3b2e041ed6b18b498233bf Mon Sep 17 00:00:00 2001 From: rinpatch Date: Tue, 12 Mar 2019 15:33:15 +0300 Subject: [PATCH 3/5] Fix upload tests --- test/upload_test.exs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/upload_test.exs b/test/upload_test.exs index bdda01b3f..c38280d4c 100644 --- a/test/upload_test.exs +++ b/test/upload_test.exs @@ -56,7 +56,7 @@ test "copies the file to the configured folder with deduping" do assert List.first(data["url"])["href"] == Pleroma.Web.base_url() <> - "/media/e7a6d0cf595bff76f14c9a98b6c199539559e8b844e02e51e5efcfd1f614a2df.jpg" + "/media/e7a6d0cf595bff76f14c9a98b6c199539559e8b844e02e51e5efcfd1f614a2df.jpg?name=an%20%5Bimage.jpg" end test "copies the file to the configured folder without deduping" do @@ -150,7 +150,8 @@ test "escapes invalid characters in url" do {:ok, data} = Upload.store(file) [attachment_url | _] = data["url"] - assert Path.basename(attachment_url["href"]) == "an%E2%80%A6%20image.jpg" + assert Path.basename(attachment_url["href"]) == + "an%E2%80%A6%20image.jpg?name=an%E2%80%A6%20image.jpg" end test "escapes reserved uri characters" do @@ -166,7 +167,7 @@ test "escapes reserved uri characters" do [attachment_url | _] = data["url"] 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" + "%3A%3F%23%5B%5D%40%21%24%26%5C%27%28%29%2A%2B%2C%3B%3D.jpg?name=%3A%3F%23%5B%5D%40%21%24%26%5C%27%28%29%2A%2B%2C%3B%3D.jpg" end end end From e2fe796c63f9df18d30810ad9daa1e7027da120f Mon Sep 17 00:00:00 2001 From: rinpatch Date: Thu, 14 Mar 2019 22:02:48 +0300 Subject: [PATCH 4/5] Add some tests --- config/config.exs | 1 + config/test.exs | 2 ++ docs/config.md | 1 + lib/pleroma/plugs/uploaded_media.ex | 2 +- lib/pleroma/upload.ex | 14 ++++++--- test/plugs/uploaded_media_plug_test.exs | 40 +++++++++++++++++++++++++ test/upload_test.exs | 6 ++-- 7 files changed, 58 insertions(+), 8 deletions(-) create mode 100644 test/plugs/uploaded_media_plug_test.exs diff --git a/config/config.exs b/config/config.exs index cd4c8e562..f889e3259 100644 --- a/config/config.exs +++ b/config/config.exs @@ -35,6 +35,7 @@ config :pleroma, Pleroma.Upload, uploader: Pleroma.Uploaders.Local, filters: [], + link_name: true, proxy_remote: false, proxy_opts: [ redirect_on_failure: false, diff --git a/config/test.exs b/config/test.exs index 6dfa698c8..a3f36c9e1 100644 --- a/config/test.exs +++ b/config/test.exs @@ -17,6 +17,8 @@ # Print only warnings and errors during test config :logger, level: :warn +config :pleroma, Pleroma.Upload, link_name: false + config :pleroma, Pleroma.Uploaders.Local, uploads: "test/uploads" config :pleroma, Pleroma.Mailer, adapter: Swoosh.Adapters.Test diff --git a/docs/config.md b/docs/config.md index a09ea95f3..e34ffe980 100644 --- a/docs/config.md +++ b/docs/config.md @@ -6,6 +6,7 @@ If you run Pleroma with ``MIX_ENV=prod`` the file is ``prod.secret.exs``, otherw ## Pleroma.Upload * `uploader`: Select which `Pleroma.Uploaders` to use * `filters`: List of `Pleroma.Upload.Filter` to use. +* `link_name`: When enabled Pleroma will add a `name` parameter to the url of the upload, for example `https://instance.tld/media/corndog.png?name=corndog.png`. This is needed to provide the correct filename in Content-Disposition headers when using filters like `Pleroma.Upload.Filter.Dedupe` * `base_url`: The base URL to access a user-uploaded file. Useful when you want to proxy the media files via another host. * `proxy_remote`: If you\'re using a remote uploader, Pleroma will proxy media requests instead of redirecting to it. * `proxy_opts`: Proxy options, see `Pleroma.ReverseProxy` documentation. diff --git a/lib/pleroma/plugs/uploaded_media.ex b/lib/pleroma/plugs/uploaded_media.ex index bc913f408..fd77b8d8f 100644 --- a/lib/pleroma/plugs/uploaded_media.ex +++ b/lib/pleroma/plugs/uploaded_media.ex @@ -30,7 +30,7 @@ def call(%{request_path: <<"/", @path, "/", file::binary>>} = conn, opts) do name = String.replace(name, "\"", "\\\"") conn - |> put_resp_header("Content-Disposition", "filename=\"#{name}\"") + |> put_resp_header("content-disposition", "filename=\"#{name}\"") conn -> conn diff --git a/lib/pleroma/upload.ex b/lib/pleroma/upload.ex index ae461d434..f72334930 100644 --- a/lib/pleroma/upload.ex +++ b/lib/pleroma/upload.ex @@ -70,7 +70,7 @@ def store(upload, opts \\ []) do %{ "type" => "Link", "mediaType" => upload.content_type, - "href" => url_from_spec(opts.base_url, url_spec, upload.name) + "href" => url_from_spec(upload, opts.base_url, url_spec) } ], "name" => Map.get(opts, :description) || upload.name @@ -219,12 +219,18 @@ defp tempfile_for_image(data) do tmp_path end - defp url_from_spec(base_url, {:file, path}, name) do - path = URI.encode(path, &char_unescaped?/1) <> "?name=#{URI.encode(name, &char_unescaped?/1)}" + defp url_from_spec(%__MODULE__{name: name}, base_url, {:file, path}) do + path = + URI.encode(path, &char_unescaped?/1) <> + if Pleroma.Config.get([__MODULE__, :link_name], false) do + "?name=#{URI.encode(name, &char_unescaped?/1)}" + else + "" + end [base_url, "media", path] |> Path.join() end - defp url_from_spec(_base_url, {:url, url}, _name), do: url + defp url_from_spec(_upload, _base_url, {:url, url}), do: url end diff --git a/test/plugs/uploaded_media_plug_test.exs b/test/plugs/uploaded_media_plug_test.exs new file mode 100644 index 000000000..414cf9186 --- /dev/null +++ b/test/plugs/uploaded_media_plug_test.exs @@ -0,0 +1,40 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2018 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Web.UploadedMediaPlugTest do + use Pleroma.Web.ConnCase + alias Pleroma.Upload + + setup_all do + File.cp!("test/fixtures/image.jpg", "test/fixtures/image_tmp.jpg") + + file = %Plug.Upload{ + content_type: "image/jpg", + path: Path.absname("test/fixtures/image_tmp.jpg"), + filename: "nice_tf.jpg" + } + + {:ok, data} = Upload.store(file) + [%{"href" => attachment_url} | _] = data["url"] + [attachment_url: attachment_url] + end + + test "does not send Content-Disposition header when name param is not set", %{ + attachment_url: attachment_url + } do + conn = get(build_conn(), attachment_url) + refute Enum.any?(conn.resp_headers, &(elem(&1, 0) == "content-disposition")) + end + + test "sends Content-Disposition header when name param is set", %{ + attachment_url: attachment_url + } do + conn = get(build_conn(), attachment_url <> "?name=\"cofe\".gif") + + assert Enum.any?( + conn.resp_headers, + &(&1 == {"content-disposition", "filename=\"\\\"cofe\\\".gif\""}) + ) + end +end diff --git a/test/upload_test.exs b/test/upload_test.exs index c38280d4c..770226478 100644 --- a/test/upload_test.exs +++ b/test/upload_test.exs @@ -56,7 +56,7 @@ test "copies the file to the configured folder with deduping" do assert List.first(data["url"])["href"] == Pleroma.Web.base_url() <> - "/media/e7a6d0cf595bff76f14c9a98b6c199539559e8b844e02e51e5efcfd1f614a2df.jpg?name=an%20%5Bimage.jpg" + "/media/e7a6d0cf595bff76f14c9a98b6c199539559e8b844e02e51e5efcfd1f614a2df.jpg" end test "copies the file to the configured folder without deduping" do @@ -151,7 +151,7 @@ test "escapes invalid characters in url" do [attachment_url | _] = data["url"] assert Path.basename(attachment_url["href"]) == - "an%E2%80%A6%20image.jpg?name=an%E2%80%A6%20image.jpg" + "an%E2%80%A6%20image.jpg" end test "escapes reserved uri characters" do @@ -167,7 +167,7 @@ test "escapes reserved uri characters" do [attachment_url | _] = data["url"] 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?name=%3A%3F%23%5B%5D%40%21%24%26%5C%27%28%29%2A%2B%2C%3B%3D.jpg" + "%3A%3F%23%5B%5D%40%21%24%26%5C%27%28%29%2A%2B%2C%3B%3D.jpg" end end end From 355f285a8693934fbc8205c2c9ecde0a758fc158 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Thu, 14 Mar 2019 22:26:54 +0300 Subject: [PATCH 5/5] Fix uploaded media plug test --- test/plugs/uploaded_media_plug_test.exs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/plugs/uploaded_media_plug_test.exs b/test/plugs/uploaded_media_plug_test.exs index 414cf9186..49cf5396a 100644 --- a/test/plugs/uploaded_media_plug_test.exs +++ b/test/plugs/uploaded_media_plug_test.exs @@ -6,7 +6,8 @@ defmodule Pleroma.Web.UploadedMediaPlugTest do use Pleroma.Web.ConnCase alias Pleroma.Upload - setup_all do + defp upload_file(context) do + Pleroma.DataCase.ensure_local_uploader(context) File.cp!("test/fixtures/image.jpg", "test/fixtures/image_tmp.jpg") file = %Plug.Upload{ @@ -20,6 +21,8 @@ defmodule Pleroma.Web.UploadedMediaPlugTest do [attachment_url: attachment_url] end + setup_all :upload_file + test "does not send Content-Disposition header when name param is not set", %{ attachment_url: attachment_url } do