Commit 523f69ee authored by Lin Jen-Shin's avatar Lin Jen-Shin

Merge branch '53357-fix-plus-in-upload-file-names' into 'master'

Use GitlabUploader#filename when generating upload URLs

See merge request gitlab-org/gitlab-ce!29915
parents ffdb1c1b 76b0518f
...@@ -203,6 +203,6 @@ class FileUploader < GitlabUploader ...@@ -203,6 +203,6 @@ class FileUploader < GitlabUploader
end end
def secure_url def secure_url
File.join('/uploads', @secret, file.filename) File.join('/uploads', @secret, filename)
end end
end end
...@@ -93,6 +93,6 @@ class PersonalFileUploader < FileUploader ...@@ -93,6 +93,6 @@ class PersonalFileUploader < FileUploader
end end
def secure_url def secure_url
File.join('/', base_dir, secret, file.filename) File.join('/', base_dir, secret, filename)
end end
end end
---
title: Fix broken URLs for uploads with a plus in the filename
merge_request: 29915
author:
type: fixed
...@@ -184,11 +184,6 @@ describe FileUploader do ...@@ -184,11 +184,6 @@ describe FileUploader do
end end
end end
describe '#cache!' do
subject do
uploader.store!(uploaded_file)
end
context 'when remote file is used' do context 'when remote file is used' do
let(:temp_file) { Tempfile.new("test") } let(:temp_file) { Tempfile.new("test") }
...@@ -196,8 +191,9 @@ describe FileUploader do ...@@ -196,8 +191,9 @@ describe FileUploader do
stub_uploads_object_storage(described_class) stub_uploads_object_storage(described_class)
end end
let(:filename) { "my file.txt" }
let(:uploaded_file) do let(:uploaded_file) do
UploadedFile.new(temp_file.path, filename: "my file.txt", remote_id: "test/123123") UploadedFile.new(temp_file.path, filename: filename, remote_id: "test/123123")
end end
let!(:fog_file) do let!(:fog_file) do
...@@ -209,15 +205,16 @@ describe FileUploader do ...@@ -209,15 +205,16 @@ describe FileUploader do
before do before do
FileUtils.touch(temp_file) FileUtils.touch(temp_file)
uploader.store!(uploaded_file)
end end
after do after do
FileUtils.rm_f(temp_file) FileUtils.rm_f(temp_file)
end end
describe '#cache!' do
it 'file is stored remotely in permament location with sanitized name' do it 'file is stored remotely in permament location with sanitized name' do
subject
expect(uploader).to be_exists expect(uploader).to be_exists
expect(uploader).not_to be_cached expect(uploader).not_to be_cached
expect(uploader).not_to be_file_storage expect(uploader).not_to be_file_storage
...@@ -228,5 +225,18 @@ describe FileUploader do ...@@ -228,5 +225,18 @@ describe FileUploader do
expect(uploader.object_store).to eq(described_class::Store::REMOTE) expect(uploader.object_store).to eq(described_class::Store::REMOTE)
end end
end end
describe '#to_h' do
subject { uploader.to_h }
let(:filename) { 'my+file.txt' }
it 'generates URL using original file name instead of filename returned by object storage' do
# GCS returns a URL with a `+` instead of `%2B`
allow(uploader.file).to receive(:url).and_return('https://storage.googleapis.com/gitlab-test-uploads/@hashed/6b/86/6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b/64c5065e62100b1a12841644256a98be/my+file.txt')
expect(subject[:url]).to end_with(filename)
end
end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment