From 2cd47bba9a4ee1b503b37c548826ef527c4e49ba Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Francisco=20Javier=20L=C3=B3pez?= <fjlopez@gitlab.com>
Date: Mon, 31 Dec 2018 10:13:09 +0100
Subject: [PATCH] Fixed api content-disposition in blob and files endpoint

---
 ...55781-fix-api-blob-content-disposition.yml |  5 +++
 lib/api/helpers.rb                            |  8 ++++-
 scripts/prepare_build.sh                      |  2 +-
 spec/lib/api/helpers_spec.rb                  | 32 +++++++++++++++++++
 spec/requests/api/files_spec.rb               |  2 +-
 spec/requests/api/repositories_spec.rb        |  2 +-
 6 files changed, 47 insertions(+), 4 deletions(-)
 create mode 100644 changelogs/unreleased/fj-55781-fix-api-blob-content-disposition.yml

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/scripts/prepare_build.sh b/scripts/prepare_build.sh
index d85c53090a4..58b74f2f07d 100644
--- a/scripts/prepare_build.sh
+++ b/scripts/prepare_build.sh
@@ -10,7 +10,7 @@ fi
 
 # Only install knapsack after bundle install! Otherwise oddly some native
 # gems could not be found under some circumstance. No idea why, hours wasted.
-retry gem install knapsack --no-ri --no-rdoc
+retry gem install knapsack --no-document
 
 cp config/gitlab.yml.example config/gitlab.yml
 sed -i 's/bin_path: \/usr\/bin\/git/bin_path: \/usr\/local\/bin\/git/' config/gitlab.yml
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