Commit 249c2489 authored by Jan Provaznik's avatar Jan Provaznik

Updated multipart to support workhorse direct uploads

parent ad086fa8
...@@ -42,10 +42,10 @@ module Gitlab ...@@ -42,10 +42,10 @@ module Gitlab
key, value = parsed_field.first key, value = parsed_field.first
if value.nil? if value.nil?
value = open_file(tmp_path, @request.params["#{key}.name"]) value = open_file(@request.params, key)
@open_files << value @open_files << value
else else
value = decorate_params_value(value, @request.params[key], tmp_path) value = decorate_params_value(value, @request.params[key])
end end
@request.update_param(key, value) @request.update_param(key, value)
...@@ -57,7 +57,7 @@ module Gitlab ...@@ -57,7 +57,7 @@ module Gitlab
end end
# This function calls itself recursively # This function calls itself recursively
def decorate_params_value(path_hash, value_hash, tmp_path) def decorate_params_value(path_hash, value_hash)
unless path_hash.is_a?(Hash) && path_hash.count == 1 unless path_hash.is_a?(Hash) && path_hash.count == 1
raise "invalid path: #{path_hash.inspect}" raise "invalid path: #{path_hash.inspect}"
end end
...@@ -70,19 +70,21 @@ module Gitlab ...@@ -70,19 +70,21 @@ module Gitlab
case path_value case path_value
when nil when nil
value_hash[path_key] = open_file(tmp_path, value_hash.dig(path_key, '.name')) value_hash[path_key] = open_file(value_hash.dig(path_key), '')
@open_files << value_hash[path_key] @open_files << value_hash[path_key]
value_hash value_hash
when Hash when Hash
decorate_params_value(path_value, value_hash[path_key], tmp_path) decorate_params_value(path_value, value_hash[path_key])
value_hash value_hash
else else
raise "unexpected path value: #{path_value.inspect}" raise "unexpected path value: #{path_value.inspect}"
end end
end end
def open_file(path, name) def open_file(params, key)
::UploadedFile.new(path, filename: name || File.basename(path), content_type: 'application/octet-stream') ::UploadedFile.from_params(
params, key,
Gitlab.config.uploads.storage_path)
end end
end end
......
...@@ -7,44 +7,23 @@ describe Gitlab::Middleware::Multipart do ...@@ -7,44 +7,23 @@ describe Gitlab::Middleware::Multipart do
let(:middleware) { described_class.new(app) } let(:middleware) { described_class.new(app) }
let(:original_filename) { 'filename' } let(:original_filename) { 'filename' }
shared_examples_for 'multipart upload files' do
it 'opens top-level files' do it 'opens top-level files' do
Tempfile.open('top-level') do |tempfile| Tempfile.open('top-level') do |tempfile|
env = post_env({ 'file' => tempfile.path }, { 'file.name' => original_filename }, Gitlab::Workhorse.secret, 'gitlab-workhorse') env = post_env({ 'file' => tempfile.path }, { 'file.name' => original_filename, 'file.path' => tempfile.path, 'file.remote_id' => remote_id }, Gitlab::Workhorse.secret, 'gitlab-workhorse')
expect(app).to receive(:call) do |env| expect_uploaded_file(tempfile, %w(file))
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) middleware.call(env)
end end
end end
it 'rejects headers signed with the wrong secret' do
env = post_env({ 'file' => '/var/empty/nonesuch' }, {}, 'x' * 32, 'gitlab-workhorse')
expect { middleware.call(env) }.to raise_error(JWT::VerificationError)
end
it 'rejects headers signed with the wrong issuer' do
env = post_env({ 'file' => '/var/empty/nonesuch' }, {}, Gitlab::Workhorse.secret, 'acme-inc')
expect { middleware.call(env) }.to raise_error(JWT::InvalidIssuerError)
end
it 'opens files one level deep' do it 'opens files one level deep' do
Tempfile.open('one-level') do |tempfile| Tempfile.open('one-level') do |tempfile|
in_params = { 'user' => { 'avatar' => { '.name' => original_filename } } } in_params = { 'user' => { 'avatar' => { '.name' => original_filename, '.path' => tempfile.path, '.remote_id' => remote_id } } }
env = post_env({ 'user[avatar]' => tempfile.path }, in_params, Gitlab::Workhorse.secret, 'gitlab-workhorse') env = post_env({ 'user[avatar]' => tempfile.path }, in_params, Gitlab::Workhorse.secret, 'gitlab-workhorse')
expect(app).to receive(:call) do |env| expect_uploaded_file(tempfile, %w(user avatar))
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) middleware.call(env)
end end
...@@ -52,18 +31,48 @@ describe Gitlab::Middleware::Multipart do ...@@ -52,18 +31,48 @@ describe Gitlab::Middleware::Multipart do
it 'opens files two levels deep' do it 'opens files two levels deep' do
Tempfile.open('two-levels') do |tempfile| Tempfile.open('two-levels') do |tempfile|
in_params = { 'project' => { 'milestone' => { 'themesong' => { '.name' => original_filename } } } } in_params = { 'project' => { 'milestone' => { 'themesong' => { '.name' => original_filename, '.path' => tempfile.path, '.remote_id' => remote_id } } } }
env = post_env({ 'project[milestone][themesong]' => tempfile.path }, in_params, Gitlab::Workhorse.secret, 'gitlab-workhorse') env = post_env({ 'project[milestone][themesong]' => tempfile.path }, in_params, Gitlab::Workhorse.secret, 'gitlab-workhorse')
expect_uploaded_file(tempfile, %w(project milestone themesong))
middleware.call(env)
end
end
def expect_uploaded_file(tempfile, path, remote: false)
expect(app).to receive(:call) do |env| expect(app).to receive(:call) do |env|
file = Rack::Request.new(env).params['project']['milestone']['themesong'] file = Rack::Request.new(env).params.dig(*path)
expect(file).to be_a(::UploadedFile) expect(file).to be_a(::UploadedFile)
expect(file.path).to eq(tempfile.path) expect(file.path).to eq(tempfile.path)
expect(file.original_filename).to eq(original_filename) expect(file.original_filename).to eq(original_filename)
expect(file.remote_id).to eq(remote_id)
end
end
end end
middleware.call(env) it 'rejects headers signed with the wrong secret' do
env = post_env({ 'file' => '/var/empty/nonesuch' }, {}, 'x' * 32, 'gitlab-workhorse')
expect { middleware.call(env) }.to raise_error(JWT::VerificationError)
end end
it 'rejects headers signed with the wrong issuer' do
env = post_env({ 'file' => '/var/empty/nonesuch' }, {}, Gitlab::Workhorse.secret, 'acme-inc')
expect { middleware.call(env) }.to raise_error(JWT::InvalidIssuerError)
end
context 'with remote file' do
let(:remote_id) { 'someid' }
it_behaves_like 'multipart upload files'
end
context 'with local file' do
let(:remote_id) { nil }
it_behaves_like 'multipart upload files'
end end
def post_env(rewritten_fields, params, secret, issuer) def post_env(rewritten_fields, params, secret, issuer)
......
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