Graceful handling of invalid XML/invalid SOAP response

Instead of having a nil pointer exception thrown when the response doesn't contain an <envelope> tag as assumed, throw an InvalidResponseError, allowing client applications to better handle unexpectedly bad responses from some SOAP server. Liauw Fendy <[email protected]> Sam Gibson <[email protected]>
savonrb · Oct 12, 2011 · 964ed9f · 964ed9f
1 parent bf830f6
commit 964ed9f
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 0 deletions.
diff --git a/lib/savon/soap/invalid_response_error.rb b/lib/savon/soap/invalid_response_error.rb
@@ -0,0 +1,11 @@
+require "savon/error"
+
+module Savon
+  module SOAP
+    # = Savon::SOAP::InvalidResponseError
+    #
+    # Represents an error when the response was not a valid SOAP envelope.
+    class InvalidResponseError < Error
+    end
+  end
+end
diff --git a/lib/savon/soap/response.rb b/lib/savon/soap/response.rb
@@ -1,5 +1,6 @@
 require "savon/soap/xml"
 require "savon/soap/fault"
+require "savon/soap/invalid_response_error"
 require "savon/http/error"
 
 module Savon
@@ -50,11 +51,17 @@ def [](key)
 
       # Returns the SOAP response header as a Hash.
       def header
+        if !hash.has_key? :envelope
+          raise Savon::SOAP::InvalidResponseError, "Unable to parse response body '#{to_xml}'"
+        end
         hash[:envelope][:header]
       end
 
       # Returns the SOAP response body as a Hash.
       def body
+        if !hash.has_key? :envelope
+          raise Savon::SOAP::InvalidResponseError, "Unable to parse response body '#{to_xml}'"
+        end
         hash[:envelope][:body]
       end
 

diff --git a/spec/savon/soap/response_spec.rb b/spec/savon/soap/response_spec.rb
@@ -121,13 +121,21 @@
       soap_response[:authenticate_response][:return].should ==
         Fixture.response_hash(:authentication)[:authenticate_response][:return]
     end
+
+    it "should throw an exception when the response body isn't parsable" do
+      lambda { invalid_soap_response.body }.should raise_error Savon::SOAP::InvalidResponseError
+    end
   end
 
   describe "#header" do
     it "should return the SOAP response header as a Hash" do
       response = soap_response :body => Fixture.response(:header)
       response.header.should include(:session_number => "ABCD1234")
     end
+
+    it "should throw an exception when the response header isn't parsable" do
+      lambda { invalid_soap_response.header }.should raise_error Savon::SOAP::InvalidResponseError
+    end
   end
 
   %w(body to_hash).each do |method|
@@ -221,4 +229,11 @@ def http_error_response
     soap_response :code => 404, :body => "Not found"
   end
 
+  def invalid_soap_response(options={})
+    defaults = { :code => 200, :headers => {}, :body => "I'm not SOAP" } 
+    response = defaults.merge options
+
+    Savon::SOAP::Response.new HTTPI::Response.new(response[:code], response[:headers], response[:body])
+  end
+
 end