From 9ac6f1d3bbf3a750619f16db3971e9d3beca48af Mon Sep 17 00:00:00 2001
From: David Taylor <david@taylorhq.com>
Date: Mon, 25 Oct 2021 12:53:50 +0100
Subject: [PATCH] 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    |  2 ++
 lib/vary_header.rb                           |  7 +++++++
 spec/requests/application_controller_spec.rb | 20 ++++++++++++++++++++
 3 files changed, 29 insertions(+)
 create mode 100644 lib/vary_header.rb

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index bdb33fee0ce..74eab9c71f3 100644
--- a/app/controllers/application_controller.rb
+++ b/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'
diff --git a/lib/vary_header.rb b/lib/vary_header.rb
new file mode 100644
index 00000000000..c80eb7a9b8d
--- /dev/null
+++ b/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
diff --git a/spec/requests/application_controller_spec.rb b/spec/requests/application_controller_spec.rb
index bff3fdcaa9a..610cebccbd2 100644
--- a/spec/requests/application_controller_spec.rb
+++ b/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