halostatue / minitar

Minimal pure-ruby support for POSIX tar(1) archives.
Other
38 stars 27 forks source link

r10k puppetfile install failing #24

Closed lngarrett closed 7 years ago

lngarrett commented 7 years ago

r10k uses minitar.

r10k puppetfile install
ERROR    -> closed gzip stream

I noticed a new version of minitar in my gemlist at the exact moment my deployments started experiencing this error. I think the most recent release is causing this.

halostatue commented 7 years ago

I use neither Puppet nor r10k. Can you please provide more information about what these are doing that they would be using minitar?

lngarrett commented 7 years ago
Archive::Tar::Minitar.unpack(File.join(fixture_path, 'puppet-boolean-bare.tar'), remote_path)

https://github.com/puppetlabs/r10k/search?utf8=%E2%9C%93&q=minitar

halostatue commented 7 years ago

So…I downloaded puppet-boolean-bare.tar and used Minitar.unpack(File.expand_path('~/Downloads/puppet-boolean-bare.tar'), 'puppet-boolean-bare') to unpack the file with no problem. There’s something expecting a compressed stream in the code, but I’m not seeing it.

As I said, I don’t use Puppet, so I’m at a loss here as to what’s going on. There’s a similar issue being discussed at elastic/logstash#6657 and elastic/logstash#6659, so there’s something going on with the gzip handling, but I have tests around this so I’m at a loss.

Which version of Ruby are you on?

lngarrett commented 7 years ago
ruby 2.0.0p648 (2015-12-16) [x86_64-linux]
halostatue commented 7 years ago

Hm. Is there a Vagrantfile you can recommend for me to test this against? This shouldn’t be failing by all appearances, but I’m not familiar with the r10k project and I don’t want to install this outside of a sandbox environment.

sharkannon commented 7 years ago

My office is having the same issue.

ruby 2.1.5p273 (2014-11-13 revision 48405) [x86_64-linux]

[xxxxx ~]$ more /etc/centos-release CentOS release 6.6 (Final)

tar-1.23-11.el6.x86_64

halostatue commented 7 years ago

@sharkannon I cannot help without something (like a Vagrantfile) that can help me reproduce and debug the issue as I do not use r10k or Puppet. I’m happy to debug (during working hours), but I don’t even know where to get started with r10k and I have no time to learn it nor interest in doing so. The only thing I want to do is fix this issue.

sharkannon commented 7 years ago

it wasn't actually r10k that was my issue, sorry.. we just use it like:

    def unzip_artifact_file file
        gzip = Zlib::GzipReader.new(file)
        input = Archive::Tar::Minitar::Input.new(gzip)
        input.each { |entry|
          input.extract_entry(@tmp_directory, entry)
        }
    end
halostatue commented 7 years ago

Excellent. Let me try that code to see if I can get a similar failure on my master branch.

halostatue commented 7 years ago

@sharkannon So, I have minitar-cli checked out next to minitar in my repo, and minitar-cli has a sample tarball.

I have slightly modified your code to make this work as a test script:

require 'zlib'
require 'archive/tar/minitar'

@tmp_directory = './tmp'

def unzip_artifact_file file
  gzip = Zlib::GzipReader.new(file)
  input = Archive::Tar::Minitar::Input.new(gzip)
  input.each { |entry|
    input.extract_entry(@tmp_directory, entry)
  }
end

unzip_artifact_file open(ARGV[0])

I then ran this with ruby -Ilib -rarchive/tar/minitar test.rb ../minitar-cli/test/fixtures/spaces.tar.gz. No exceptions, but I got this result from find tmp (which was expected):

tmp
tmp/.autotest
tmp/.byebug_history
tmp/.coveralls.yml
tmp/.gemtest
tmp/.gitignore
tmp/.hoerc
tmp/.pullreview.yml
tmp/.rubocop.yml
tmp/.simplecov-prelude.rb
tmp/.travis.yml
tmp/appveyor.yml
tmp/bin
tmp/bin/minitar
tmp/Code-of-Conduct.md
tmp/Contributing.md
tmp/docs
tmp/docs/bsdl.txt
tmp/docs/ruby.txt
tmp/Gemfile
tmp/Gemfile.lock
tmp/History.md
tmp/lib
tmp/lib/minitar
tmp/lib/minitar/cli
tmp/lib/minitar/cli/command
tmp/lib/minitar/cli/command/create.rb
tmp/lib/minitar/cli/command/extract.rb
tmp/lib/minitar/cli/command/help.rb
tmp/lib/minitar/cli/command/list.rb
tmp/lib/minitar/cli/command.rb
tmp/lib/minitar/cli/commander.rb
tmp/lib/minitar/cli.rb
tmp/Licence.md
tmp/Manifest.txt
tmp/minitar-cli.gemspec
tmp/Rakefile
tmp/README.rdoc

