Don't send HTTP headers that have nil values (#1948)

Net::HTTP does not gracefully handle HTTP options/headers
that have nil values. This updates Fetchers::Url to verify
that all headers we attempt to configure have non-nil,
non-empty values.

This originally surfaced via the audit cookbook with the
chef-automate fetcher in use without the data_collector
token being set.

Signed-off-by: Adam Leff <adam@leff.co>

lib/fetchers/url.rb

@@ -143,21 +143,6 @@ module Fetchers
     def download_archive_to_temp
       return @temp_archive_path if ! @temp_archive_path.nil?
       Inspec::Log.debug("Fetching URL: #{@target}")
-      http_opts = {}
-      http_opts['ssl_verify_mode'.to_sym] = OpenSSL::SSL::VERIFY_NONE if @insecure
-      if @config
-        if @config['server_type'] == 'automate'
-          http_opts['chef-delivery-enterprise'] = @config['automate']['ent']
-          if @config['automate']['token_type'] == 'dctoken'
-            http_opts['x-data-collector-token'] = @config['token']
-          else
-            http_opts['chef-delivery-user'] = @config['user']
-            http_opts['chef-delivery-token'] = @config['token']
-          end
-        elsif @token
-          http_opts['Authorization'] = "Bearer #{@token}"
-        end
-      end
       remote = open(@target, http_opts)
       @archive_type = file_type_from_remote(remote) # side effect :(
       archive = Tempfile.new(['inspec-dl-', @archive_type])
@@ -178,5 +163,42 @@ module Fetchers
       @temp_archive_path = nil
       final_path
     end
+
+    def http_opts
+      opts = {}
+      opts[:ssl_verify_mode] = OpenSSL::SSL::VERIFY_NONE if @insecure
+
+      if @config['server_type'] == 'automate'
+        opts['chef-delivery-enterprise'] = @config['automate']['ent']
+        if @config['automate']['token_type'] == 'dctoken'
+          opts['x-data-collector-token'] = @config['token']
+        else
+          opts['chef-delivery-user'] = @config['user']
+          opts['chef-delivery-token'] = @config['token']
+        end
+      elsif @token
+        opts['Authorization'] = "Bearer #{@token}"
+      end
+
+      # Do not send any headers that have nil values.
+      # Net::HTTP does not gracefully handle this situation.
+      check_for_missing_values!(opts)
+
+      opts
+    end
+
+    def check_for_missing_values!(opts)
+      keys_missing_values = opts.keys.delete_if do |k|
+        if opts[k].nil?
+          false
+        elsif opts[k].respond_to?(:empty?) && opts[k].empty?
+          false
+        else
+          true
+        end
+      end
+      raise 'Unable to fetch profile - the following HTTP headers have no value: ' \
+        "#{keys_missing_values.join(', ')}" unless keys_missing_values.empty?
+    end
   end
 end

test/unit/fetchers/url_test.rb

@@ -134,4 +134,136 @@ describe Fetchers::Url do
       subject.resolved_source[:url].must_equal(target)
     end
   end
+
+  describe '#http_opts' do
+    let(:subject) { Fetchers::Url.new('fake_url', config) }
+
+    describe 'when insecure is specified' do
+      let(:config) { { 'insecure' => true } }
+      it 'returns a hash containing an ssl_verify_mode setting' do
+        subject.send(:http_opts)[:ssl_verify_mode].must_equal OpenSSL::SSL::VERIFY_NONE
+      end
+    end
+
+    describe 'when insecure is not specific' do
+      let(:config) { {} }
+      it 'returns a hash that does not contain an ssl_verify_mode setting' do
+        subject.send(:http_opts).key?(:ssl_verify_mode).must_equal false
+      end
+    end
+
+    describe 'when the server is an automate server using dctoken' do
+      describe 'when the config is properly populated' do
+        let(:config) do
+          {
+            'server_type' => 'automate',
+            'automate' => {
+              'ent' => 'my_ent',
+              'token_type' => 'dctoken',
+            },
+            'token' => 'my_token',
+          }
+        end
+
+        it 'returns a properly formatted headers hash' do
+          headers = subject.send(:http_opts)
+          headers['chef-delivery-enterprise'].must_equal 'my_ent'
+          headers['x-data-collector-token'].must_equal 'my_token'
+        end
+      end
+
+      describe 'when the enterprise is not supplied' do
+        it 'raises an exception' do
+          proc {
+            config = {
+              'server_type' => 'automate',
+              'automate' => { 'token_type' => 'dctoken' },
+              'token' => 'my_token',
+            }
+
+            Fetchers::Url.new('fake_url', config).send(:http_opts)
+          }.must_raise RuntimeError
+        end
+      end
+
+      describe 'when the token is not supplied' do
+        it 'raises an exception' do
+          proc {
+            config = {
+              'server_type' => 'automate',
+              'automate' => {
+                'ent' => 'my_ent',
+                'token_type' => 'dctoken',
+              },
+            }
+
+            Fetchers::Url.new('fake_url', config).send(:http_opts)
+          }.must_raise RuntimeError
+        end
+      end
+    end
+
+    describe 'when the server is an automate server not using dctoken' do
+      describe 'when the config is properly populated' do
+        let(:config) do
+          {
+            'server_type' => 'automate',
+            'automate' => {
+              'ent' => 'my_ent',
+              'token_type' => 'usertoken',
+            },
+            'user' => 'my_user',
+            'token' => 'my_token',
+          }
+        end
+        it 'returns a properly formatted headers hash' do
+          headers = subject.send(:http_opts)
+          headers['chef-delivery-enterprise'].must_equal 'my_ent'
+          headers['chef-delivery-user'].must_equal 'my_user'
+          headers['chef-delivery-token'].must_equal 'my_token'
+        end
+      end
+
+      describe 'when the user is not supplied' do
+        it 'raises an exception' do
+          proc {
+            config = {
+              'server_type' => 'automate',
+              'automate' => {
+                'ent' => 'my_ent',
+                'token_type' => 'usertoken',
+              },
+              'token' => 'my_token',
+            }
+
+            Fetchers::Url.new('fake_url', config).send(:http_opts)
+          }.must_raise RuntimeError
+        end
+      end
+
+      describe 'when the token is not supplied' do
+        it 'raises an exception' do
+          proc {
+            config = {
+              'server_type' => 'automate',
+              'automate' => {
+                'ent' => 'my_ent',
+                'token_type' => 'usertoken',
+              },
+              'user' => 'my_user',
+            }
+
+            Fetchers::Url.new('fake_url', config).send(:http_opts)
+          }.must_raise RuntimeError
+        end
+      end
+    end
+
+    describe 'when only a token is supplied' do
+      let(:config) { { 'token' => 'my_token' } }
+      it 'returns a hash containing an Authorization header' do
+        subject.send(:http_opts)['Authorization'].must_equal "Bearer my_token"
+      end
+    end
+  end
 end