diff options
author | Sean McGivern <sean@gitlab.com> | 2018-12-31 13:42:54 +0000 |
---|---|---|
committer | Sean McGivern <sean@gitlab.com> | 2018-12-31 13:42:54 +0000 |
commit | ca14b70d5201852751d79d6a0827b81689fff5be (patch) | |
tree | 63f8f322f9bdbb2f288c48676eb16b5906399cdf | |
parent | a352a95e9a4e6e925e43e1b0a3dc2cac6d33cef4 (diff) | |
parent | 2cd47bba9a4ee1b503b37c548826ef527c4e49ba (diff) | |
download | gitlab-ce-ca14b70d5201852751d79d6a0827b81689fff5be.tar.gz |
Merge branch 'fj-55781-fix-api-blob-content-disposition' into 'master'
Fixed content-disposition in blob and files API endpoint
Closes #55781
See merge request gitlab-org/gitlab-ce!24078
-rw-r--r-- | changelogs/unreleased/fj-55781-fix-api-blob-content-disposition.yml | 5 | ||||
-rw-r--r-- | lib/api/helpers.rb | 8 | ||||
-rw-r--r-- | spec/lib/api/helpers_spec.rb | 32 | ||||
-rw-r--r-- | spec/requests/api/files_spec.rb | 2 | ||||
-rw-r--r-- | spec/requests/api/repositories_spec.rb | 2 |
5 files changed, 46 insertions, 3 deletions
diff --git a/changelogs/unreleased/fj-55781-fix-api-blob-content-disposition.yml b/changelogs/unreleased/fj-55781-fix-api-blob-content-disposition.yml new file mode 100644 index 00000000000..2e1d9889b22 --- /dev/null +++ b/changelogs/unreleased/fj-55781-fix-api-blob-content-disposition.yml @@ -0,0 +1,5 @@ +--- +title: Fix content-disposition in blobs and files API endpoint +merge_request: 24078 +author: +type: fixed diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index c3eca713712..6c1a730935a 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -496,7 +496,7 @@ module API def send_git_blob(repository, blob) env['api.format'] = :txt content_type 'text/plain' - header['Content-Disposition'] = "attachment; filename=#{blob.name.inspect}" + header['Content-Disposition'] = content_disposition('attachment', blob.name) header(*Gitlab::Workhorse.send_git_blob(repository, blob)) end @@ -529,5 +529,11 @@ module API params[:archived] end + + def content_disposition(disposition, filename) + disposition += %(; filename=#{filename.inspect}) if filename.present? + + disposition + end end end diff --git a/spec/lib/api/helpers_spec.rb b/spec/lib/api/helpers_spec.rb index 58a49124ce6..1c73a936e17 100644 --- a/spec/lib/api/helpers_spec.rb +++ b/spec/lib/api/helpers_spec.rb @@ -148,4 +148,36 @@ describe API::Helpers do it_behaves_like 'user namespace finder' end + + describe '#send_git_blob' do + context 'content disposition' do + let(:repository) { double } + let(:blob) { double(name: 'foobar') } + + let(:send_git_blob) do + subject.send(:send_git_blob, repository, blob) + end + + before do + allow(subject).to receive(:env).and_return({}) + allow(subject).to receive(:content_type) + allow(subject).to receive(:header).and_return({}) + allow(Gitlab::Workhorse).to receive(:send_git_blob) + end + + context 'when blob name is null' do + let(:blob) { double(name: nil) } + + it 'returns only the disposition' do + expect(send_git_blob['Content-Disposition']).to eq 'attachment' + end + end + + context 'when blob name is not null' do + it 'returns disposition with the blob name' do + expect(send_git_blob['Content-Disposition']).to eq 'attachment; filename="foobar"' + end + end + end + end end diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb index 0ba1f2d7a2b..a0aee937185 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -190,7 +190,7 @@ describe API::Files do get api(url, current_user), params: params - expect(headers['Content-Disposition']).to match(/^attachment/) + expect(headers['Content-Disposition']).to eq('attachment; filename="popen.rb"') end context 'when mandatory params are not given' do diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb index 181fe6246ae..b6b57803a6a 100644 --- a/spec/requests/api/repositories_spec.rb +++ b/spec/requests/api/repositories_spec.rb @@ -171,7 +171,7 @@ describe API::Repositories do it 'forces attachment content disposition' do get api(route, current_user) - expect(headers['Content-Disposition']).to match(/^attachment/) + expect(headers['Content-Disposition']).to eq 'attachment' end context 'when sha does not exist' do |