Skip to content

Commit

Permalink
UX: Allow secure media URLs to be cached for a short period of time
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
davidtaylorhq committed May 18, 2020
1 parent 303dece commit 96848b7
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 0 deletions.
5 changes: 5 additions & 0 deletions app/controllers/uploads_controller.rb
Expand Up @@ -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
Expand Down Expand Up @@ -151,6 +153,9 @@ def handle_secure_upload_request(upload, path_with_ext = nil)
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?
Expand Down
10 changes: 10 additions & 0 deletions spec/requests/uploads_controller_spec.rb
Expand Up @@ -396,6 +396,16 @@ def upload_file(file, folder = "images")
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

This comment has been minimized.

Copy link
@jjaffeux

jjaffeux May 18, 2020

Contributor

came here for this 👍


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)
Expand Down

0 comments on commit 96848b7

Please sign in to comment.