Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Matching http.body[0,1000] does not work for quite large files #147

Closed
TafThorne opened this issue Jan 27, 2011 · 12 comments
Closed

Matching http.body[0,1000] does not work for quite large files #147

TafThorne opened this issue Jan 27, 2011 · 12 comments

Comments

@TafThorne
Copy link

We've run into a small problem with the v0.8.3 version of Savon. My service is reporting a SOAP fault, yet I'm not getting an exception raised by Savon.

I think my problem is due to this line in savon/soap/fault.rb:

  # Returns whether a SOAP fault is present.
  def present?
     @present ||= http.body[0,1000] =~ /<(.+:)?Body>(\s*)<(.+:)?Fault>/

In our case, the http.body is about 5k in size, with the Fault tag near the end, so I never get a match.

We have patched the file locally by replacing "http.body[0,1000]" with "http.body" so that all of the body is checked.

This issue/bug seems to still be present in v0.8.4

@rubiii
Copy link
Contributor

rubiii commented Jan 27, 2011

it was implemented because with large responses, parsing the entire body with a regepx is damn slow. still trying to find a good solution for this.

@TafThorne
Copy link
Author

I have closed this issue by accident haven't I... can someone reopen it please or shall I recreate it?

@rubiii
Copy link
Contributor

rubiii commented Jan 27, 2011

i've tested quite a few possible solutions and even though i'm more comfortable with using regexps, it's 22x slower than using some native string method. maybe something rather simple like include? could work:

soap1_fault = body.include?("faultcode>") && body.include?("faultstring>")
soap2_fault = body.include?("Code>") && body.include?("Reason>")
body.include?("Fault>") && (soap1_fault || soap2_fault)

i know it's ugly, but ... opinions?

@TafThorne
Copy link
Author

One of the guys here has made a suggestion that sounds quite good to me:
"The www.w3.org web site I was looking at is http://www.w3.org/TR/2000/NOTE-SOAP-20000508/#_Toc478383529 section 6.2
which says:

"In case of a SOAP error while processing the request, the SOAP HTTP server MUST issue an HTTP 500 "Internal Server Error" response and include a SOAP message in the response containing a SOAP Fault element (see section 4.4) indicating the SOAP processing error."

So my suggestion is that the code can perhaps search for the 500 Internal Server Error code in the HTTP response first."

This would mean that responses without a fault should be binned quite quickly based on the HTTP response and you would only have to perform the expensive and exhaustive search on files known to have a problem.

@rubiii
Copy link
Contributor

rubiii commented Jan 27, 2011

thanks guys!

from my experience, relying on soap services to follow the specifications is almost always impossible. soap implementations are a mess. but sending a specific response code looks like something most services should be able to do. i'll give it a shot.

ps. could you create a gist/pastie containing your soap fault? might be interesting for regression and performance tests.

@TafThorne
Copy link
Author

Did you want just the fault or the full response? We can probably do something about anonymising any information in the response.

@rubiii
Copy link
Contributor

rubiii commented Jan 27, 2011

the complete xml (if it's not too much work anonymising private information).

@rubiii
Copy link
Contributor

rubiii commented Jan 28, 2011

your problem should be fixed with v0.8.5.

@TafThorne
Copy link
Author

Thank You.

Here is an example of our output in a gist as requested.
We are not generating the status 500 code at present but are quite happy to patch savon internally until we generate it. We have been able to get v0.8.5 to work well in our code.

@TafThorne
Copy link
Author

I have just seen that i156 would add a configurable option, so using that until we update our server code should work well :-)

@rubiii
Copy link
Contributor

rubiii commented Feb 15, 2011

thank you guys for getting involved with this issue. i really appreciate your help! your problem should be fixed with v0.8.6. i did not add a configuration option for implementations that don't follow the soap spec, but i think this would be a pretty good idea if we find any other cases like this.

thanks again!

@jkingdon
Copy link
Contributor

The unconditional code, as found in 0.8.6, is fine with me. I'm not the one who had the 5 megabyte responses ;-).

calamitas pushed a commit to calamitas/savon that referenced this issue Jun 7, 2011
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
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants