bugfix: url helper loading zip and tar

lib/inspec/targets/url.rb

@@ -12,15 +12,13 @@ module Inspec::Targets
     def handles?(target)
       uri = URI.parse(target)
       return false if uri.nil? or uri.scheme.nil?
-      %{ http https }.include? uri.scheme
+      return false unless %{ http https }.include? uri.scheme
+      true
     rescue URI::Error => _e
       false
     end
 
     def resolve(target, opts = {})
-      # abort if the target does not start with http or https
-      return nil unless target.start_with?('https://', 'http://')
-
       # support for github url
       m = %r{^https?://(www\.)?github\.com/(?<user>[\w-]+)/(?<repo>[\w-]+)(\.git)?(/)?$}.match(target)
       if m
@@ -31,10 +29,14 @@ module Inspec::Targets
       resolve_archive(url, opts)
     end
 
+    private
+
     # download url into archive using opts,
     # returns File object and content-type from HTTP headers
-    def download_archive(url, archive, opts)
+    def download_archive(url, opts)
+      archive = Tempfile.new('inspec-dl-')
       archive.binmode
+
       remote = open(
         url,
         http_basic_authentication: [opts['user'] || '', opts['password'] || ''],
@@ -48,25 +50,40 @@ module Inspec::Targets
       [archive, remote.meta['content-type']]
     end
 
-    def resolve_archive(url, opts)
+    def ensure_suffix(path, suffix)
-      archive, content_type = download_archive(url, Tempfile.new(['inspec-dl-', '.tar.gz']), opts)
+      return path if path.end_with?(suffix)
+      File.rename(path, path + suffix)
+      path + suffix
+    end
 
-      # replace extension with zip if we detected a zip content type
+    def resolve_archive_path(archive_helper, path, opts)
-      if ['application/x-zip-compressed', 'application/zip'].include?(content_type)
+      # this should not happen, catch it in case we have an internal error:
-        # rename file for proper detection in DirHelper
+      unless archive_helper.handles?(path)
-        pn = Pathname.new(archive.path)
+        throw RuntimeError, "Failed to load downloaded archive in #{path} with "\
-        new_path = pn.dirname.join(pn.basename.to_s.gsub('tar.gz', 'zip'))
+              "#{archive_helper}. Internal error, please file a bugreport."
-        File.rename(pn.to_s, new_path.to_s)
-
-        content = ZipHelper.new.resolve(new_path)
-        File.unlink(new_path)
-      elsif ['application/x-gzip', 'application/gzip'].include?(content_type)
-        # use tar helper as default (otherwise returns nil)
-        content = TarHelper.new.resolve(archive.path)
-        archive.unlink
       end
 
-      content
+      res = archive_helper.resolve(path, opts)
+      File.unlink(path)
+      res
+    end
+
+    def resolve_archive(url, opts)
+      archive, content_type = download_archive(url, opts)
+
+      # replace extension with zip if we detected a zip content type
+      case content_type
+      when 'application/x-zip-compressed', 'application/zip'
+        path = ensure_suffix(archive.path, '.zip')
+        resolve_archive_path(ZipHelper, path, opts)
+      when 'application/x-gzip', 'application/gzip'
+        path = ensure_suffix(archive.path, '.tar.gz')
+        resolve_archive_path(TarHelper, path, opts)
+      when nil
+        {}
+      else
+        throw RuntimeError, "Failed to resolve URL target, its metadata did not match ZIP or TAR: #{content_type}"
+      end
     end
 
     def to_s

test/unit/targets/url_test.rb

@@ -3,131 +3,137 @@
 # author: Dominik Richter
 
 require 'helper'
+require 'mocha/setup'
 
 describe Inspec::Targets::UrlHelper do
-  let(:helper) { Inspec::Targets::UrlHelper.new }
+  let(:url_helper) { Inspec::Targets::UrlHelper.new }
 
   it 'handles http' do
-    helper.handles?('http://chef.io').must_equal true
+    url_helper.handles?('http://chef.io').must_equal true
   end
 
   it 'handles https' do
-    helper.handles?('https://chef.io').must_equal true
+    url_helper.handles?('https://chef.io').must_equal true
   end
 
   it 'returns false if given an invalid URL' do
-    helper.handles?('cheshire_cat').must_equal false
+    url_helper.handles?('cheshire_cat').must_equal false
   end
 
   it 'returns false if given an URL with a protocol different from http[s]' do
-    helper.handles?('gopher://chef.io').must_equal false
+    url_helper.handles?('gopher://chef.io').must_equal false
   end
 
   it 'resolves various github urls' do
-    hlpr = Minitest::Mock.new
+    mock = Minitest::Mock.new
-    helper.stub :resolve_zip, hlpr do
+    url_helper.stub :download_archive, mock do
       %w{https://github.com/chef/inspec
          https://github.com/chef/inspec.git
          https://www.github.com/chef/inspec.git
          http://github.com/chef/inspec
          http://github.com/chef/inspec.git
          http://www.github.com/chef/inspec.git}.each do |github|
-        hlpr.expect :call, nil, ['https://github.com/chef/inspec/archive/master.tar.gz', {}]
+        mock.expect :call, nil, ['https://github.com/chef/inspec/archive/master.tar.gz', {}]
 
-        helper.resolve(github)
+        url_helper.resolve(github)
       end
-      hlpr.verify
+      mock.verify
     end
   end
 
   it 'leaves proper, non-github urls unchanged' do
     url = 'https://chef.io/something.tar.gz'
-    hlpr = Minitest::Mock.new
+    mock = Minitest::Mock.new
-    hlpr.expect :call, nil, [url, {}]
+    mock.expect :call, nil, [url, {}]
-    helper.stub :resolve_zip, hlpr do
+    url_helper.stub :download_archive, mock do
-        helper.resolve(url)
+        url_helper.resolve(url)
     end
-    hlpr.verify
+    mock.verify
   end
 
-  let (:url) { 'https://github.com/chef/inspec/archive/master.tar.gz' }
-  let (:opts) { { http_basic_authentication: ['', ''] } }
-
   def archive_of_type(type)
-    archive = Minitest::Mock.new
+    mock = Minitest::Mock.new
-    archive.expect :write, nil, ["#{type}-content"]
+    mock.expect :write, nil, ["#{type}-content"]
-    archive.expect :path, "/path/to/#{type}-archive.tar.gz" # always tar.gz!
+    mock.expect :path, "/path/to/#{type}-archive.tar.gz" # always tar.gz!
     [:binmode, :rewind, :close, :unlink].each do |meth|
-      archive.expect meth, nil
+      mock.expect meth, nil
     end
-    archive
+    mock
   end
 
   def remote_of_type(type, content_type)
-    remote = Minitest::Mock.new
+    mock = Minitest::Mock.new
-    remote.expect :read, "#{type}-content"
+    mock.expect :read, "#{type}-content"
-    remote.expect :meta, { 'content-type' => content_type }
+    mock.expect :meta, { 'content-type' => content_type }
-    remote
+    mock
   end
 
-  let (:archive_sth) { archive_of_type('sth') }
+  describe 'with a funny archive and content-type' do
-  let (:remote_sth) { remote_of_type('sth', 'application/x-very-funny') }
+    let (:url) { 'https://github.com/chef/inspec/archive/master.tar.gz' }
+    let (:remote_mock) { remote_of_type('sth', 'application/x-very-funny') }
 
-  it 'downloads an archive and returns it with its content-type' do
+    it 'will download, but fails at resolving this content-type' do
-    helper.stub :open, remote_sth, [url, opts] do
+      url_helper.expects(:open).returns(remote_mock)
-      helper.download_archive(url, archive_sth, {}).must_equal([archive_sth, 'application/x-very-funny'])
+      proc { url_helper.resolve(url) }.must_throw RuntimeError
+      remote_mock.verify
     end
-    remote_sth.verify
-  end
 
-  it 'downloads an archive and returns it with its content-type using options, too' do
+    it 'downloads an archive and returns it with its content-type using options, too' do
-    helper.stub :open, remote_sth, [url, { http_basic_authentication: ['alice', 'pw'] }] do
+      url_helper.expects(:open).returns(remote_mock)
-      helper.download_archive(url, archive_sth, 'user' => 'alice', 'password' => 'pw').must_equal([archive_sth, 'application/x-very-funny'])
+      r, m = url_helper.method(:download_archive)
-    end
+                       .call(url, 'user' => 'alice', 'password' => 'pw')
-    remote_sth.verify
+      m.must_equal('application/x-very-funny')
-  end
+      r.must_be_kind_of(File)
-
+      r.unlink
-  let (:archive_zip) { archive_of_type('zip') }
+      remote_mock.verify
-  let (:archive_tgz) { archive_of_type('tgz') }
-
-  let (:tarhelper) do
-    th = Minitest::Mock.new
-    th.expect :resolve, 'tgz-content', ['/path/to/tgz-archive.tar.gz']
-    th
-  end
-
-  %w{ application/gzip application/x-gzip }.each do |content_type|
-    it "unpacks a tarball (#{content_type}) with TarHelper and returns the content" do
-      Tempfile.stub :new, archive_tgz, [['inspec-dl-', '.tar.gz']] do
-        helper.stub :download_archive, [archive_tgz, content_type], [url, archive_tgz, opts] do
-          Inspec::Targets::TarHelper.stub :new, tarhelper do
-            helper.resolve_zip(url, {}).must_equal('tgz-content')
-          end
-        end
-      end
-      tarhelper.verify
     end
   end
 
-  let (:ziphelper) do
+  describe 'with a tar.gz archive' do
-    zip = Minitest::Mock.new
+    let (:url) { 'https://github.com/chef/inspec/archive/master.tar.gz' }
-    zip.expect :resolve, 'zip-content', [Pathname.new('/path/to/zip-archive.zip')]
+    let (:profile_path) { MockLoader.profile_tgz('complete-profile') }
-    zip
+    let (:archive_path) { profile_path.sub(/.tgz$/, '')[1..-1] }
+
+    it 'resolves the url' do
+      url_helper.expects(:download_archive).returns([File.new(profile_path), 'application/x-gzip'])
+      res = url_helper.resolve(url)
+      # TODO: the leading '/' is removed due to tar-handling; this should be
+      # a different ref altogether containing the right relative path of the tar
+
+      res.must_be_kind_of Array
+      res.length.must_equal 2
+
+      res[0][:type].must_equal :test
+      res[0][:content].wont_be_empty
+      res[0][:ref].must_equal "#{archive_path}/controls/filesystem_spec.rb"
+
+      res[1][:type].must_equal :metadata
+      res[1][:content].wont_be_empty
+      res[1][:ref].must_equal "#{archive_path}/inspec.yml"
+    end
   end
 
-  %w{ application/zip application/x-zip-compressed }.each do |content_type|
+  describe 'with a zip archive' do
-    it "renames and unpacks a zip file (#{content_type}) with ZipHelper and returns the content" do
+    let (:url) { 'https://github.com/chef/inspec/archive/master.zip' }
-      helper.stub :download_archive, [archive_zip, content_type], [url, archive_zip, opts] do
+    let (:profile_path) { MockLoader.profile_zip('complete-profile') }
-        Tempfile.stub :new, archive_zip, [['inspec-dl-', '.tar.gz']] do
+    let (:archive_path) { profile_path.sub(/.zip$/, '')[1..-1] }
-          File.stub :rename, nil, ['/path/to/zip-archive.tar.gz', '/path/to/zip-archive.zip'] do
+
-            Inspec::Targets::ZipHelper.stub :new, ziphelper do
+    it 'resolves the url' do
-              File.stub :unlink, nil, ['/path/to/zip-archive.zip'] do
+      url_helper.expects(:download_archive).returns([File.new(profile_path), 'application/zip'])
-                helper.resolve_zip(url, {}).must_equal('zip-content')
+      res = url_helper.resolve(url)
-              end
+      # TODO: the leading '/' is removed due to tar-handling; this should be
-            end
+      # a different ref altogether containing the right relative path of the tar
-          end
+
-        end
+      res.must_be_kind_of Array
-      end
+      res.length.must_equal 2
+
+      res[0][:type].must_equal :test
+      res[0][:content].wont_be_empty
+      res[0][:ref].must_equal "#{archive_path}/controls/filesystem_spec.rb"
+
+      res[1][:type].must_equal :metadata
+      res[1][:content].wont_be_empty
+      res[1][:ref].must_equal "#{archive_path}/inspec.yml"
     end
   end
 end