Automatic merge of gitlab-org/gitlab-ce master

changelogs/unreleased/fj-55882-fix-files-api-content-disposition.yml

+---
+title: Fix files/blob api endpoints content disposition
+merge_request: 24267
+author:
+type: fixed

lib/api/helpers.rb

@@ -535,7 +535,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
 

scripts/review_apps/review-apps.sh

@@ -31,7 +31,9 @@ function ensure_namespace() {
 
 function install_tiller() {
   echo "Checking Tiller..."
-  helm init --upgrade
+  helm init \
+    --upgrade \
+    --replicas 2
   kubectl rollout status -n "$TILLER_NAMESPACE" -w "deployment/tiller-deploy"
   if ! helm version --debug; then
     echo "Failed to init Tiller."

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

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

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