Skip to content

Commit

Permalink
changed the soap fault detection
Browse files Browse the repository at this point in the history
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 savonrb#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
  • Loading branch information
rubiii committed Jan 28, 2011
1 parent 98655c0 commit 2526703
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 4 deletions.
13 changes: 12 additions & 1 deletion lib/savon/soap/fault.rb
Expand Up @@ -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.
Expand All @@ -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]}"
Expand Down
7 changes: 6 additions & 1 deletion spec/savon/soap/fault_spec.rb
Expand Up @@ -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|
Expand Down Expand Up @@ -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]
Expand Down
4 changes: 2 additions & 2 deletions spec/savon/soap/response_spec.rb
Expand Up @@ -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
Expand Down

0 comments on commit 2526703

Please sign in to comment.