From 601a2e54aecccedfc0bdd94939d44cc07b4d6588 Mon Sep 17 00:00:00 2001 From: Brice Figureau Date: Mon, 10 Aug 2009 23:08:21 +0200 Subject: Fix #2516 - Fix format detection when content-type contains charset Even though Puppet never transmist a charset information in its response/request content-type, some proxy (especially Apache with the infamous AddDefaultCharset configuration) may add this "incorrect" information. This patch makes sure that only the mime-type is used when looking for the format associated with a response or a request. The patch also provides a better error message when the client or server code is fed with a request whose mime-type can not be mapped to a known format. It also fixes a typo noticed by the original reporter. Signed-off-by: Brice Figureau --- lib/puppet/indirector/rest.rb | 6 ++++-- lib/puppet/network/format_handler.rb | 3 ++- lib/puppet/network/http/handler.rb | 2 ++ spec/unit/indirector/rest.rb | 11 +++++++++++ spec/unit/network/format_handler.rb | 5 +++++ spec/unit/network/http/handler.rb | 10 ++++++++++ 6 files changed, 34 insertions(+), 3 deletions(-) diff --git a/lib/puppet/indirector/rest.rb b/lib/puppet/indirector/rest.rb index 4d818c986..35847de88 100644 --- a/lib/puppet/indirector/rest.rb +++ b/lib/puppet/indirector/rest.rb @@ -41,11 +41,13 @@ class Puppet::Indirector::REST < Puppet::Indirector::Terminus raise "No content type in http response; cannot parse" end + content_type = response['content-type'].gsub(/\s*;.*$/,'') # strip any appended charset + # Convert the response to a deserialized object. if multiple - model.convert_from_multiple(response['content-type'], response.body) + model.convert_from_multiple(content_type, response.body) else - model.convert_from(response['content-type'], response.body) + model.convert_from(content_type, response.body) end else # Raise the http error if we didn't get a 'success' of some kind. diff --git a/lib/puppet/network/format_handler.rb b/lib/puppet/network/format_handler.rb index 17b863fda..2ffbcef3d 100644 --- a/lib/puppet/network/format_handler.rb +++ b/lib/puppet/network/format_handler.rb @@ -81,9 +81,10 @@ module Puppet::Network::FormatHandler out = format when %r{\w+/\w+} out = mime(format) - when + else out = format(format) end + raise ArgumentError, "No format match the given format name or mime-type (%s)" % format if out.nil? out.name end diff --git a/lib/puppet/network/http/handler.rb b/lib/puppet/network/http/handler.rb index 27e8dbd1d..817661db1 100644 --- a/lib/puppet/network/http/handler.rb +++ b/lib/puppet/network/http/handler.rb @@ -42,7 +42,9 @@ module Puppet::Network::HTTP::Handler def request_format(request) if header = content_type_header(request) + header.gsub!(/\s*;.*$/,'') # strip any charset format = Puppet::Network::FormatHandler.mime(header) + raise "Client sent a mime-type (%s) that doesn't correspond to a format we support" % header if format.nil? return format.name.to_s if format.suitable? end diff --git a/spec/unit/indirector/rest.rb b/spec/unit/indirector/rest.rb index 696af9ae7..a352f2a2a 100755 --- a/spec/unit/indirector/rest.rb +++ b/spec/unit/indirector/rest.rb @@ -121,6 +121,17 @@ describe Puppet::Indirector::REST do @searcher.deserialize(response, true).should == "myobjects" end + + it "should strip the content-type header to keep only the mime-type" do + @model.expects(:convert_from).with("text/plain", "mydata").returns "myobject" + + response = mock 'response' + response.stubs(:[]).with("content-type").returns "text/plain; charset=utf-8" + response.stubs(:body).returns "mydata" + response.stubs(:code).returns "200" + + @searcher.deserialize(response) + end end describe "when creating an HTTP client" do diff --git a/spec/unit/network/format_handler.rb b/spec/unit/network/format_handler.rb index 604ae1be3..8a79a58b3 100755 --- a/spec/unit/network/format_handler.rb +++ b/spec/unit/network/format_handler.rb @@ -87,6 +87,11 @@ describe Puppet::Network::FormatHandler do lambda { Puppet::Network::FormatHandler.protected_format(:myformat).render("foo") }.should raise_error(Puppet::Network::FormatHandler::FormatError) end + it "should raise an error if we couldn't find a format by name or mime-type" do + Puppet::Network::FormatHandler.stubs(:format).with(:myformat).returns nil + lambda { Puppet::Network::FormatHandler.protected_format(:myformat) }.should raise_error + end + describe "when using formats" do before do @format = mock 'format' diff --git a/spec/unit/network/http/handler.rb b/spec/unit/network/http/handler.rb index a0c817361..1addb5890 100755 --- a/spec/unit/network/http/handler.rb +++ b/spec/unit/network/http/handler.rb @@ -129,6 +129,16 @@ describe Puppet::Network::HTTP::Handler do @handler.do_exception(@response, "A test", 404) end + it "should raise an error if the request is formatted in an unknown format" do + @handler.stubs(:content_type_header).returns "unknown format" + lambda { @handler.request_format(@request) }.should raise_error + end + + it "should still find the correct format if content type contains charset information" do + @handler.stubs(:content_type_header).returns "text/plain; charset=UTF-8" + @handler.request_format(@request).should == "s" + end + describe "when finding a model instance" do before do @irequest = stub 'indirection_request', :method => :find, :indirection_name => "my_handler", :to_hash => {}, :key => "my_result", :model => @model_class -- cgit