Fix #2392 - use Content-Type for REST communication

There were two problems:
 * server->client communications is using Content-Type with the
direct format name instead of the format mime-type.
 * client->server communications is not using Content-Type to
send the format of the serialized object. Instead it is using the
first member of the Accept header. The Accept header is usually
reserved for the other side, ie what the client will accept
when the server will respond.

This patch makes sure s->c communication contains correct Content-Type
headers.

This patch also adds a Content-Type header containing the mime-type of
the object sent by the client when saving.

Signed-off-by: Brice Figureau <brice-puppet@daysofwonder.com>

6 files changed, 72 insertions, 18 deletions

diff --git a/lib/puppet/indirector/rest.rb b/lib/puppet/indirector/rest.rb
index 6903f9a0d..909be9a45 100644
--- a/lib/puppet/indirector/rest.rb
+++ b/lib/puppet/indirector/rest.rb
@@ -81,7 +81,7 @@ class Puppet::Indirector::REST < Puppet::Indirector::Terminus
 
     def save(request)
         raise ArgumentError, "PUT does not accept options" unless request.options.empty?
-        deserialize network(request).put(indirection2uri(request), request.instance.render, headers)
+        deserialize network(request).put(indirection2uri(request), request.instance.render, headers.merge({ "Content-Type" => request.instance.mime }))
     end
 
     private
diff --git a/lib/puppet/network/format_handler.rb b/lib/puppet/network/format_handler.rb
index 0a7e9dcad..17b863fda 100644
--- a/lib/puppet/network/format_handler.rb
+++ b/lib/puppet/network/format_handler.rb
@@ -23,7 +23,7 @@ module Puppet::Network::FormatHandler
             @format = format
         end
 
-        [:intern, :intern_multiple, :render, :render_multiple].each do |method|
+        [:intern, :intern_multiple, :render, :render_multiple, :mime].each do |method|
             define_method(method) do |*args|
                 protect(method, args)
             end
@@ -65,11 +65,28 @@ module Puppet::Network::FormatHandler
     # Use a delegator to make sure any exceptions generated by our formats are
     # handled intelligently.
     def self.protected_format(name)
+        name = format_to_canonical_name(name)
         @format_protectors ||= {}
         @format_protectors[name] ||= FormatProtector.new(name)
         @format_protectors[name]
     end
 
+    # Return a format name given:
+    #  * a format name
+    #  * a mime-type
+    #  * a format instance
+    def self.format_to_canonical_name(format)
+        case format
+        when Puppet::Network::Format
+            out = format
+        when %r{\w+/\w+}
+            out = mime(format)
+        when
+            out = format(format)
+        end
+        out.name
+    end
+
     module ClassMethods
         def format_handler
             Puppet::Network::FormatHandler
@@ -123,6 +140,12 @@ module Puppet::Network::FormatHandler
             Puppet::Network::FormatHandler.protected_format(format).render(self)
         end
 
+        def mime(format = nil)
+            format ||= self.class.default_format
+
+            Puppet::Network::FormatHandler.protected_format(format).mime
+        end
+
         def support_format?(name)
             self.class.support_format?(name)
         end
diff --git a/lib/puppet/network/http/handler.rb b/lib/puppet/network/http/handler.rb
index 7a2408c67..4df2c4141 100644
--- a/lib/puppet/network/http/handler.rb
+++ b/lib/puppet/network/http/handler.rb
@@ -17,8 +17,14 @@ module Puppet::Network::HTTP::Handler
         raise NotImplementedError
     end
 
-    # Which format to use when serializing our response.  Just picks
-    # the first value in the accept header, at this point.
+    # Retrieve the Content-Type header from the http request.
+    def content_type_header(request)
+        raise NotImplementedError
+    end
+
+    # Which format to use when serializing our response or interpreting the request.  
+    # IF the client provided a Content-Type use this, otherwise use the Accept header
+    # and just pick the first value.
     def format_to_use(request)
         unless header = accept_header(request)
             raise ArgumentError, "An Accept header must be provided to pick the right format"
@@ -28,12 +34,25 @@ module Puppet::Network::HTTP::Handler
         header.split(/,\s*/).each do |name|
             next unless format = Puppet::Network::FormatHandler.format(name)
             next unless format.suitable?
-            return name
+            return format
         end
 
         raise "No specified acceptable formats (%s) are functional on this machine" % header
     end
 
+    def request_format(request)
+        if header = content_type_header(request)
+            format = Puppet::Network::FormatHandler.mime(header)
+            return format.name.to_s if format.suitable?
+        end
+
+        raise "No Content-Type header was received, it isn't possible to unserialize the request"
+    end
+
+    def format_to_mime(format)
+        format.is_a?(Puppet::Network::Format) ? format.mime : format
+    end
+
     def initialize_for_puppet(server)
         @server = server
     end
