We write uploads also on disk when Object Storage direct upload is enabled
The following discussion from !18855 (merged) should be addressed:
-
@nolith started a discussion: (+1 comment) @ayufan we are still sending a valid
TempPath
so the artifact is written to the diskI found
UploadFile
not able to handle remote only files so I had to hack it a bitdiff --git a/app/uploaders/object_storage.rb b/app/uploaders/object_storage.rb index eb708e260fb..7921b310e8f 100644 --- a/app/uploaders/object_storage.rb +++ b/app/uploaders/object_storage.rb @@ -163,6 +163,7 @@ module ObjectStorage end def workhorse_local_upload_path + return if self.object_store_enabled? && self.direct_upload_enabled? File.join(self.root, TMP_UPLOAD_PATH) end diff --git a/lib/uploaded_file.rb b/lib/uploaded_file.rb index 5dc85b2baea..c898359e022 100644 --- a/lib/uploaded_file.rb +++ b/lib/uploaded_file.rb @@ -16,36 +16,44 @@ class UploadedFile attr_reader :remote_id attr_reader :sha256 + attr_reader :size - def initialize(path, filename: nil, content_type: "application/octet-stream", sha256: nil, remote_id: nil) - raise InvalidPathError, "#{path} file does not exist" unless ::File.exist?(path) + def initialize(path, filename: nil, content_type: "application/octet-stream", sha256: nil, remote_id: nil, size: nil) + if remote_id.blank? + raise InvalidPathError, "#{path} file does not exist" unless ::File.exist?(path) + + @tempfile = File.new(path, 'rb') + @size = @tempfile.size + else + @size = size + end @content_type = content_type @original_filename = filename || ::File.basename(path) @content_type = content_type @sha256 = sha256 @remote_id = remote_id - @tempfile = File.new(path, 'rb') end def self.from_params(params, field, upload_path) - unless params["#{field}.path"] - raise InvalidPathError, "file is invalid" if params["#{field}.remote_id"] - - return - end + file_path = nil - file_path = File.realpath(params["#{field}.path"]) + if params["#{field}.path"] + file_path = File.realpath(params["#{field}.path"]) - unless self.allowed_path?(file_path, [upload_path, Dir.tmpdir].compact) - raise InvalidPathError, "insecure path used '#{file_path}'" + unless self.allowed_path?(file_path, [upload_path, Dir.tmpdir].compact) + raise InvalidPathError, "insecure path used '#{file_path}'" + end + else + raise InvalidPathError, "file is invalid" if params["#{field}.remote_id"].blank? end UploadedFile.new(file_path, filename: params["#{field}.name"], content_type: params["#{field}.type"] || 'application/octet-stream', sha256: params["#{field}.sha256"], - remote_id: params["#{field}.remote_id"]) + remote_id: params["#{field}.remote_id"], + size: params["#{field}.size"]) end def self.allowed_path?(file_path, paths)
Should I push my changes?
Edited by Alessio Caiazza