github-mirror/discourse
2
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2025-06-06 23:07:28 +08:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

FIX: Use ActionDispatch::Http::ContentDisposition for uploads content-disposition (#10108)

Browse Source 
See https://meta.discourse.org/t/broken-pipe-error-when-uploading-to-a-s3-clone-a-pdf-with-a-name-containing-e-i-etc/155414

When setting content-disposition for attachment, use the ContentDisposition class to format it. This handles filenames with weird characters and localization (accented characters) correctly.
This commit is contained in:
Martin Brennan
2020-06-23 17:10:56 +10:00
committed by GitHub
parent 97f045d06a
commit e92909aa77
4 changed files with 10 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
lib/file_store/s3_store.rb
View File

@ -59,7 +59,9 @@ module FileStore
# HTML players, and when a direct link is provided to any file but an image # HTML players, and when a direct link is provided to any file but an image
# it will download correctly in the browser. # it will download correctly in the browser.
if !FileHelper.is_supported_image?(filename) if !FileHelper.is_supported_image?(filename)
options[:content_disposition] = "attachment; filename=\"#{filename}\"" options[:content_disposition] = ActionDispatch::Http::ContentDisposition.format(
disposition: "attachment", filename: filename
)
end end
path.prepend(File.join(upload_path, "/")) if Rails.configuration.multisite path.prepend(File.join(upload_path, "/")) if Rails.configuration.multisite

5
lib/file_store/to_s3_migration.rb
View File

@ -224,8 +224,9 @@ module FileStore
upload = Upload.find_by(url: "/#{file}") upload = Upload.find_by(url: "/#{file}")
if upload&.original_filename if upload&.original_filename
options[:content_disposition] = options[:content_disposition] = ActionDispatch::Http::ContentDisposition.format(
%Q{attachment; filename="#{upload.original_filename}"} disposition: "attachment", filename: upload.original_filename
)
end end
if upload&.secure if upload&.secure

2
spec/components/file_store/s3_store_spec.rb
View File

@ -91,7 +91,7 @@ describe FileStore::S3Store do
acl: "private", acl: "private",
cache_control: "max-age=31556952, public, immutable", cache_control: "max-age=31556952, public, immutable",
content_type: "application/pdf", content_type: "application/pdf",
content_disposition: "attachment; filename=\"#{upload.original_filename}\"", content_disposition: "attachment; filename=\"#{upload.original_filename}\"; filename*=UTF-8''#{upload.original_filename}",
body: uploaded_file).returns(Aws::S3::Types::PutObjectOutput.new(etag: "\"#{etag}\"")) body: uploaded_file).returns(Aws::S3::Types::PutObjectOutput.new(etag: "\"#{etag}\""))
expect(store.store_upload(uploaded_file, upload)).to eq( expect(store.store_upload(uploaded_file, upload)).to eq(

6
spec/multisite/s3_store_spec.rb
View File

@ -44,7 +44,7 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do
let(:uploaded_file) { file_from_fixtures("small.pdf", "pdf") } let(:uploaded_file) { file_from_fixtures("small.pdf", "pdf") }
it "adds an attachment content-disposition with the original filename" do it "adds an attachment content-disposition with the original filename" do
disp_opts = { content_disposition: "attachment; filename=\"#{original_filename}\"", content_type: "application/pdf" } disp_opts = { content_disposition: "attachment; filename=\"#{original_filename}\"; filename*=UTF-8''#{original_filename}", content_type: "application/pdf" }
s3_helper.expects(:upload).with(uploaded_file, kind_of(String), upload_opts.merge(disp_opts)).returns(["path", "etag"]) s3_helper.expects(:upload).with(uploaded_file, kind_of(String), upload_opts.merge(disp_opts)).returns(["path", "etag"])
upload = build_upload upload = build_upload
store.store_upload(uploaded_file, upload) store.store_upload(uploaded_file, upload)
@ -56,7 +56,7 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do
let(:uploaded_file) { file_from_fixtures("small.mp4", "media") } let(:uploaded_file) { file_from_fixtures("small.mp4", "media") }
it "adds an attachment content-disposition with the original filename" do it "adds an attachment content-disposition with the original filename" do
disp_opts = { content_disposition: "attachment; filename=\"#{original_filename}\"", content_type: "application/mp4" } disp_opts = { content_disposition: "attachment; filename=\"#{original_filename}\"; filename*=UTF-8''#{original_filename}", content_type: "application/mp4" }
s3_helper.expects(:upload).with(uploaded_file, kind_of(String), upload_opts.merge(disp_opts)).returns(["path", "etag"]) s3_helper.expects(:upload).with(uploaded_file, kind_of(String), upload_opts.merge(disp_opts)).returns(["path", "etag"])
upload = build_upload upload = build_upload
store.store_upload(uploaded_file, upload) store.store_upload(uploaded_file, upload)
@ -68,7 +68,7 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do
let(:uploaded_file) { file_from_fixtures("small.mp3", "media") } let(:uploaded_file) { file_from_fixtures("small.mp3", "media") }
it "adds an attachment content-disposition with the original filename" do it "adds an attachment content-disposition with the original filename" do
disp_opts = { content_disposition: "attachment; filename=\"#{original_filename}\"", content_type: "audio/mpeg" } disp_opts = { content_disposition: "attachment; filename=\"#{original_filename}\"; filename*=UTF-8''#{original_filename}", content_type: "audio/mpeg" }
s3_helper.expects(:upload).with(uploaded_file, kind_of(String), upload_opts.merge(disp_opts)).returns(["path", "etag"]) s3_helper.expects(:upload).with(uploaded_file, kind_of(String), upload_opts.merge(disp_opts)).returns(["path", "etag"])
upload = build_upload upload = build_upload
store.store_upload(uploaded_file, upload) store.store_upload(uploaded_file, upload)