@@ -106,9 +125,7 @@ module Puppet::Network::HTTP::Handler
     def do_destroy(indirection_request, request, response)
         result = indirection_request.model.destroy(indirection_request.key, indirection_request.to_hash)
 
-        set_content_type(response, "yaml")
-
-        set_response(response, result.to_yaml)
+        return_yaml_response(response, result)
     end
 
     # Execute our save.
@@ -116,14 +133,10 @@ module Puppet::Network::HTTP::Handler
         data = body(request).to_s
         raise ArgumentError, "No data to save" if !data or data.empty?
 
-        format = format_to_use(request)
-
-        obj = indirection_request.model.convert_from(format_to_use(request), data)
+        format = request_format(request)
+        obj = indirection_request.model.convert_from(format, data)
         result = save_object(indirection_request, obj)
-
-        set_content_type(response, "yaml")
-
-        set_response(response, result.to_yaml)
+        return_yaml_response(response, result)
     end
 
     # resolve node name from peer's ip address
@@ -139,6 +152,11 @@ module Puppet::Network::HTTP::Handler
 
   private
 
+    def return_yaml_response(response, body)
+        set_content_type(response, Puppet::Network::FormatHandler.format("yaml"))
+        set_response(response, body.to_yaml)
+    end
+
     # LAK:NOTE This has to be here for testing; it's a stub-point so
     # we keep infinite recursion from happening.
     def save_object(ind_request, object)
diff --git a/lib/puppet/network/http/mongrel/rest.rb b/lib/puppet/network/http/mongrel/rest.rb
index 369d4c6c4..fe1ed1088 100644
--- a/lib/puppet/network/http/mongrel/rest.rb
+++ b/lib/puppet/network/http/mongrel/rest.rb
@@ -15,6 +15,10 @@ class Puppet::Network::HTTP::MongrelREST < Mongrel::HttpHandler
         request.params[ACCEPT_HEADER]
     end
 
+    def content_type_header(request)
+        request.params["HTTP_CONTENT_TYPE"]
+    end
+
     # which HTTP verb was used in this request
     def http_method(request)
         request.params[Mongrel::Const::REQUEST_METHOD]
@@ -41,7 +45,7 @@ class Puppet::Network::HTTP::MongrelREST < Mongrel::HttpHandler
     end
 
     def set_content_type(response, format)
-        response.header['Content-Type'] = format
+        response.header['Content-Type'] = format_to_mime(format)
     end
 
     # produce the body of the response
diff --git a/lib/puppet/network/http/rack/rest.rb b/lib/puppet/network/http/rack/rest.rb
index e98bffc1e..f2ba39336 100644
--- a/lib/puppet/network/http/rack/rest.rb
+++ b/lib/puppet/network/http/rack/rest.rb
@@ -14,7 +14,7 @@ class Puppet::Network::HTTP::RackREST < Puppet::Network::HTTP::RackHttpHandler
     end
 
     def set_content_type(response, format)
-        response[ContentType] = format
+        response[ContentType] = format_to_mime(format)
     end
 
     # produce the body of the response
@@ -28,6 +28,11 @@ class Puppet::Network::HTTP::RackREST < Puppet::Network::HTTP::RackHttpHandler
         request.env[HEADER_ACCEPT]
     end
 
+    # Retrieve the accept header from the http request.
+    def content_type_header(request)
+        request.env['HTTP_CONTENT_TYPE']
+    end
+
     # Return which HTTP verb was used in this request.
     def http_method(request)
         request.request_method
diff --git a/lib/puppet/network/http/webrick/rest.rb b/lib/puppet/network/http/webrick/rest.rb
index 5f77da87a..287fa39ac 100644
--- a/lib/puppet/network/http/webrick/rest.rb
+++ b/lib/puppet/network/http/webrick/rest.rb
@@ -27,6 +27,10 @@ class Puppet::Network::HTTP::WEBrickREST < WEBrick::HTTPServlet::AbstractServlet
         request["accept"]
     end
 
+    def content_type_header(request)
+        request["content-type"]
+    end
+
     def http_method(request)
         request.request_method
     end
@@ -41,7 +45,7 @@ class Puppet::Network::HTTP::WEBrickREST < WEBrick::HTTPServlet::AbstractServlet
 
     # Set the specified format as the content type of the response.
     def set_content_type(response, format)
-        response["content-type"] = format
+        response["content-type"] = format_to_mime(format)
     end
 
     def set_response(response, result, status = 200)