diff --git a/app/helpers/workhorse_helper.rb b/app/helpers/workhorse_helper.rb index f74b53d68a1b8bd8dc51258a3e09753aa07314a4..28dd1b002926a7abf45254f4b0b92513868b0e79 100644 --- a/app/helpers/workhorse_helper.rb +++ b/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 @@ def set_workhorse_internal_api_content_type 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 diff --git a/config/feature_flags/development/attachment_with_filename.yml b/config/feature_flags/development/attachment_with_filename.yml new file mode 100644 index 0000000000000000000000000000000000000000..8d3a96404ef9a37caa834c5609b12d3b4bd6908e --- /dev/null +++ b/config/feature_flags/development/attachment_with_filename.yml @@ -0,0 +1,8 @@ +--- +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 diff --git a/spec/controllers/projects/design_management/designs/raw_images_controller_spec.rb b/spec/controllers/projects/design_management/designs/raw_images_controller_spec.rb index f664604ac15d900fcb241aac330acb288f76213d..e0f86876f670139d28421a534d1537b6912269dd 100644 --- a/spec/controllers/projects/design_management/designs/raw_images_controller_spec.rb +++ b/spec/controllers/projects/design_management/designs/raw_images_controller_spec.rb @@ -37,13 +37,24 @@ # 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 diff --git a/spec/support/shared_examples/controllers/snippet_blob_shared_examples.rb b/spec/support/shared_examples/controllers/snippet_blob_shared_examples.rb index c3e8f807afb82f460a8c1be165ab2c3ca11559cc..62aaec851620a693ee3fdc305d20a777a7ec6a2e 100644 --- a/spec/support/shared_examples/controllers/snippet_blob_shared_examples.rb +++ b/spec/support/shared_examples/controllers/snippet_blob_shared_examples.rb @@ -17,6 +17,38 @@ 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' }