changed the soap fault detection

probably the third change in the last few versions. using a regexp was too slow for large responses and checking only the first 1000 characters did not work with soap faults near the end of large response bodies. see issue #147. here's how the new code works: * check the response code for 500 (internal server error), which (according to the specs) must be used for soap faults * then use String#include? instead of a regexp to check for mandatory soap 1.1 and 1.2 fault tags
savonrb · Jan 28, 2011 · 2526703 · 2526703
1 parent 98655c0
commit 2526703
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 4 deletions.
diff --git a/lib/savon/soap/fault.rb b/lib/savon/soap/fault.rb
@@ -19,7 +19,7 @@ def initialize(http)
 
       # Returns whether a SOAP fault is present.
       def present?
-        @present ||= http.body[0,1000] =~ /<(.+:)?Body>(\s*)<(.+:)?Fault>/
+        @present ||= http.code == 500 && http.body.include?("Fault>") && (soap1_fault? || soap2_fault?)
       end
 
       # Returns the SOAP fault message.
@@ -35,6 +35,17 @@ def to_hash
 
     private
 
+      # Returns whether the response contains a SOAP 1.1 fault.
+      def soap1_fault?
+        http.body.include?("faultcode>") && http.body.include?("faultstring>")
+      end
+
+      # Returns whether the response contains a SOAP 1.2 fault.
+      def soap2_fault?
+        http.body.include?("Code>") && http.body.include?("Reason>")
+      end
+
+      # Returns the SOAP fault message by version.
       def message_by_version(fault)
         if fault[:faultcode]
           "(#{fault[:faultcode]}) #{fault[:faultstring]}"

diff --git a/spec/savon/soap/fault_spec.rb b/spec/savon/soap/fault_spec.rb
@@ -32,6 +32,11 @@
     it "should return false unless the HTTP response contains a SOAP fault" do
       no_fault.should_not be_present
     end
+
+    it "should return false if the HTTP response code is not 500" do
+      fault = Savon::SOAP::Fault.new new_response(:code => 200, :body => Fixture.response(:soap_fault))
+      fault.should_not be_present
+    end
   end
 
   [:message, :to_s].each do |method|
@@ -80,7 +85,7 @@
   end
 
   def new_response(options = {})
-    defaults = { :code => 200, :headers => {}, :body => Fixture.response(:authentication) }
+    defaults = { :code => 500, :headers => {}, :body => Fixture.response(:authentication) }
     response = defaults.merge options
 
     HTTPI::Response.new response[:code], response[:headers], response[:body]

diff --git a/spec/savon/soap/response_spec.rb b/spec/savon/soap/response_spec.rb
@@ -159,12 +159,12 @@
   def soap_response(options = {})
     defaults = { :code => 200, :headers => {}, :body => Fixture.response(:authentication) }
     response = defaults.merge options
-    
+
     Savon::SOAP::Response.new HTTPI::Response.new(response[:code], response[:headers], response[:body])
   end
 
   def soap_fault_response
-    soap_response :body => Fixture.response(:soap_fault)
+    soap_response :code => 500, :body => Fixture.response(:soap_fault)
   end
 
   def http_error_response