From ad31d0726ac1aabfb97ed9746591e315420f17bb Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 13 Mar 2020 11:30:27 -0500 Subject: [PATCH 1/9] Do not trust remote Cache-Control headers for mediaproxy --- lib/pleroma/reverse_proxy/reverse_proxy.ex | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/lib/pleroma/reverse_proxy/reverse_proxy.ex b/lib/pleroma/reverse_proxy/reverse_proxy.ex index a281a00dc..8db3f78bb 100644 --- a/lib/pleroma/reverse_proxy/reverse_proxy.ex +++ b/lib/pleroma/reverse_proxy/reverse_proxy.ex @@ -7,7 +7,7 @@ defmodule Pleroma.ReverseProxy do @keep_req_headers ~w(accept user-agent accept-encoding cache-control if-modified-since) ++ ~w(if-unmodified-since if-none-match if-range range) - @resp_cache_headers ~w(etag date last-modified cache-control) + @resp_cache_headers ~w(etag date last-modified) @keep_resp_headers @resp_cache_headers ++ ~w(content-type content-disposition content-encoding content-range) ++ ~w(accept-ranges vary) @@ -34,9 +34,6 @@ defmodule Pleroma.ReverseProxy do * request: `#{inspect(@keep_req_headers)}` * response: `#{inspect(@keep_resp_headers)}` - If no caching headers (`#{inspect(@resp_cache_headers)}`) are returned by upstream, `cache-control` will be - set to `#{inspect(@default_cache_control_header)}`. - Options: * `redirect_on_failure` (default `false`). Redirects the client to the real remote URL if there's any HTTP @@ -297,16 +294,12 @@ defp build_resp_headers(headers, opts) do defp build_resp_cache_headers(headers, _opts) do has_cache? = Enum.any?(headers, fn {k, _} -> k in @resp_cache_headers end) - has_cache_control? = List.keymember?(headers, "cache-control", 0) cond do - has_cache? && has_cache_control? -> - headers - has_cache? -> # There's caching header present but no cache-control -- we need to explicitely override it # to public as Plug defaults to "max-age=0, private, must-revalidate" - List.keystore(headers, "cache-control", 0, {"cache-control", "public"}) + List.keystore(headers, "cache-control", 0, {"cache-control", @default_cache_control_header}) true -> List.keystore( From e04e16bbc05b035c11b83d5134436d791c512421 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 13 Mar 2020 11:31:55 -0500 Subject: [PATCH 2/9] Do not strip Cache-Control headers from media. Trust the Pleroma backend. --- installation/pleroma.nginx | 2 -- 1 file changed, 2 deletions(-) diff --git a/installation/pleroma.nginx b/installation/pleroma.nginx index 7f48b614b..688be3e71 100644 --- a/installation/pleroma.nginx +++ b/installation/pleroma.nginx @@ -90,8 +90,6 @@ server { proxy_ignore_client_abort on; proxy_buffering on; chunked_transfer_encoding on; - proxy_ignore_headers Cache-Control; - proxy_hide_header Cache-Control; proxy_pass http://127.0.0.1:4000; } } From c62195127d93761703954af97e328675ee853805 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 13 Mar 2020 11:46:40 -0500 Subject: [PATCH 3/9] Update comment to reflect what the code is actually doing --- lib/pleroma/reverse_proxy/reverse_proxy.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/reverse_proxy/reverse_proxy.ex b/lib/pleroma/reverse_proxy/reverse_proxy.ex index 8db3f78bb..072a3d263 100644 --- a/lib/pleroma/reverse_proxy/reverse_proxy.ex +++ b/lib/pleroma/reverse_proxy/reverse_proxy.ex @@ -297,8 +297,8 @@ defp build_resp_cache_headers(headers, _opts) do cond do has_cache? -> - # There's caching header present but no cache-control -- we need to explicitely override it - # to public as Plug defaults to "max-age=0, private, must-revalidate" + # There's caching header present but no cache-control -- we need to set our own + # as Plug defaults to "max-age=0, private, must-revalidate" List.keystore(headers, "cache-control", 0, {"cache-control", @default_cache_control_header}) true -> From 413177c8f0e4b15eb085c4efa26c94d572ee8d88 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 13 Mar 2020 12:02:58 -0500 Subject: [PATCH 4/9] Set correct Cache-Control header for local media --- lib/pleroma/plugs/uploaded_media.ex | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/pleroma/plugs/uploaded_media.ex b/lib/pleroma/plugs/uploaded_media.ex index f372829a2..57097baae 100644 --- a/lib/pleroma/plugs/uploaded_media.ex +++ b/lib/pleroma/plugs/uploaded_media.ex @@ -14,6 +14,8 @@ defmodule Pleroma.Plugs.UploadedMedia do # no slashes @path "media" + @default_cache_control_header "public max-age=86400 must-revalidate" + def init(_opts) do static_plug_opts = [] @@ -58,6 +60,10 @@ defp get_media(conn, {:static_dir, directory}, _, opts) do Map.get(opts, :static_plug_opts) |> Map.put(:at, [@path]) |> Map.put(:from, directory) + |> Map.put(:cache_control_for_etags, @default_cache_control_header) + |> Map.put(:headers, %{ + "cache-control" => @default_cache_control_header + }) conn = Plug.Static.call(conn, static_opts) From 470090471dabdd3863b9082f1c7aba6c84e7e703 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 13 Mar 2020 12:20:33 -0500 Subject: [PATCH 5/9] Fix test to use new cache-control settings --- test/reverse_proxy_test.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/reverse_proxy_test.exs b/test/reverse_proxy_test.exs index 18d70862c..f1690ade9 100644 --- a/test/reverse_proxy_test.exs +++ b/test/reverse_proxy_test.exs @@ -294,7 +294,7 @@ test "add cache-control", %{conn: conn} do |> expect(:stream_body, fn _ -> :done end) conn = ReverseProxy.call(conn, "/cache") - assert {"cache-control", "public"} in conn.resp_headers + assert {"cache-control", "public, max-age=1209600"} in conn.resp_headers end end From db36b48180fda3b0632a5088e45fb0dbf42952c1 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 13 Mar 2020 12:23:14 -0500 Subject: [PATCH 6/9] Remove test verifying we preserve cache-control headers; we don't --- test/reverse_proxy_test.exs | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/test/reverse_proxy_test.exs b/test/reverse_proxy_test.exs index f1690ade9..87c6aca4e 100644 --- a/test/reverse_proxy_test.exs +++ b/test/reverse_proxy_test.exs @@ -275,17 +275,6 @@ test "returns 400 on non GET, HEAD requests", %{conn: conn} do end describe "cache resp headers" do - test "returns headers", %{conn: conn} do - ClientMock - |> expect(:request, fn :get, "/cache/" <> ttl, _, _, _ -> - {:ok, 200, [{"cache-control", "public, max-age=" <> ttl}], %{}} - end) - |> expect(:stream_body, fn _ -> :done end) - - conn = ReverseProxy.call(conn, "/cache/10") - assert {"cache-control", "public, max-age=10"} in conn.resp_headers - end - test "add cache-control", %{conn: conn} do ClientMock |> expect(:request, fn :get, "/cache", _, _, _ -> From 3b1b183b42019adc9d09b0c1af703b25e313167d Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 13 Mar 2020 12:27:50 -0500 Subject: [PATCH 7/9] Synchronize cache-control header for local media with the mediaproxy --- lib/pleroma/plugs/uploaded_media.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pleroma/plugs/uploaded_media.ex b/lib/pleroma/plugs/uploaded_media.ex index 57097baae..74427709d 100644 --- a/lib/pleroma/plugs/uploaded_media.ex +++ b/lib/pleroma/plugs/uploaded_media.ex @@ -14,7 +14,7 @@ defmodule Pleroma.Plugs.UploadedMedia do # no slashes @path "media" - @default_cache_control_header "public max-age=86400 must-revalidate" + @default_cache_control_header "public, max-age=1209600" def init(_opts) do static_plug_opts = From 7321429a2ea134d7d920d8c977c4ec7bdcafc5e1 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 13 Mar 2020 12:42:06 -0500 Subject: [PATCH 8/9] Lint --- lib/pleroma/reverse_proxy/reverse_proxy.ex | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/reverse_proxy/reverse_proxy.ex b/lib/pleroma/reverse_proxy/reverse_proxy.ex index 072a3d263..8b713b8f4 100644 --- a/lib/pleroma/reverse_proxy/reverse_proxy.ex +++ b/lib/pleroma/reverse_proxy/reverse_proxy.ex @@ -299,7 +299,12 @@ defp build_resp_cache_headers(headers, _opts) do has_cache? -> # There's caching header present but no cache-control -- we need to set our own # as Plug defaults to "max-age=0, private, must-revalidate" - List.keystore(headers, "cache-control", 0, {"cache-control", @default_cache_control_header}) + List.keystore( + headers, + "cache-control", + 0, + {"cache-control", @default_cache_control_header} + ) true -> List.keystore( From 6a28c198af415c81596587f765e6c8c9388e9714 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Fri, 13 Mar 2020 22:12:33 +0300 Subject: [PATCH 9/9] uploaded media plug: do not inject compile-time params on every request --- lib/pleroma/plugs/uploaded_media.ex | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/pleroma/plugs/uploaded_media.ex b/lib/pleroma/plugs/uploaded_media.ex index 74427709d..36ff024a7 100644 --- a/lib/pleroma/plugs/uploaded_media.ex +++ b/lib/pleroma/plugs/uploaded_media.ex @@ -18,7 +18,10 @@ defmodule Pleroma.Plugs.UploadedMedia do def init(_opts) do static_plug_opts = - [] + [ + headers: %{"cache-control" => @default_cache_control_header}, + cache_control_for_etags: @default_cache_control_header + ] |> Keyword.put(:from, "__unconfigured_media_plug") |> Keyword.put(:at, "/__unconfigured_media_plug") |> Plug.Static.init() @@ -60,10 +63,6 @@ defp get_media(conn, {:static_dir, directory}, _, opts) do Map.get(opts, :static_plug_opts) |> Map.put(:at, [@path]) |> Map.put(:from, directory) - |> Map.put(:cache_control_for_etags, @default_cache_control_header) - |> Map.put(:headers, %{ - "cache-control" => @default_cache_control_header - }) conn = Plug.Static.call(conn, static_opts)