alfonsodg / demo-web2py

Apache License 2.0
0 stars 0 forks source link

Soap Fault does not return 500 HTTP status code (violates W3C soap spec) #151

Closed alfonsodg closed 10 years ago

alfonsodg commented 10 years ago

From dragonfy...@gmail.com on January 10, 2011 12:27:33

What steps will reproduce the problem? 1. Create a soap service in web2py, using pysimplesoap (decorator)

  1. Pass this service invalid information/cause a soap fault
  2. Check HTTP response code What is the expected output? What do you see instead? HTTP response code should be 500, but is 200 (OK) Per soap spec here, Soap faults over HTTP should return a 500 response code with the fault, due to the underlying HTTP protocol. http://www.w3.org/TR/2007/REC-soap12-part0-20070427/#L26866 Section 4.1.2 just above Example 11. What version of the product are you using? On what operating system? 1.91.4 (2010-12-22 17:31:23) Ubuntu 9.10, kernel version 2.6.31-14 Python 2.6.4 ( r264 :75706, Nov 2 2009, 14:38:03) [GCC 4.4.1] on linux2 Please provide any additional information below. I've attached two patch files to rectify the situation. I'm using them right now, and they work great. The most efficient method of determining a fault is to have it returned from the pysimplesoap framework, so there are modifications to pysimplesoap in this patch, the "server.diff" file.

With full knowledge that running patches is not rocket science, from web2py root: patch -p1 < tools.diff patch -p1 < server.diff

Looking at the patch files, it modifies gluon/contrib/pysimplesoap/server.py: Add an option (return_status=False) defaulting to false for previous functionality to maintain backwards compatibility in pysimplesoap, that on true will return a tuple instead of a string. This tuple will contain (fault, response), with fault being either true if there was a fault, or false if there was not. Response is exactly the same as previously. gluon/tools.py: Pass return_status=True to pysimplesoap, and if return of fault is true, set response.status = 500.

There were 3 directions to go on the modification, all three of them have drawbacks. 1) I could look anywhere in the resulting string from pysimplesoap for a soap fault tag, which would be brittle since it's a string search, and would require some overhead above a simple bool compare. This is brittle mainly because it will catch if that tag is contained within (for example) a CDATA tag as text returned to the service. 2) I could invoke an XML parser, and look at a particular tag location for a soap fault. This also is somewhat brittle, though trending on the side of false negatives rather than false positives, higher overhead than previous, but should not return false positives. Due to the overhead, this isn't a great option, especially considering it's more of a hack than a solution. 3) Modify pysimplesoap to return explicitly if there is a soap fault or not. This is already captured in pysimplesoap, but not handed back. Since setting response.status within pysimplesoap isn't an option for a multitude of reasons, passing it back to web2py to set appropriately was the most efficient option. This does, however, require that pysimplesoap and the web2py gluon code be modified and pushed together.

I'm opening a bug (will commment with bug ID) in pysimplesoap to get upstream up to date with this patch as well.

Attachment: tools.diff server.diff

Original issue: http://code.google.com/p/web2py/issues/detail?id=153

alfonsodg commented 10 years ago

From dragonfy...@gmail.com on January 10, 2011 09:31:21

pysimplesoap (upstream) defect: http://code.google.com/p/pysimplesoap/issues/detail?id=21

alfonsodg commented 10 years ago

From dragonfy...@gmail.com on January 10, 2011 09:49:14

Couple more areas of the spec that detail the response code requirement. Table 20 in this link: http://www.w3.org/TR/soap12-part2/#http-respbindprocess Table 17 and below in this link: http://www.w3.org/TR/soap12-part2/#http-reqbindwaitstate This, (in multiple places) states that it's a requirement to send back various status codes on soap faults. This patch only fixes the 500 status code (which is by far the most common for a soap fault) but I have not verified that auth returns an appropriate 401 (I would assume it does). I also have not validated (though I assume these are also defects) that the other codes detailed in Table 20 are not handled appropriately, as with the 500 response code.

Also, env:Sender is not appropriately handled, as this should return a 400 code, not a 500. This may be handled by other areas of the framework however.

alfonsodg commented 10 years ago

From dragonfy...@gmail.com on January 10, 2011 09:56:49

Just tried it. 405, and the 3xxs are handled appropriately (outside soap framework, simply by web2py). 202 can never be reached in the current code, as there is always a response (as far as I could replicate).

400 and 415 error I'm not sure how to reproduce, so if anyone can check that out it would be wonderful. Not a holdup for implementing the patch, since if they are broken, they are broken before and after the patch is applied.

alfonsodg commented 10 years ago

From reingart@gmail.com on January 12, 2011 00:52:44

dragonfyre13: thanks for your patch. It looks nice but I'll recommend to avoid adding a new parameter return_status=True to the soap dispatch function. You can just return a tuple, backward compatibility is not necessary here, this is not a web2py defect and the dispatcher is an internal function not designed to be called from the user.

About 400 HTTP error, I think it might happen with a bad XML received, you can reproduce it with: wget " http://www.sistemasagiles.com.ar/simulador/wsfev1/call/soap?op=FEDummy " --post-data="<asfsd>" which now returns a SoapFault (VersionMismatch.ExpatError - no element found: line 1, column 7) but an incorrect 200 HTTP status.

415 should be reported when no "Content-Type: text/xml" header is received.

It will be very helpful if you can post a modified patch, thanks in advance for your collaboration.

PS: please post new tools.diff here and server.diff at pysimplesoap project

alfonsodg commented 10 years ago

From dragonfy...@gmail.com on January 18, 2011 20:07:18

FYI - Finally getting around to this, took me a few days to find time. I'm posting some stuff at the associated bug, but wanted anyone who cares about the issue here to know it hasn't been forgotten.

alfonsodg commented 10 years ago

From massimo....@gmail.com on March 01, 2011 11:48:41

Can you please email me your patch?

alfonsodg commented 10 years ago

From massimo....@gmail.com on December 05, 2012 07:58:28

Owner: reingart@gmail.com

alfonsodg commented 10 years ago

From massimo....@gmail.com on September 01, 2013 11:53:27

We upgraded pysimplesoap recently. This issue may be gone. Please confirm one way or another or it will be closed.

alfonsodg commented 10 years ago

From reingart@gmail.com on January 19, 2014 14:05:29

fixed in PR 355 (web2py) and pysimplesoap 1.11

I didn't used your patch completely as submited, but instead I've implemented a more backward compatible way.

Also, I've updated the contrib and added an example app and test.

Thanks!

Status: Fixed