[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, upstream, updated. puppet-0.24.5-rc3-1456-g2f0b1e5

[James Turnbull]

The following commit has been merged in the upstream branch:
commit a45c4354b9ed8deaeb3173a495f06602472faebe
Author: Markus Roberts <Markus at reality.com>
Date:   Tue Sep 8 17:28:21 2009 -0700

    Fix for #2600 (wrong number of arguments under older mongrel)
    
    This was an API compatibility problem with mongrel's HTTPResponse.start()
    method between Mongrel 1.0.x and 1.1.x (the number of parameters changed).
    The older version does not provide the option to set the response header
    message which was used (redundantly with the response body) to return the
    error message when the HTTP response was signaling an error.
    
    In order to suport the older version the call was wrapped with a fallback
    and the coresponding code in the other rest implementations was adjusted
    to always send the error message in the response body.  Then the rest
    terminus was adjusted to pull the message from the response body (if it
    is present) rather than from the header (which is only used as a fallback
    for dealing with older puppetmasters), and the tests were augmeted to
    verify this behaviour.
    
    Signed-off-by: Markus Roberts <Markus at reality.com>

diff --git a/lib/puppet/indirector/rest.rb b/lib/puppet/indirector/rest.rb
index 35847de..e1ee89f 100644
--- a/lib/puppet/indirector/rest.rb
+++ b/lib/puppet/indirector/rest.rb
@@ -51,7 +51,7 @@ class Puppet::Indirector::REST < Puppet::Indirector::Terminus
             end
         else
             # Raise the http error if we didn't get a 'success' of some kind.
-            message = "Error %s on SERVER: %s" % [response.code, response.message]
+            message = "Error %s on SERVER: %s" % [response.code, (response.body||'').empty? ? response.message : response.body]
             raise Net::HTTPError.new(message, response)
         end
     end
diff --git a/lib/puppet/network/http/mongrel/rest.rb b/lib/puppet/network/http/mongrel/rest.rb
index 8a4de1c..7b28d88 100644
--- a/lib/puppet/network/http/mongrel/rest.rb
+++ b/lib/puppet/network/http/mongrel/rest.rb
@@ -50,16 +50,17 @@ class Puppet::Network::HTTP::MongrelREST < Mongrel::HttpHandler
 
     # produce the body of the response
     def set_response(response, result, status = 200)
-        args = [status]
-
         # Set the 'reason' (or 'message', as it's called in Webrick), when
-        # we have a failure.
-        if status >= 300
-            args << false << result
-        end
-
-        response.start(*args) do |head, body|
-            body.write(result)
+        # we have a failure, unless we're on a version of mongrel that doesn't
+        # support this.
+        if status < 300
+            response.start(status) { |head, body| body.write(result) }
+        else
+            begin
+                response.start(status,false,result) { |head, body| body.write(result) }
+            rescue ArgumentError
+	        response.start(status)              { |head, body| body.write(result) }
+            end
         end
     end
 
diff --git a/lib/puppet/network/http/webrick/rest.rb b/lib/puppet/network/http/webrick/rest.rb
index 287fa39..274665d 100644
--- a/lib/puppet/network/http/webrick/rest.rb
+++ b/lib/puppet/network/http/webrick/rest.rb
@@ -50,11 +50,8 @@ class Puppet::Network::HTTP::WEBrickREST < WEBrick::HTTPServlet::AbstractServlet
 
     def set_response(response, result, status = 200)
         response.status = status
-        if status >= 200 and status < 300
-            response.body = result
-        else
-            response.reason_phrase = result
-        end
+        response.body          = result if status >= 200 and status != 304
+        response.reason_phrase = result if status < 200 or status >= 300
     end
 
     # Retrieve node/cert/ip information from the request object.
diff --git a/spec/unit/indirector/rest.rb b/spec/unit/indirector/rest.rb
index a352f2a..d98e6f5 100755
--- a/spec/unit/indirector/rest.rb
+++ b/spec/unit/indirector/rest.rb
@@ -90,15 +90,37 @@ describe Puppet::Indirector::REST do
             @searcher.deserialize(response).should be_nil
         end
 
-        it "should fail if the response code is not in the 200s" do
-            @model.expects(:convert_from).never
-
-            response = mock 'response'
-            response.stubs(:code).returns "300"
-            response.stubs(:message).returns "There was a problem"
-
-            lambda { @searcher.deserialize(response) }.should raise_error(Net::HTTPError)
-        end
+        [300,400,403,405,500,501,502,503,504].each { |rc|
+            describe "when the response code is #{rc}" do
+                before :each do
+                    @model.expects(:convert_from).never
+
+                    @response = mock 'response'
+                    @response.stubs(:code).returns rc.to_s
+                    @response.stubs(:message).returns "There was a problem (header)"
+                end
+
+                it "should fail" do
+                    @response.stubs(:body).returns nil
+                    lambda { @searcher.deserialize(@response) }.should raise_error(Net::HTTPError)
+                end
+
+                it "should take the error message from the body, if present" do
+                    @response.stubs(:body).returns "There was a problem (body)"
+                    lambda { @searcher.deserialize(@response) }.should raise_error(Net::HTTPError,"Error #{rc} on SERVER: There was a problem (body)")
+                end
+
+                it "should take the error message from the response header if the body is empty" do
+                    @response.stubs(:body).returns ""
+                    lambda { @searcher.deserialize(@response) }.should raise_error(Net::HTTPError,"Error #{rc} on SERVER: There was a problem (header)")
+                end
+
+                it "should take the error message from the response header if the body is absent" do
+                    @response.stubs(:body).returns nil
+                    lambda { @searcher.deserialize(@response) }.should raise_error(Net::HTTPError,"Error #{rc} on SERVER: There was a problem (header)")
+                end
+            end
+        }    
 
         it "should return the results of converting from the format specified by the content-type header if the response code is in the 200s" do
             @model.expects(:convert_from).with("myformat", "mydata").returns "myobject"

-- 
Puppet packaging for Debian