diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 366c06548ac..814b65ac39d 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -111,7 +111,7 @@ class UploadsController < ApplicationController if Discourse.store.internal? send_file_local_upload(upload) else - redirect_to Discourse.store.url_for(upload, force_download: params[:dl] == "1") + redirect_to Discourse.store.url_for(upload, force_download: force_download?) end else render_404 @@ -160,11 +160,13 @@ class UploadsController < ApplicationController # url_for figures out the full URL, handling multisite DBs, # and will return a presigned URL for the upload if path_with_ext.blank? - return redirect_to Discourse.store.url_for(upload) + return redirect_to Discourse.store.url_for(upload, force_download: force_download?) end redirect_to Discourse.store.signed_url_for_path( - path_with_ext, expires_in: S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS + path_with_ext, + expires_in: S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS, + force_download: force_download? ) end @@ -183,6 +185,10 @@ class UploadsController < ApplicationController protected + def force_download? + params[:dl] == "1" + end + def xhr_not_allowed raise Discourse::InvalidParameters.new("XHR not allowed") end diff --git a/lib/file_store/s3_store.rb b/lib/file_store/s3_store.rb index d80e1ff5d75..fcc289123cf 100644 --- a/lib/file_store/s3_store.rb +++ b/lib/file_store/s3_store.rb @@ -169,9 +169,9 @@ module FileStore url.sub(File.join("#{schema}#{absolute_base_url}", folder), File.join(SiteSetting.Upload.s3_cdn_url, "/")) end - def signed_url_for_path(path, expires_in: S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS) + def signed_url_for_path(path, expires_in: S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS, force_download: false) key = path.sub(absolute_base_url + "/", "") - presigned_url(key, expires_in: expires_in) + presigned_url(key, expires_in: expires_in, force_download: force_download) end def cache_avatar(avatar, user_id) diff --git a/spec/requests/uploads_controller_spec.rb b/spec/requests/uploads_controller_spec.rb index 02b4c035946..ca4d257bfa2 100644 --- a/spec/requests/uploads_controller_spec.rb +++ b/spec/requests/uploads_controller_spec.rb @@ -402,6 +402,14 @@ describe UploadsController do get upload.short_path expect(response).to redirect_to(Discourse.store.signed_url_for_path(Discourse.store.get_path_for_upload(upload))) + expect(response.header['Location']).not_to include('response-content-disposition=attachment') + end + + it "respects the force download (dl) param" do + sign_in(user) + freeze_time + get upload.short_path, params: { dl: '1' } + expect(response.header['Location']).to include('response-content-disposition=attachment') end it "has the correct caching header" do