FIX: Include the Vary:Accept header on all Accept-based responses (#14647)

By default, Rails only includes the Vary:Accept header in responses when the Accept: header is included in the request. This means that proxies/browsers may cache a response to a request with a missing Accept header, and then later serve that cached version for a request which **does** supply the Accept header. This can lead to some very unexpected behavior in browsers.

This commit adds the Vary:Accept header for all requests, even if the Accept header is not present in the request. If a format parameter (e.g. `.json` suffix) is included in the path, then the Accept header is still omitted. (The format parameter takes precedence over any Accept: header, so the response is no longer varies based on the Accept header)

app/controllers/application_controller.rb

@@ -9,6 +9,7 @@ class ApplicationController < ActionController::Base
   include GlobalPath
   include Hijack
   include ReadOnlyHeader
+  include VaryHeader
 
   attr_reader :theme_id
 
@@ -46,6 +47,7 @@ class ApplicationController < ActionController::Base
   after_action  :perform_refresh_session
   after_action  :dont_cache_page
   after_action  :conditionally_allow_site_embedding
+  after_action  :ensure_vary_header
 
   HONEYPOT_KEY ||= 'HONEYPOT_KEY'
   CHALLENGE_KEY ||= 'CHALLENGE_KEY'

lib/vary_header.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+module VaryHeader
+  def ensure_vary_header
+    response.headers['Vary'] ||= 'Accept' if !params[:format]
+  end
+end

spec/requests/application_controller_spec.rb

@@ -831,4 +831,24 @@ RSpec.describe ApplicationController do
       end
     end
   end
+
+  describe 'vary header' do
+    it 'includes Vary:Accept on all requests where format is not explicit' do
+      # Rails default behaviour - include Vary:Accept when Accept is supplied
+      get "/latest", headers: { "Accept" => "application/json" }
+      expect(response.status).to eq(200)
+      expect(response.headers["Vary"]).to eq("Accept")
+
+      # Discourse additional behaviour (see lib/vary_header.rb)
+      # Include Vary:Accept even when Accept is not supplied
+      get "/latest"
+      expect(response.status).to eq(200)
+      expect(response.headers["Vary"]).to eq("Accept")
+
+      # Not needed, because the path 'format' parameter overrides the Accept header
+      get "/latest.json"
+      expect(response.status).to eq(200)
+      expect(response.headers["Vary"]).to eq(nil)
+    end
+  end
 end