Skip to content
Snippets Groups Projects
Unverified Commit 34c2a59c authored by Alessio Caiazza's avatar Alessio Caiazza
Browse files

Honour workhorse provided file name

In the attempt to unify file uploading at workhorse level gitlab-org/gitlab-workhorse!230
we moved to a prefix-based tempfile creation in order to avoid upload collisions.

Artifacts and LFS uploads already set original_filename to workhorse provided filename

This commit add the same feature to `Gitlab::Middleware::Multipart`
parent 7611ec4e
No related branches found
No related tags found
No related merge requests found
Loading
Loading
@@ -42,7 +42,7 @@ module Gitlab
 
key, value = parsed_field.first
if value.nil?
value = open_file(tmp_path)
value = open_file(tmp_path, @request.params["#{key}.name"])
@open_files << value
else
value = decorate_params_value(value, @request.params[key], tmp_path)
Loading
Loading
@@ -70,7 +70,7 @@ module Gitlab
 
case path_value
when nil
value_hash[path_key] = open_file(tmp_path)
value_hash[path_key] = open_file(tmp_path, value_hash.dig(path_key, '.name'))
@open_files << value_hash[path_key]
value_hash
when Hash
Loading
Loading
@@ -81,8 +81,8 @@ module Gitlab
end
end
 
def open_file(path)
::UploadedFile.new(path, File.basename(path), 'application/octet-stream')
def open_file(path, name)
::UploadedFile.new(path, name || File.basename(path), 'application/octet-stream')
end
end
 
Loading
Loading
Loading
Loading
@@ -5,15 +5,17 @@ require 'tempfile'
describe Gitlab::Middleware::Multipart do
let(:app) { double(:app) }
let(:middleware) { described_class.new(app) }
let(:original_filename) { 'filename' }
 
it 'opens top-level files' do
Tempfile.open('top-level') do |tempfile|
env = post_env({ 'file' => tempfile.path }, { 'file.name' => 'filename' }, Gitlab::Workhorse.secret, 'gitlab-workhorse')
env = post_env({ 'file' => tempfile.path }, { 'file.name' => original_filename }, Gitlab::Workhorse.secret, 'gitlab-workhorse')
 
expect(app).to receive(:call) do |env|
file = Rack::Request.new(env).params['file']
expect(file).to be_a(::UploadedFile)
expect(file.path).to eq(tempfile.path)
expect(file.original_filename).to eq(original_filename)
end
 
middleware.call(env)
Loading
Loading
@@ -34,13 +36,14 @@ describe Gitlab::Middleware::Multipart do
 
it 'opens files one level deep' do
Tempfile.open('one-level') do |tempfile|
in_params = { 'user' => { 'avatar' => { '.name' => 'filename' } } }
in_params = { 'user' => { 'avatar' => { '.name' => original_filename } } }
env = post_env({ 'user[avatar]' => tempfile.path }, in_params, Gitlab::Workhorse.secret, 'gitlab-workhorse')
 
expect(app).to receive(:call) do |env|
file = Rack::Request.new(env).params['user']['avatar']
expect(file).to be_a(::UploadedFile)
expect(file.path).to eq(tempfile.path)
expect(file.original_filename).to eq(original_filename)
end
 
middleware.call(env)
Loading
Loading
@@ -49,13 +52,14 @@ describe Gitlab::Middleware::Multipart do
 
it 'opens files two levels deep' do
Tempfile.open('two-levels') do |tempfile|
in_params = { 'project' => { 'milestone' => { 'themesong' => { '.name' => 'filename' } } } }
in_params = { 'project' => { 'milestone' => { 'themesong' => { '.name' => original_filename } } } }
env = post_env({ 'project[milestone][themesong]' => tempfile.path }, in_params, Gitlab::Workhorse.secret, 'gitlab-workhorse')
 
expect(app).to receive(:call) do |env|
file = Rack::Request.new(env).params['project']['milestone']['themesong']
expect(file).to be_a(::UploadedFile)
expect(file.path).to eq(tempfile.path)
expect(file.original_filename).to eq(original_filename)
end
 
middleware.call(env)
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment