From b42a9cbecbef30cc0e1c5780aa3881b477e15307 Mon Sep 17 00:00:00 2001
From: Athar Hameed <athar@gitlab.com>
Date: Tue, 9 Mar 2021 10:00:18 +0000
Subject: [PATCH] Return attachment plus the filename in the
Content-Disposition header [RUN ALL RSPEC] [RUN AS-IF-FOSS]
---
app/helpers/workhorse_helper.rb | 10 +++++-
.../development/attachment_with_filename.yml | 8 +++++
.../designs/raw_images_controller_spec.rb | 15 +++++++--
.../snippet_blob_shared_examples.rb | 32 +++++++++++++++++++
4 files changed, 62 insertions(+), 3 deletions(-)
create mode 100644 config/feature_flags/development/attachment_with_filename.yml
diff --git a/app/helpers/workhorse_helper.rb b/app/helpers/workhorse_helper.rb
index f74b53d68a1b8..28dd1b002926a 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 0000000000000..8d3a96404ef9a
--- /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 f664604ac15d9..e0f86876f6701 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 c3e8f807afb82..62aaec851620a 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' }
--
GitLab