Merge pull request #5938 from joshcooper/ticket/master/PUP-3940-accep…

…t-mime-types

(PUP-3940) Send proper mime types in the Accept header

lib/puppet/indirector/rest.rb

@@ -10,6 +10,7 @@ class Puppet::Indirector::REST < Puppet::Indirector::Terminus
   include Puppet::Network::HTTP::Compression.module
 
   IndirectedRoutes = Puppet::Network::HTTP::API::IndirectedRoutes
+  EXCLUDED_FORMATS = [:yaml, :b64_zlib_yaml, :dot]
 
   class << self
     attr_reader :server_setting, :port_setting
@@ -101,11 +102,10 @@ def self.port
   # Provide appropriate headers.
   def headers
     # yaml is not allowed on the network
-    network_formats = model.supported_formats.reject do |format|
-      [:yaml, :b64_zlib_yaml].include?(format)
-    end
+    network_formats = model.supported_formats - EXCLUDED_FORMATS
+    mime_types = network_formats.map { |f| model.get_format(f).mime }
     common_headers = {
-      "Accept"                                     => network_formats.join(", "),
+      "Accept"                                     => mime_types.join(', '),
       Puppet::Network::HTTP::HEADER_PUPPET_VERSION => Puppet.version
     }
 

lib/puppet/type/file/source.rb

@@ -16,6 +16,10 @@ module Puppet
   Puppet::Type.type(:file).newparam(:source) do
     include Puppet::Network::HTTP::Compression.module
 
+    BINARY_MIME_TYPES = [
+      Puppet::Network::FormatHandler.format_for('binary').mime
+    ].join(', ').freeze
+
     attr_accessor :source, :local
     desc <<-'EOT'
       A source file, which will be copied into place on the local system. This
@@ -295,7 +299,7 @@ def get_from_puppet_source(source_uri, content_uri, &block)
 
       request.do_request(:fileserver) do |req|
         connection = Puppet::Network::HttpPool.http_instance(req.server, req.port)
-        connection.request_get(Puppet::Network::HTTP::API::IndirectedRoutes.request_to_uri(req), add_accept_encoding({"Accept" => "binary"}), &block)
+        connection.request_get(Puppet::Network::HTTP::API::IndirectedRoutes.request_to_uri(req), add_accept_encoding({"Accept" => BINARY_MIME_TYPES}), &block)
       end
     end
 

spec/unit/indirector/rest_spec.rb

@@ -278,13 +278,19 @@ def save_request(key, instance, options={})
   it 'excludes yaml from the Accept header' do
     model.expects(:supported_formats).returns([:json, :pson, :yaml, :binary])
 
-    expect(terminus.headers['Accept']).to eq('json, pson, binary')
+    expect(terminus.headers['Accept']).to eq('application/json, text/pson, application/octet-stream')
   end
 
   it 'excludes b64_zlib_yaml from the Accept header' do
     model.expects(:supported_formats).returns([:json, :pson, :b64_zlib_yaml])
 
-    expect(terminus.headers['Accept']).to eq('json, pson')
+    expect(terminus.headers['Accept']).to eq('application/json, text/pson')
+  end
+
+  it 'excludes dot from the Accept header' do
+    model.expects(:supported_formats).returns([:json, :dot])
+
+    expect(terminus.headers['Accept']).to eq('application/json')
   end
 
   describe "when creating an HTTP client" do
@@ -432,10 +438,10 @@ def save_request(key, instance, options={})
       expect(terminus.find(request)).to eq(model.new('name', 'decoded body'))
     end
 
-    it "provides an Accept header containing the list of supported formats joined with commas" do
-      connection.expects(:get).with(anything, has_entry("Accept" => "supported, formats")).returns(response)
+    it "provides an Accept header containing the list of supported mime types joined with commas" do
+      connection.expects(:get).with(anything, has_entry("Accept" => "application/json, text/pson")).returns(response)
 
-      terminus.model.expects(:supported_formats).returns %w{supported formats}
+      terminus.model.expects(:supported_formats).returns [:json, :pson]
       terminus.find(request)
     end
 
@@ -538,9 +544,9 @@ def save_request(key, instance, options={})
     end
 
     it "should provide an Accept header containing the list of supported formats joined with commas" do
-      connection.expects(:get).with(anything, has_entry("Accept" => "supported, formats")).returns(mock_response(200, ''))
+      connection.expects(:get).with(anything, has_entry("Accept" => "application/json, text/pson")).returns(mock_response(200, ''))
 
-      terminus.model.expects(:supported_formats).returns %w{supported formats}
+      terminus.model.expects(:supported_formats).returns [:json, :pson]
       terminus.search(request)
     end
 
@@ -597,9 +603,9 @@ def save_request(key, instance, options={})
     end
 
     it "should provide an Accept header containing the list of supported formats joined with commas" do
-      connection.expects(:delete).with(anything, has_entry("Accept" => "supported, formats")).returns(response)
+      connection.expects(:delete).with(anything, has_entry("Accept" => "application/json, text/pson")).returns(response)
 
-      terminus.model.expects(:supported_formats).returns %w{supported formats}
+      terminus.model.expects(:supported_formats).returns [:json, :pson]
       terminus.destroy(request)
     end
 
@@ -657,10 +663,10 @@ def save_request(key, instance, options={})
     end
 
     it "should provide an Accept header containing the list of supported formats joined with commas" do
-      connection.expects(:put).with(anything, anything, has_entry("Accept" => "supported, formats")).returns(response)
+      connection.expects(:put).with(anything, anything, has_entry("Accept" => "application/json, text/pson")).returns(response)
 
       instance.expects(:render).returns('')
-      model.expects(:supported_formats).returns %w{supported formats}
+      model.expects(:supported_formats).returns [:json, :pson]
       instance.expects(:mime).returns "supported"
 
       terminus.save(request)

spec/unit/type/file/source_spec.rb

@@ -612,6 +612,19 @@
         resource.write(source)
       end
 
+      it 'should request binary content' do
+        response.stubs(:code).returns('200')
+        Puppet::Network::HttpPool.stubs(:http_instance).returns(conn)
+        source.stubs(:metadata).returns stub_everything('metadata', :source => 'puppet:///test/foo bar', :ftype => 'file')
+
+        conn.unstub(:request_get)
+        conn.expects(:request_get).with do |_, options|
+          expect(options).to include('Accept' => 'application/octet-stream')
+        end.yields(response)
+
+        resource.write(source)
+      end
+
       describe 'when handling file_content responses' do
         before do
           File.open(filename, 'w') {|f| f.write "initial file content"}