Merge branch '297381-snippet-download-with-correct-extension' into 'master'

Return attachment plus the filename in the Content-Disposition header [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!55066

app/helpers/workhorse_helper.rb

@@ -7,7 +7,7 @@ module WorkhorseHelper
   def send_git_blob(repository, blob, inline: true)
     headers.store(*Gitlab::Workhorse.send_git_blob(repository, blob))
 
-    headers['Content-Disposition'] = inline ? 'inline' : 'attachment'
+    headers['Content-Disposition'] = inline ? 'inline' : content_disposition_attachment(repository.project, blob.name)
 
     # If enabled, this will override the values set above
     workhorse_set_content_type!
@@ -48,4 +48,12 @@ module WorkhorseHelper
   def workhorse_set_content_type!
     headers[Gitlab::Workhorse::DETECT_HEADER] = "true"
   end
+
+  def content_disposition_attachment(project, filename)
+    if Feature.enabled?(:attachment_with_filename, project, default_enabled: :yaml)
+      ActionDispatch::Http::ContentDisposition.format(disposition: 'attachment', filename: filename)
+    else
+      'attachment'
+    end
+  end
 end

config/feature_flags/development/attachment_with_filename.yml

+---
+name: attachment_with_filename
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/55066
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/323714
+milestone: '13.10'
+type: development
+group: group::editor
+default_enabled: false

spec/controllers/projects/design_management/designs/raw_images_controller_spec.rb

@@ -37,13 +37,24 @@ RSpec.describe Projects::DesignManagement::Designs::RawImagesController do
       # For security, .svg images should only ever be served with Content-Disposition: attachment.
       # If this specs ever fails we must assess whether we should be serving svg images.
       # See https://gitlab.com/gitlab-org/gitlab/issues/12771
-      it 'serves files with `Content-Disposition: attachment`' do
+      it 'serves files with `Content-Disposition` header set to attachment plus the filename' do
         subject
 
-        expect(response.header['Content-Disposition']).to eq('attachment')
+        expect(response.header['Content-Disposition']).to match "attachment; filename=\"#{design.filename}\""
         expect(response).to have_gitlab_http_status(:ok)
       end
 
+      context 'when the feature flag attachment_with_filename is disabled' do
+        it 'serves files with just `attachment` in the disposition header' do
+          stub_feature_flags(attachment_with_filename: false)
+
+          subject
+
+          expect(response.header['Content-Disposition']).to eq('attachment')
+          expect(response).to have_gitlab_http_status(:ok)
+        end
+      end
+
       it 'serves files with Workhorse' do
         subject
 

spec/support/shared_examples/controllers/snippet_blob_shared_examples.rb

@@ -17,6 +17,38 @@ RSpec.shared_examples 'raw snippet blob' do
     end
   end
 
+  context 'Content Disposition' do
+    context 'when the disposition is inline' do
+      let(:inline) { true }
+
+      it 'returns inline in the content disposition header' do
+        subject
+
+        expect(response.header['Content-Disposition']).to eq('inline')
+      end
+    end
+
+    context 'when the disposition is attachment' do
+      let(:inline) { false }
+
+      it 'returns attachment plus the filename in the content disposition header' do
+        subject
+
+        expect(response.header['Content-Disposition']).to match "attachment; filename=\"#{filepath}\""
+      end
+
+      context 'when the feature flag attachment_with_filename is disabled' do
+        it 'returns just attachment in the disposition header' do
+          stub_feature_flags(attachment_with_filename: false)
+
+          subject
+
+          expect(response.header['Content-Disposition']).to eq 'attachment'
+        end
+      end
+    end
+  end
+
   context 'with invalid file path' do
     let(:filepath) { 'doesnotexist' }