Fix format-dependent redirects being cached regardless of requested format (#27632)

config/routes.rb

@@ -3,6 +3,18 @@
 require 'sidekiq_unique_jobs/web'
 require 'sidekiq-scheduler/web'
 
+class RedirectWithVary < ActionDispatch::Routing::PathRedirect
+  def build_response(req)
+    super.tap do |response|
+      response.headers['Vary'] = 'Origin, Accept'
+    end
+  end
+end
+
+def redirect_with_vary(path)
+  RedirectWithVary.new(301, path)
+end
+
 Rails.application.routes.draw do
   # Paths of routes on the web app that to not require to be indexed or
   # have alternative format representations requiring separate controllers
@@ -90,10 +102,13 @@ Rails.application.routes.draw do
     confirmations: 'auth/confirmations',
   }
 
-  get '/users/:username', to: redirect('/@%{username}'), constraints: lambda { |req| req.format.nil? || req.format.html? }
+  # rubocop:disable Style/FormatStringToken - those do not go through the usual formatting functions and are not safe to correct
-  get '/users/:username/following', to: redirect('/@%{username}/following'), constraints: lambda { |req| req.format.nil? || req.format.html? }
+  get '/users/:username', to: redirect_with_vary('/@%{username}'), constraints: lambda { |req| req.format.nil? || req.format.html? }
-  get '/users/:username/followers', to: redirect('/@%{username}/followers'), constraints: lambda { |req| req.format.nil? || req.format.html? }
+  get '/users/:username/following', to: redirect_with_vary('/@%{username}/following'), constraints: lambda { |req| req.format.nil? || req.format.html? }
-  get '/users/:username/statuses/:id', to: redirect('/@%{username}/%{id}'), constraints: lambda { |req| req.format.nil? || req.format.html? }
+  get '/users/:username/followers', to: redirect_with_vary('/@%{username}/followers'), constraints: lambda { |req| req.format.nil? || req.format.html? }
+  get '/users/:username/statuses/:id', to: redirect_with_vary('/@%{username}/%{id}'), constraints: lambda { |req| req.format.nil? || req.format.html? }
+  # rubocop:enable Style/FormatStringToken
+
   get '/authorize_follow', to: redirect { |_, request| "/authorize_interaction?#{request.params.to_query}" }
 
   resources :accounts, path: 'users', only: [:show], param: :username do

spec/requests/cache_spec.rb

@@ -124,7 +124,7 @@ describe 'Caching behavior' do
       expect(response.cookies).to be_empty
     end
 
-    it 'sets public cache control' do
+    it 'sets public cache control', :aggregate_failures do
       # expect(response.cache_control[:max_age]&.to_i).to be_positive
       expect(response.cache_control[:public]).to be_truthy
       expect(response.cache_control[:private]).to be_falsy
@@ -141,11 +141,8 @@ describe 'Caching behavior' do
   end
 
   shared_examples 'non-cacheable error' do
-    it 'does not return HTTP success' do
+    it 'does not return HTTP success and does not have cache headers', :aggregate_failures do
       expect(response).to_not have_http_status(200)
-    end
-
-    it 'does not have cache headers' do
       expect(response.cache_control[:public]).to be_falsy
     end
   end
@@ -180,6 +177,15 @@ describe 'Caching behavior' do
   end
 
   context 'when anonymously accessed' do
+    describe '/users/alice' do
+      it 'redirects with proper cache header', :aggregate_failures do
+        get '/users/alice'
+
+        expect(response).to redirect_to('/@alice')
+        expect(response.headers['Vary']&.split(',')&.map { |x| x.strip.downcase }).to include('accept')
+      end
+    end
+
     TestEndpoints::ALWAYS_CACHED.each do |endpoint|
       describe endpoint do
         before { get endpoint }