UX: Allow secure media URLs to be cached for a short period of time

UX: Allow secure media URLs to be cached for a short period of time

Signed S3 URLs are valid for 15 seconds, so we can safely allow the browser to cache them for 10 seconds. This should help with large numbers of requests when composing a post with many images.

diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb
index ccbd762..64fcd79 100644
--- a/app/controllers/uploads_controller.rb
+++ b/app/controllers/uploads_controller.rb
@@ -10,6 +10,8 @@ class UploadsController < ApplicationController
 
   before_action :is_asset_path, only: [:show, :show_short, :show_secure]
 
+  SECURE_REDIRECT_GRACE_SECONDS = 5
+
   def create
     # capture current user for block later on
     me = current_user
@@ -151,6 +153,9 @@ class UploadsController < ApplicationController
       return render_404 if current_user.nil?
     end
 
+    cache_seconds = S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS - SECURE_REDIRECT_GRACE_SECONDS
+    expires_in cache_seconds.seconds # defaults to public: false, so only cached by the client browser
+
     # url_for figures out the full URL, handling multisite DBs,
     # and will return a presigned URL for the upload
     if path_with_ext.blank?
diff --git a/spec/requests/uploads_controller_spec.rb b/spec/requests/uploads_controller_spec.rb
index d773ec1..d8eaee5 100644
--- a/spec/requests/uploads_controller_spec.rb
+++ b/spec/requests/uploads_controller_spec.rb
@@ -396,6 +396,16 @@ describe UploadsController do
           expect(response).to redirect_to(Discourse.store.signed_url_for_path(Discourse.store.get_path_for_upload(upload)))
         end
 
+        it "has the correct caching header" do
+          sign_in(user)
+          get upload.short_path
+
+          expected_max_age = S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS - UploadsController::SECURE_REDIRECT_GRACE_SECONDS
+          expect(expected_max_age).to be > 0 # Sanity check that the constants haven't been set to broken values
+
+          expect(response.headers["Cache-Control"]).to eq("max-age=#{expected_max_age}, private")
+        end
+
         it "raises invalid access if the user cannot access the upload access control post" do
           sign_in(user)
           post = Fabricate(:post)

GitHub sha: 96848b76

1 Like

came here for this :+1:

1 Like