From 4bc1fde1059c6d953d91f5f57146781cd5559025 Mon Sep 17 00:00:00 2001 From: Claire Date: Sat, 3 Jul 2021 21:13:47 +0200 Subject: [PATCH] Fix anonymous access to outbox not being cached by the reverse proxy (#16458) * Fix anonymous access to outbox not being cached by the reverse proxy Up until now, anonymous access to outbox was marked as public, but with a 0 duration for caching, which means remote proxies would only serve from cache when the server was completely overwhelmed. Changed that cache duration to one minute, so that repeated anonymous access to one account's outbox can be appropriately cached. Also added `Signature` to the `Vary` header in case a page is requested, so that authenticated fetches are never served from cache (which only contains public toots). * Remove Vary: Accept header from webfinger controller Indeed, we have stopped returning xrd, and only ever return jrd, so the Accept request header does not matter anymore. * Cache negative webfinger hits for 3 minutes --- .../activitypub/outboxes_controller.rb | 10 +++++++++- .../well_known/webfinger_controller.rb | 3 ++- .../activitypub/outboxes_controller_spec.rb | 16 ++++++++++++---- .../well_known/webfinger_controller_spec.rb | 4 ++++ 4 files changed, 27 insertions(+), 6 deletions(-) diff --git a/app/controllers/activitypub/outboxes_controller.rb b/app/controllers/activitypub/outboxes_controller.rb index 4a52560ac..b2aab56a5 100644 --- a/app/controllers/activitypub/outboxes_controller.rb +++ b/app/controllers/activitypub/outboxes_controller.rb @@ -11,7 +11,11 @@ class ActivityPub::OutboxesController < ActivityPub::BaseController before_action :set_cache_headers def show - expires_in(page_requested? ? 0 : 3.minutes, public: public_fetch_mode? && !(signed_request_account.present? && page_requested?)) + if page_requested? + expires_in(1.minute, public: public_fetch_mode? && signed_request_account.nil?) + else + expires_in(3.minutes, public: public_fetch_mode?) + end render json: outbox_presenter, serializer: ActivityPub::OutboxSerializer, adapter: ActivityPub::Adapter, content_type: 'application/activity+json' end @@ -76,4 +80,8 @@ class ActivityPub::OutboxesController < ActivityPub::BaseController def set_account @account = params[:account_username].present? ? Account.find_local!(username_param) : Account.representative end + + def set_cache_headers + response.headers['Vary'] = 'Signature' if authorized_fetch_mode? || page_requested? + end end diff --git a/app/controllers/well_known/webfinger_controller.rb b/app/controllers/well_known/webfinger_controller.rb index 0227f722a..2b296ea3b 100644 --- a/app/controllers/well_known/webfinger_controller.rb +++ b/app/controllers/well_known/webfinger_controller.rb @@ -4,7 +4,6 @@ module WellKnown class WebfingerController < ActionController::Base include RoutingHelper - before_action { response.headers['Vary'] = 'Accept' } before_action :set_account before_action :check_account_suspension @@ -39,10 +38,12 @@ module WellKnown end def bad_request + expires_in(3.minutes, public: true) head 400 end def not_found + expires_in(3.minutes, public: true) head 404 end diff --git a/spec/controllers/activitypub/outboxes_controller_spec.rb b/spec/controllers/activitypub/outboxes_controller_spec.rb index d23f2c17c..1722690db 100644 --- a/spec/controllers/activitypub/outboxes_controller_spec.rb +++ b/spec/controllers/activitypub/outboxes_controller_spec.rb @@ -55,6 +55,10 @@ RSpec.describe ActivityPub::OutboxesController, type: :controller do it_behaves_like 'cachable response' + it 'does not have a Vary header' do + expect(response.headers['Vary']).to be_nil + end + context 'when account is permanently suspended' do before do account.suspend! @@ -96,6 +100,10 @@ RSpec.describe ActivityPub::OutboxesController, type: :controller do it_behaves_like 'cachable response' + it 'returns Vary header with Signature' do + expect(response.headers['Vary']).to include 'Signature' + end + context 'when account is permanently suspended' do before do account.suspend! @@ -144,7 +152,7 @@ RSpec.describe ActivityPub::OutboxesController, type: :controller do end it 'returns private Cache-Control header' do - expect(response.headers['Cache-Control']).to eq 'max-age=0, private' + expect(response.headers['Cache-Control']).to eq 'max-age=60, private' end end @@ -170,7 +178,7 @@ RSpec.describe ActivityPub::OutboxesController, type: :controller do end it 'returns private Cache-Control header' do - expect(response.headers['Cache-Control']).to eq 'max-age=0, private' + expect(response.headers['Cache-Control']).to eq 'max-age=60, private' end end @@ -195,7 +203,7 @@ RSpec.describe ActivityPub::OutboxesController, type: :controller do end it 'returns private Cache-Control header' do - expect(response.headers['Cache-Control']).to eq 'max-age=0, private' + expect(response.headers['Cache-Control']).to eq 'max-age=60, private' end end @@ -220,7 +228,7 @@ RSpec.describe ActivityPub::OutboxesController, type: :controller do end it 'returns private Cache-Control header' do - expect(response.headers['Cache-Control']).to eq 'max-age=0, private' + expect(response.headers['Cache-Control']).to eq 'max-age=60, private' end end end diff --git a/spec/controllers/well_known/webfinger_controller_spec.rb b/spec/controllers/well_known/webfinger_controller_spec.rb index 1075456f3..8574d369d 100644 --- a/spec/controllers/well_known/webfinger_controller_spec.rb +++ b/spec/controllers/well_known/webfinger_controller_spec.rb @@ -24,6 +24,10 @@ describe WellKnown::WebfingerController, type: :controller do expect(response).to have_http_status(200) end + it 'does not set a Vary header' do + expect(response.headers['Vary']).to be_nil + end + it 'returns application/jrd+json' do expect(response.media_type).to eq 'application/jrd+json' end