Merge branch 'fj-55882-fix-files-api-content-disposition' into 'master'

Fix files/blob api endpoint content disposition

Closes #55882

See merge request gitlab-org/gitlab-ce!24267

5 files changed, 34 insertions, 19 deletions

diff --git a/changelogs/unreleased/fj-55882-fix-files-api-content-disposition.yml b/changelogs/unreleased/fj-55882-fix-files-api-content-disposition.yml
new file mode 100644
index 00000000000..f64b29644b0
--- /dev/null
+++ b/changelogs/unreleased/fj-55882-fix-files-api-content-disposition.yml
@@ -0,0 +1,5 @@
+---
+title: Fix files/blob api endpoints content disposition
+merge_request: 24267
+author:
+type: fixed
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index aae54fb34bc..74927b4db81 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -496,7 +496,11 @@ module API
     def send_git_blob(repository, blob)
       env['api.format'] = :txt
       content_type 'text/plain'
-      header['Content-Disposition'] = content_disposition('attachment', blob.name)
+      header['Content-Disposition'] = content_disposition('inline', blob.name)
+
+      # Let Workhorse examine the content and determine the better content disposition
+      header[Gitlab::Workhorse::DETECT_HEADER] = "true"
+
       header(*Gitlab::Workhorse.send_git_blob(repository, blob))
     end
 
diff --git a/spec/lib/api/helpers_spec.rb b/spec/lib/api/helpers_spec.rb
index 1c73a936e17..e1aea82653d 100644
--- a/spec/lib/api/helpers_spec.rb
+++ b/spec/lib/api/helpers_spec.rb
@@ -150,32 +150,36 @@ describe API::Helpers do
   end
 
   describe '#send_git_blob' do
-    context 'content disposition' do
-      let(:repository) { double }
-      let(:blob) { double(name: 'foobar') }
+    let(:repository) { double }
+    let(:blob) { double(name: 'foobar') }
 
-      let(:send_git_blob) do
-        subject.send(:send_git_blob, repository, blob)
-      end
+    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
+    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
+
+    it 'sets Gitlab::Workhorse::DETECT_HEADER header' do
+      expect(send_git_blob[Gitlab::Workhorse::DETECT_HEADER]).to eq "true"
+    end
 
+    context 'content disposition' do
       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'
+          expect(send_git_blob['Content-Disposition']).to eq 'inline'
         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"'
+          expect(send_git_blob['Content-Disposition']).to eq 'inline; filename="foobar"'
         end
       end
     end
diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb
index a0aee937185..9b32dc78274 100644
--- a/spec/requests/api/files_spec.rb
+++ b/spec/requests/api/files_spec.rb
@@ -183,14 +183,15 @@ describe API::Files do
         get api(url, current_user), params: params
 
         expect(response).to have_gitlab_http_status(200)
+        expect(headers[Gitlab::Workhorse::DETECT_HEADER]).to eq "true"
       end
 
-      it 'forces attachment content disposition' do
+      it 'sets inline content disposition by default' do
         url = route(file_path) + "/raw"
 
         get api(url, current_user), params: params
 
-        expect(headers['Content-Disposition']).to eq('attachment; filename="popen.rb"')
+        expect(headers['Content-Disposition']).to eq('inline; 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 b6b57803a6a..0adc95cfbeb 100644
--- a/spec/requests/api/repositories_spec.rb
+++ b/spec/requests/api/repositories_spec.rb
@@ -166,12 +166,13 @@ describe API::Repositories do
         get api(route, current_user)
 
         expect(response).to have_gitlab_http_status(200)
+        expect(headers[Gitlab::Workhorse::DETECT_HEADER]).to eq "true"
       end
 
-      it 'forces attachment content disposition' do
+      it 'sets inline content disposition by default' do
         get api(route, current_user)
 
-        expect(headers['Content-Disposition']).to eq 'attachment'
+        expect(headers['Content-Disposition']).to eq 'inline'
       end
 
       context 'when sha does not exist' do