From 49219508bc2e01fe724830ca31a7cfa7adba15cf Mon Sep 17 00:00:00 2001
From: Claire <claire.github-309c@sitedethib.com>
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 4a52560aca..b2aab56a56 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 0227f722a7..2b296ea3be 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 d23f2c17cb..1722690db1 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 1075456f33..8574d369d1 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