chef/artifactory-client

upload() on repository resource is not uploading correct artifact in matching size

Closed this issue · 3 comments

I have a .jar file which is like 47MB (don't ask why it's so big). We are using the upload() under lib/artifactory/resources/repository.rb to upload. We found the uploaded artifact only has ~30 bytes or something in size. We did some debugging and found there could be an issue on how this file is passed.

I'm not sure why file has to pass in the hash { file: file} :

  def upload(path_or_io, path, properties = {}, headers = {})
      file = if path_or_io.is_a?(File)
               path_or_io
             else
               File.new(File.expand_path(path_or_io))
             end

      matrix   = to_matrix_properties(properties)
      endpoint = File.join("#{url_safe(key)}#{matrix}", path)

      response = client.put(endpoint, { file: file }, headers)
      Resource::Artifact.from_hash(response)
    end

But changing it to this will work:

    def upload(path_or_io, path, properties = {}, headers = {})
      file = if path_or_io.is_a?(File)
               path_or_io
             else
               File.new(File.expand_path(path_or_io))
             end

      matrix   = to_matrix_properties(properties)
      endpoint = File.join("#{url_safe(key)}#{matrix}", path)

      headers = { "Content-Length" => "#{File.size(file)}" }
      response = client.put(endpoint, file, headers)
      Resource::Artifact.from_hash(response)
    end

Since I'm not entirely sure this is by design to pass in a hash or not. I cannot file this as a bug, but this definitely need to be addressed.

If we look further at lib/artifactory/client.rb , the passed hash will be setting as a form data which obviously don't work in this case by simply giving a File object.

      # Setup PATCH/POST/PUT
      if [:patch, :post, :put].include?(verb)
        if data.respond_to?(:read)
          request.body_stream = data
        elsif data.is_a?(Hash)
          request.form_data = data
        else
          request.body = data
        end
      end

I could have this fix, but I prefer to discuss whether this is by design or not.

Thanks

This is a bug, but I'm not sure why our tests aren't catching it...

@sethvargo I guess the test probably couldn't catch it, because it does indeed get uploaded, but just not in the correct size. Not sure if Rspec can also add checks for that stub file's size too.

I've locally fix it, but would like to see how you want to have it fix. Because I'm on a private repo and cannot push back to Github now.

@xbeta I'd like to see how I can test it properly