From 5fc63a1d232608530a4cd4a45c34a2bed695169a Mon Sep 17 00:00:00 2001
From: Tim Zallmann <tzallmann@gitlab.com>
Date: Mon, 21 Jan 2019 22:25:54 +0100
Subject: [PATCH] Changed the Caching of User Avatars to be public and to 5
 minutes

---
 app/controllers/concerns/uploads_actions.rb | 12 +++++++++++-
 app/controllers/uploads_controller.rb       |  4 ++++
 spec/controllers/uploads_controller_spec.rb |  9 ++++++++-
 3 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/app/controllers/concerns/uploads_actions.rb b/app/controllers/concerns/uploads_actions.rb
index c114e16edf8..7106c61e749 100644
--- a/app/controllers/concerns/uploads_actions.rb
+++ b/app/controllers/concerns/uploads_actions.rb
@@ -29,7 +29,13 @@ module UploadsActions
   def show
     return render_404 unless uploader&.exists?
 
-    expires_in 0.seconds, must_revalidate: true, private: true
+    if cache_privately?
+      expires_in 0.seconds, must_revalidate: true, private: true
+    else
+      # We need to reset caching from the applications controller to get rid of the no-store value
+      headers['Cache-Control'] = ''
+      expires_in 5.minutes, public: true, must_revalidate: false
+    end
 
     disposition = uploader.image_or_video? ? 'inline' : 'attachment'
 
@@ -114,6 +120,10 @@ module UploadsActions
     nil
   end
 
+  def cache_privately?
+    true
+  end
+
   def model
     strong_memoize(:model) { find_model }
   end
diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb
index fa5d84633b5..20a5a0d2f42 100644
--- a/app/controllers/uploads_controller.rb
+++ b/app/controllers/uploads_controller.rb
@@ -70,6 +70,10 @@ class UploadsController < ApplicationController
     end
   end
 
+  def cache_privately?
+    true unless (User === model || Appearance === model)
+  end
+
   def upload_model_class
     MODEL_CLASSES[params[:model]] || raise(UnknownUploadModelError)
   end
diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb
index 19142aa1272..482f9af7211 100644
--- a/spec/controllers/uploads_controller_spec.rb
+++ b/spec/controllers/uploads_controller_spec.rb
@@ -12,6 +12,13 @@ shared_examples 'content not cached without revalidation and no-store' do
   end
 end
 
+shared_examples 'content publicy cached' do
+  it 'ensures content is publicly cached' do
+    # Fixed in newer versions of ActivePack, it will only output a single `private`.
+    expect(subject['Cache-Control']).to eq('max-age=300, public')
+  end
+end
+
 describe UploadsController do
   let!(:user) { create(:user, avatar: fixture_file_upload("spec/fixtures/dk.png", "image/png")) }
 
@@ -184,7 +191,7 @@ describe UploadsController do
             expect(response).to have_gitlab_http_status(200)
           end
 
-          it_behaves_like 'content not cached without revalidation and no-store' do
+          it_behaves_like 'content publicy cached' do
             subject do
               get :show, params: { model: 'user', mounted_as: 'avatar', id: user.id, filename: 'dk.png' }
 
-- 
2.30.9