diff options
-rw-r--r-- | app/controllers/active_storage/disk_controller.rb | 4 | ||||
-rw-r--r-- | app/models/active_storage/blob.rb | 2 | ||||
-rw-r--r-- | app/models/active_storage/service.rb | 2 | ||||
-rw-r--r-- | app/models/active_storage/service/disk_service.rb | 8 | ||||
-rw-r--r-- | app/models/active_storage/service/gcs_service.rb | 8 | ||||
-rw-r--r-- | app/models/active_storage/service/s3_service.rb | 5 | ||||
-rw-r--r-- | app/models/active_storage/variant.rb | 2 | ||||
-rw-r--r-- | test/models/blob_test.rb | 2 | ||||
-rw-r--r-- | test/service/disk_service_test.rb | 2 | ||||
-rw-r--r-- | test/service/gcs_service_test.rb | 5 | ||||
-rw-r--r-- | test/service/mirror_service_test.rb | 4 | ||||
-rw-r--r-- | test/service/s3_service_test.rb | 4 |
12 files changed, 27 insertions, 21 deletions
diff --git a/app/controllers/active_storage/disk_controller.rb b/app/controllers/active_storage/disk_controller.rb index a42b4833a7..986eee6504 100644 --- a/app/controllers/active_storage/disk_controller.rb +++ b/app/controllers/active_storage/disk_controller.rb @@ -11,8 +11,8 @@ class ActiveStorage::DiskController < ActionController::Base def show if key = decode_verified_key - # FIXME: Find a way to set the correct content type - send_data disk_service.download(key), filename: params[:filename], disposition: disposition_param + # FIXME: Do we need to sign or otherwise validate the content type? + send_data disk_service.download(key), filename: params[:filename], disposition: disposition_param, content_type: params[:content_type] else head :not_found end diff --git a/app/models/active_storage/blob.rb b/app/models/active_storage/blob.rb index fdf9a2c37d..3340c88d12 100644 --- a/app/models/active_storage/blob.rb +++ b/app/models/active_storage/blob.rb @@ -57,7 +57,7 @@ class ActiveStorage::Blob < ActiveRecord::Base def url(expires_in: 5.minutes, disposition: :inline) - service.url key, expires_in: expires_in, disposition: disposition, filename: filename + service.url key, expires_in: expires_in, disposition: disposition, filename: filename, content_type: content_type end def url_for_direct_upload(expires_in: 5.minutes) diff --git a/app/models/active_storage/service.rb b/app/models/active_storage/service.rb index 745b1a615f..9d370d0a2b 100644 --- a/app/models/active_storage/service.rb +++ b/app/models/active_storage/service.rb @@ -74,7 +74,7 @@ class ActiveStorage::Service raise NotImplementedError end - def url(key, expires_in:, disposition:, filename:) + def url(key, expires_in:, disposition:, filename:, content_type:) raise NotImplementedError end diff --git a/app/models/active_storage/service/disk_service.rb b/app/models/active_storage/service/disk_service.rb index 59b180d0e8..3cde203a31 100644 --- a/app/models/active_storage/service/disk_service.rb +++ b/app/models/active_storage/service/disk_service.rb @@ -51,15 +51,17 @@ class ActiveStorage::Service::DiskService < ActiveStorage::Service end end - def url(key, expires_in:, disposition:, filename:) + def url(key, expires_in:, disposition:, filename:, content_type:) instrument :url, key do |payload| verified_key_with_expiration = ActiveStorage.verifier.generate(key, expires_in: expires_in, purpose: :blob_key) generated_url = if defined?(Rails) && defined?(Rails.application) - Rails.application.routes.url_helpers.rails_disk_blob_path(verified_key_with_expiration, disposition: disposition, filename: filename) + Rails.application.routes.url_helpers.rails_disk_blob_path \ + verified_key_with_expiration, + disposition: disposition, filename: filename, content_type: content_type else - "/rails/active_storage/disk/#{verified_key_with_expiration}/#{filename}?disposition=#{disposition}" + "/rails/active_storage/disk/#{verified_key_with_expiration}/#{filename}?disposition=#{disposition}&content_type=#{content_type}" end payload[:url] = generated_url diff --git a/app/models/active_storage/service/gcs_service.rb b/app/models/active_storage/service/gcs_service.rb index 7053a130c0..4530de22f6 100644 --- a/app/models/active_storage/service/gcs_service.rb +++ b/app/models/active_storage/service/gcs_service.rb @@ -42,10 +42,12 @@ class ActiveStorage::Service::GCSService < ActiveStorage::Service end end - def url(key, expires_in:, disposition:, filename:) + def url(key, expires_in:, disposition:, filename:, content_type:) instrument :url, key do |payload| - query = { "response-content-disposition" => "#{disposition}; filename=\"#{filename}\"" } - generated_url = file_for(key).signed_url(expires: expires_in, query: query) + generated_url = file_for(key).signed_url expires: expires_in, query: { + "response-content-disposition" => "#{disposition}; filename=\"#{filename}\"", + "response-content-type" => content_type + } payload[:url] = generated_url diff --git a/app/models/active_storage/service/s3_service.rb b/app/models/active_storage/service/s3_service.rb index efffdec157..4c17f9902f 100644 --- a/app/models/active_storage/service/s3_service.rb +++ b/app/models/active_storage/service/s3_service.rb @@ -47,10 +47,11 @@ class ActiveStorage::Service::S3Service < ActiveStorage::Service end end - def url(key, expires_in:, disposition:, filename:) + def url(key, expires_in:, disposition:, filename:, content_type:) instrument :url, key do |payload| generated_url = object_for(key).presigned_url :get, expires_in: expires_in, - response_content_disposition: "#{disposition}; filename=\"#{filename}\"" + response_content_disposition: "#{disposition}; filename=\"#{filename}\"", + response_content_type: content_type payload[:url] = generated_url diff --git a/app/models/active_storage/variant.rb b/app/models/active_storage/variant.rb index 8785625cc8..d0fee3c62c 100644 --- a/app/models/active_storage/variant.rb +++ b/app/models/active_storage/variant.rb @@ -19,7 +19,7 @@ class ActiveStorage::Variant end def url(expires_in: 5.minutes, disposition: :inline) - service.url key, expires_in: expires_in, disposition: disposition, filename: blob.filename + service.url key, expires_in: expires_in, disposition: disposition, filename: blob.filename, content_type: blob.content_type end diff --git a/test/models/blob_test.rb b/test/models/blob_test.rb index 8a3d0e8124..b6ba63b25e 100644 --- a/test/models/blob_test.rb +++ b/test/models/blob_test.rb @@ -35,6 +35,6 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase private def expected_url_for(blob, disposition: :inline) - "/rails/active_storage/disk/#{ActiveStorage.verifier.generate(blob.key, expires_in: 5.minutes, purpose: :blob_key)}/#{blob.filename}?disposition=#{disposition}" + "/rails/active_storage/disk/#{ActiveStorage.verifier.generate(blob.key, expires_in: 5.minutes, purpose: :blob_key)}/#{blob.filename}?disposition=#{disposition}&content_type=#{blob.content_type}" end end diff --git a/test/service/disk_service_test.rb b/test/service/disk_service_test.rb index e9a96003f1..1dae4a2f00 100644 --- a/test/service/disk_service_test.rb +++ b/test/service/disk_service_test.rb @@ -7,6 +7,6 @@ class ActiveStorage::Service::DiskServiceTest < ActiveSupport::TestCase test "url generation" do assert_match /rails\/active_storage\/disk\/.*\/avatar\.png\?disposition=inline/, - @service.url(FIXTURE_KEY, expires_in: 5.minutes, disposition: :inline, filename: "avatar.png") + @service.url(FIXTURE_KEY, expires_in: 5.minutes, disposition: :inline, filename: "avatar.png", content_type: "image/png") end end diff --git a/test/service/gcs_service_test.rb b/test/service/gcs_service_test.rb index 4cde4b9289..57fe4d4562 100644 --- a/test/service/gcs_service_test.rb +++ b/test/service/gcs_service_test.rb @@ -29,9 +29,10 @@ if SERVICE_CONFIGURATIONS[:gcs] test "signed URL generation" do freeze_time do url = SERVICE.bucket.signed_url(FIXTURE_KEY, expires: 120) + - "&response-content-disposition=inline%3B+filename%3D%22test.txt%22" + "&response-content-disposition=inline%3B+filename%3D%22test.txt%22" + + "&response-content-type=text%2Fplain" - assert_equal url, @service.url(FIXTURE_KEY, expires_in: 2.minutes, disposition: :inline, filename: "test.txt") + assert_equal url, @service.url(FIXTURE_KEY, expires_in: 2.minutes, disposition: :inline, filename: "test.txt", content_type: "text/plain") end end end diff --git a/test/service/mirror_service_test.rb b/test/service/mirror_service_test.rb index fd3d8125d6..129e11d06f 100644 --- a/test/service/mirror_service_test.rb +++ b/test/service/mirror_service_test.rb @@ -46,8 +46,8 @@ class ActiveStorage::Service::MirrorServiceTest < ActiveSupport::TestCase test "URL generation in primary service" do freeze_time do - assert_equal SERVICE.primary.url(FIXTURE_KEY, expires_in: 2.minutes, disposition: :inline, filename: "test.txt"), - @service.url(FIXTURE_KEY, expires_in: 2.minutes, disposition: :inline, filename: "test.txt") + assert_equal SERVICE.primary.url(FIXTURE_KEY, expires_in: 2.minutes, disposition: :inline, filename: "test.txt", content_type: "text/plain"), + @service.url(FIXTURE_KEY, expires_in: 2.minutes, disposition: :inline, filename: "test.txt", content_type: "text/plain") end end diff --git a/test/service/s3_service_test.rb b/test/service/s3_service_test.rb index 049511497b..a6040ec1d5 100644 --- a/test/service/s3_service_test.rb +++ b/test/service/s3_service_test.rb @@ -27,8 +27,8 @@ if SERVICE_CONFIGURATIONS[:s3] end test "signed URL generation" do - assert_match /#{SERVICE_CONFIGURATIONS[:s3][:bucket]}\.s3.(\S+)?amazonaws.com.*response-content-disposition=inline.*avatar\.png/, - @service.url(FIXTURE_KEY, expires_in: 5.minutes, disposition: :inline, filename: "avatar.png") + assert_match /#{SERVICE_CONFIGURATIONS[:s3][:bucket]}\.s3.(\S+)?amazonaws.com.*response-content-disposition=inline.*avatar\.png.*response-content-type=image%2Fpng/, + @service.url(FIXTURE_KEY, expires_in: 5.minutes, disposition: :inline, filename: "avatar.png", content_type: "image/png") end test "uploading with server-side encryption" do |