Commit 579ac9d7 authored by Kamil Trzciński's avatar Kamil Trzciński Committed by Alessio Caiazza

Merge branch 'jprovazn-fix-filename' into 'master'

Fix filename for accelerated uploads

See merge request gitlab-org/gitlab-ce!20672
parent 35c387cb
......@@ -132,10 +132,6 @@ class FileUploader < GitlabUploader
}
end
def filename
self.file.filename
end
def upload=(value)
super
......
---
title: Fix filename for accelerated uploads
merge_request:
author:
type: fixed
......@@ -21,7 +21,7 @@ class UploadedFile
raise InvalidPathError, "#{path} file does not exist" unless ::File.exist?(path)
@content_type = content_type
@original_filename = filename || ::File.basename(path)
@original_filename = sanitize_filename(filename || path)
@content_type = content_type
@sha256 = sha256
@remote_id = remote_id
......@@ -55,6 +55,16 @@ class UploadedFile
end
end
# copy-pasted from CarrierWave::SanitizedFile
def sanitize_filename(name)
name = name.tr("\\", "/") # work-around for IE
name = ::File.basename(name)
name = name.gsub(CarrierWave::SanitizedFile.sanitize_regexp, "_")
name = "_#{name}" if name =~ /\A\.+\z/
name = "unnamed" if name.empty?
name.mb_chars.to_s
end
def path
@tempfile.path
end
......
require 'spec_helper'
describe UploadedFile do
describe ".from_params" do
let(:temp_dir) { Dir.tmpdir }
let(:temp_file) { Tempfile.new("test", temp_dir) }
let(:upload_path) { nil }
subject do
described_class.from_params(params, :file, upload_path)
end
before do
FileUtils.touch(temp_file)
......@@ -16,9 +10,19 @@ describe UploadedFile do
after do
FileUtils.rm_f(temp_file)
end
describe ".from_params" do
let(:upload_path) { nil }
after do
FileUtils.rm_r(upload_path) if upload_path
end
subject do
described_class.from_params(params, :file, upload_path)
end
context 'when valid file is specified' do
context 'only local path is specified' do
let(:params) do
......@@ -37,7 +41,7 @@ describe UploadedFile do
context 'all parameters are specified' do
let(:params) do
{ 'file.path' => temp_file.path,
'file.name' => 'my_file.txt',
'file.name' => 'dir/my file&.txt',
'file.type' => 'my/type',
'file.sha256' => 'sha256',
'file.remote_id' => 'remote_id' }
......@@ -48,7 +52,7 @@ describe UploadedFile do
end
it "generates filename from path" do
expect(subject.original_filename).to eq('my_file.txt')
expect(subject.original_filename).to eq('my_file_.txt')
expect(subject.content_type).to eq('my/type')
expect(subject.sha256).to eq('sha256')
expect(subject.remote_id).to eq('remote_id')
......@@ -113,4 +117,11 @@ describe UploadedFile do
end
end
end
describe '#sanitize_filename' do
it { expect(described_class.new(temp_file.path).sanitize_filename('spaced name')).to eq('spaced_name') }
it { expect(described_class.new(temp_file.path).sanitize_filename('#$%^&')).to eq('_____') }
it { expect(described_class.new(temp_file.path).sanitize_filename('..')).to eq('_..') }
it { expect(described_class.new(temp_file.path).sanitize_filename('')).to eq('unnamed') }
end
end
......@@ -157,4 +157,50 @@ describe FileUploader do
uploader.upload = upload
end
end
describe '#cache!' do
subject do
uploader.store!(uploaded_file)
end
context 'when remote file is used' do
let(:temp_file) { Tempfile.new("test") }
let!(:fog_connection) do
stub_uploads_object_storage(described_class)
end
let(:uploaded_file) do
UploadedFile.new(temp_file.path, filename: "my file.txt", remote_id: "test/123123")
end
let!(:fog_file) do
fog_connection.directories.get('uploads').files.create(
key: 'tmp/uploads/test/123123',
body: 'content'
)
end
before do
FileUtils.touch(temp_file)
end
after do
FileUtils.rm_f(temp_file)
end
it 'file is stored remotely in permament location with sanitized name' do
subject
expect(uploader).to be_exists
expect(uploader).not_to be_cached
expect(uploader).not_to be_file_storage
expect(uploader.path).not_to be_nil
expect(uploader.path).not_to include('tmp/upload')
expect(uploader.path).not_to include('tmp/cache')
expect(uploader.url).to include('/my_file.txt')
expect(uploader.object_store).to eq(described_class::Store::REMOTE)
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