Your code could be simplified to:

def unzip_artifact_file(file)
  Archive::Tar::Minitar.unpack(Zlib::GzipReader.new(file), @tmp_directory)
end

A safer way of doing this if you don’t want the “quick, unpack *this” behaviour, would be:

def unzip_artifact_file file
  Archive::Tar::Minitar::Input.open(Zlib::GzipReader.new(file)) do |input|
    input.each { |entry| input.extract_entry(@tmp_directory, entry) }
  end
end

As it is, you’re leaking Zlib::GzipReader objects and open handles (and probably files, too). Both versions I show will end up calling File#close on the file passed into the method, which is probably what you want to happen because you’re otherwise leaking open files.

There may be something with your tarball, but I can’t get this to fail on my Mac—and I don’t have a good CentOS Vagrantfile around to test on something similar to what you have.

jordansissel commented 7 years ago

Reproducing:

% touch /tmp/a
% tar -zcf a.tar.gz /tmp/a

Error in JRuby 1.7.26:

% ruby -rminitar -rzlib -e 'tgz = Zlib::GzipReader.new(File.new(ARGV[0])); tar = Archive::Tar::Minitar::Input.open(tgz); tar.each { |e| }' a.tar.gz
IOError: closed stream
        seek at org/jruby/RubyIO.java:1826
      rewind at org/jruby/ext/zlib/JZlibRubyGzipReader.java:205
      rewind at /home/jls/.rvm/gems/jruby-1.7.26/gems/minitar-0.6/lib/archive/tar/minitar/reader.rb:182
  each_entry at /home/jls/.rvm/gems/jruby-1.7.26/gems/minitar-0.6/lib/archive/tar/minitar/input.rb:89
      (root) at -e:1

Error in Ruby 2.3.0

% ruby -rminitar -rzlib -e 'tgz = Zlib::GzipReader.new(File.new(ARGV[0])); tar = Archive::Tar::Minitar::Input.open(tgz); tar.each { |e| }' a.tar.gz

/home/jls/.rvm/gems/ruby-2.3.0/gems/minitar-0.6/lib/archive/tar/minitar/reader.rb:182:in `rewind': closed gzip stream (Zlib::GzipFile::Error)
        from /home/jls/.rvm/gems/ruby-2.3.0/gems/minitar-0.6/lib/archive/tar/minitar/reader.rb:182:in `rewind'
        from /home/jls/.rvm/gems/ruby-2.3.0/gems/minitar-0.6/lib/archive/tar/minitar/input.rb:89:in `each_entry'
        from -e:1:in `<main>'

Testing on minitar 0.5.4, the above example works.

sharkannon commented 7 years ago

@halostatue I couldn't get it to fail on my MAC either.. only failed on Centos6 with the versions I listed (at least for me)

jordansissel commented 7 years ago

Interestingly, minitar-cli seems to work OK?

% ruby -rzlib =minitar list a.tar.gz
tmp/a

(I had to run with ruby -rzlib to work around a separate issue uninitialized constant Archive::Tar::Minitar::CLI::Command::List::Zlib)

halostatue commented 7 years ago

I’ll have a fix for this in five minutes or so. I moved the position of an ensure block in various #open and it closes the stream. The main difference is that I mostly use the block forms when I’m working with this so that I don’t have dangling file handles:

ruby -rminitar -rzlib -e 'tgz = Zlib::GzipReader.new(File.new(ARGV[0])); Archive::Tar::Minitar::Input.open(tgz) { |tar| tar.each { |e| }}' a.tar.gz
jordansissel commented 7 years ago

minitar-cli uses Archive::Tar::Minitar::Input.each_entry. My tests are using Input.open(). If I switch to use #each_entry then it works.

jordansissel commented 7 years ago

@halostatue awesome! thank you for this effort <3

halostatue commented 7 years ago

Input.open takes a block or not; if you use a block, it works. If you don’t it doesn’t, because I removed a rescue context.

halostatue commented 7 years ago

Well, moved, not removed.

halostatue commented 7 years ago

@jordansissel Can I get some eyeballs on halostatue/minitar-cli#2 and the linked pull request here as soon as I have it fixed?

jordansissel commented 7 years ago

@halostatue Yep! I'm available for PR testing.

halostatue commented 7 years ago

PR on #25 should fix this. Your command-line worked once I remembered to add -Ilib (otherwise it grabs the installed gem).