Commit e586b6e3 authored by Stan Hu's avatar Stan Hu

Fix Content-Disposition header not working in Azure Blob storage

Prior to this commit, files downloaded in Azure Blob Storage would not
have a Content-Disposition header sent, which would cause files to be
saved with an unfriendly
filename. https://gitlab.com/gitlab-org/gitlab-fog-azure-rm/-/merge_requests/25
added support for retrieving this response header by requesting this in
the Azure shared access signature (SAS) token. We now patch CarrierWave
to send these custom options to the File object.

We drop the dynamic parameter check in the CarrierWave patch since we
have a fog-google version that supports the options. Plus, this dynamic
parameter doesn't work with mocks since the mocks don't copy the exactly
method signature.

Relates to https://gitlab.com/gitlab-org/gitlab/-/issues/341061

Changelog: fixed
parent 06b9bf39
...@@ -129,7 +129,7 @@ gem 'fog-local', '~> 0.6' ...@@ -129,7 +129,7 @@ gem 'fog-local', '~> 0.6'
gem 'fog-openstack', '~> 1.0' gem 'fog-openstack', '~> 1.0'
gem 'fog-rackspace', '~> 0.1.1' gem 'fog-rackspace', '~> 0.1.1'
gem 'fog-aliyun', '~> 0.3' gem 'fog-aliyun', '~> 0.3'
gem 'gitlab-fog-azure-rm', '~> 1.1.1', require: false gem 'gitlab-fog-azure-rm', '~> 1.2.0', require: false
# for Google storage # for Google storage
gem 'google-api-client', '~> 0.33' gem 'google-api-client', '~> 0.33'
......
...@@ -467,7 +467,7 @@ GEM ...@@ -467,7 +467,7 @@ GEM
activesupport (>= 3.0) activesupport (>= 3.0)
request_store (>= 1.0) request_store (>= 1.0)
scientist (~> 1.6, >= 1.6.0) scientist (~> 1.6, >= 1.6.0)
gitlab-fog-azure-rm (1.1.1) gitlab-fog-azure-rm (1.2.0)
azure-storage-blob (~> 2.0) azure-storage-blob (~> 2.0)
azure-storage-common (~> 2.0) azure-storage-common (~> 2.0)
fog-core (= 2.1.0) fog-core (= 2.1.0)
...@@ -1466,7 +1466,7 @@ DEPENDENCIES ...@@ -1466,7 +1466,7 @@ DEPENDENCIES
gitlab-chronic (~> 0.10.5) gitlab-chronic (~> 0.10.5)
gitlab-dangerfiles (~> 2.3.0) gitlab-dangerfiles (~> 2.3.0)
gitlab-experiment (~> 0.6.4) gitlab-experiment (~> 0.6.4)
gitlab-fog-azure-rm (~> 1.1.1) gitlab-fog-azure-rm (~> 1.2.0)
gitlab-labkit (~> 0.21.1) gitlab-labkit (~> 0.21.1)
gitlab-license (~> 2.0) gitlab-license (~> 2.0)
gitlab-mail_room (~> 0.0.9) gitlab-mail_room (~> 0.0.9)
......
...@@ -49,14 +49,8 @@ module CarrierWave ...@@ -49,14 +49,8 @@ module CarrierWave
local_file = local_directory.files.new(key: path) local_file = local_directory.files.new(key: path)
expire_at = options[:expire_at] || ::Fog::Time.now + @uploader.fog_authenticated_url_expiration expire_at = options[:expire_at] || ::Fog::Time.now + @uploader.fog_authenticated_url_expiration
case @uploader.fog_credentials[:provider] case @uploader.fog_credentials[:provider]
when 'AWS', 'Google' when 'AWS', 'Google', 'AzureRM'
# Older versions of fog-google do not support options as a parameter local_file.url(expire_at, options)
if url_options_supported?(local_file)
local_file.url(expire_at, options)
else
warn "Options hash not supported in #{local_file.class}. You may need to upgrade your Fog provider."
local_file.url(expire_at)
end
when 'Rackspace' when 'Rackspace'
connection.get_object_https_url(@uploader.fog_directory, path, expire_at, options) connection.get_object_https_url(@uploader.fog_directory, path, expire_at, options)
when 'OpenStack' when 'OpenStack'
......
...@@ -81,19 +81,32 @@ RSpec.describe 'CarrierWave::Storage::Fog::File' do ...@@ -81,19 +81,32 @@ RSpec.describe 'CarrierWave::Storage::Fog::File' do
end end
describe '#authenticated_url' do describe '#authenticated_url' do
let(:expire_at) { 24.hours.from_now }
let(:options) { { expire_at: expire_at } }
it 'has an authenticated URL' do it 'has an authenticated URL' do
expect(subject.authenticated_url).to eq("https://sa.blob.core.windows.net/test_container/test_blob?token") expect(subject.authenticated_url(options)).to eq("https://sa.blob.core.windows.net/test_container/test_blob?token")
end end
context 'with custom expire_at' do context 'with custom expire_at' do
it 'properly sets expires param' do it 'properly sets expires param' do
expire_at = 24.hours.from_now expect_next_instance_of(Fog::Storage::AzureRM::File) do |file|
expect(file).to receive(:url).with(expire_at, options).and_call_original
end
expect(subject.authenticated_url(options)).to eq("https://sa.blob.core.windows.net/test_container/test_blob?token")
end
end
context 'with content_disposition option' do
let(:options) { { expire_at: expire_at, content_disposition: 'attachment' } }
it 'passes options' do
expect_next_instance_of(Fog::Storage::AzureRM::File) do |file| expect_next_instance_of(Fog::Storage::AzureRM::File) do |file|
expect(file).to receive(:url).with(expire_at).and_call_original expect(file).to receive(:url).with(expire_at, options).and_call_original
end end
expect(subject.authenticated_url(expire_at: expire_at)).to eq("https://sa.blob.core.windows.net/test_container/test_blob?token") expect(subject.authenticated_url(options)).to eq("https://sa.blob.core.windows.net/test_container/test_blob?token")
end end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment