savonrb / wasabi

A simple WSDL parser
MIT License
90 stars 84 forks source link

Wasabi::Parser#input_output_for a68840b changes fallback operation_name => port_message_type #32

Closed tiredpixel closed 10 years ago

tiredpixel commented 10 years ago

Forgive me; I do not know if this is a bug in wasabi or in the types of WSDL I (and seemingly others) are using—but it does appear to be the cause of savonrb/savon#482.

In a68840b88cb3166bb87ad2bf832cdd280092b965, Wasabi::Parser#input_output_for was introduced, replacing Wasabi::Parser#input_for. Previously, if there was no message_type, [port_message_ns_id, operation_name] was returned. This commit changed the return in such a case to [port_message_ns_id, port_message_type], however. This did not result in any tests breaking.

I have been attempting to use a specific service's WSDL for authentication. I'm afraid I am not at liberty to post the complete WSDL, so I have obscured it and posted a much-reduced version (thus, it is possible I have removed something I should not have). It follows:

<wsdl:definitions xmlns:apachesoap="http://xml.apache.org/xml-soap" xmlns:impl="http://www.example.com/api/v1" xmlns:intf="http://www.example.com/api/v1" xmlns:soapenc="http://schemas.xmlsoap.org/soap/encoding/" xmlns:wsdl="http://schemas.xmlsoap.org/wsdl/" xmlns:wsdlsoap="http://schemas.xmlsoap.org/wsdl/soap/" xmlns:xsd="http://www.w3.org/2001/XMLSchema" targetNamespace="http://www.example.com/api/v1">
<wsdl:types>
<schema xmlns="http://www.w3.org/2001/XMLSchema" targetNamespace="http://www.example.com/api/v1">
<import namespace="http://schemas.xmlsoap.org/soap/encoding/"/>
</complexType>
</schema>
</wsdl:types>
<wsdl:message name="authenticateResponse">
<wsdl:part name="authenticateReturn" type="impl:UserProfile"></wsdl:part>
</wsdl:message>
<wsdl:message name="authenticateRequest">
<wsdl:part name="username" type="soapenc:string"></wsdl:part>
<wsdl:part name="password" type="soapenc:string"></wsdl:part>
</wsdl:message>
<wsdl:portType name="LoginRemote">
<wsdl:operation name="authenticate" parameterOrder="username password">
<wsdl:input message="impl:authenticateRequest" name="authenticateRequest"></wsdl:input>
<wsdl:output message="impl:authenticateResponse" name="authenticateResponse"></wsdl:output>
<wsdl:fault message="impl:ApiException" name="ApiException"></wsdl:fault>
</wsdl:operation>
</wsdl:portType>
<wsdl:binding name="loginSoapBinding" type="impl:LoginRemote">
<wsdlsoap:binding style="rpc" transport="http://schemas.xmlsoap.org/soap/http"/>
</wsdl:binding>
<wsdl:service name="LoginRemoteService">
<wsdl:port binding="impl:loginSoapBinding" name="login">
<wsdlsoap:address location="https://api.example.com/v1/api/api/login"/>
</wsdl:port>
</wsdl:service>
</wsdl:definitions>

Previously, the authenticate operation sent <authenticate>; with the referenced commit, it sends <authenticateRequest>. This returns an error from the service.

I have been consulting the specification, to try to check what is the correct behaviour. I think it might be related to §2.4.5; however, I'm afraid not familiar enough with this to be able to decide. I do note, however, that the input name is specified, which would lead me to expect the current (post-referenced-commit) behaviour—indeed, even without this attribute being specified, the Solicit-Response Operation could result in the same (§2.4.3).

Might someone familiar with the specification be able to clarify this behaviour for me, and decide whether this is in fact a bug?

Peace, tiredpixel

tjarratt commented 10 years ago

Confirming that this is a bug that is affecting the current release of Savon. My current opinion is that the change in a68840b should be reverted, unless I find a simple fix for this.

Thanks so much for taking the time to write this up, @tiredpixel, it's been very helpful. My only regret is that this hasn't been addressed for ~2 months.

tjarratt commented 10 years ago

Hey @tiredpixel, thank you again for writing this up. I'm going to spend some time consulting the specification you so kindly linked so I can glean what the correct behavior is. From what I can tell on the discussions on Savon, we need something in between (eg: usually use the operation name, but occasionally use the port message type).

In the mean time, can you confirm that this is fixed in Savon 2.3.2?

tiredpixel commented 10 years ago

@tjarratt, you are most welcome; I'm delighted to hear my investigation was of use. I wish I could have made a decision about the correct behaviour when I was looking into this, but I'm afraid I'm not familiar enough with the specification to make that call. Thank you for looking into this further; if I can be of further use, then please let me know.

I can confirm that it works for me in Savon v2.3.2—however, I think it would be best if someone else who has been experiencing this issue could confirm, too, as when I tried just now v2.3.0 also worked (which it certainly didn't previously). That makes me suspect that the service I was experiencing the issues with a couple of months ago has updated the WSDL, so it no longer a valid check.

tiredpixel commented 10 years ago

@tjarratt, my mistake; I was being daft. It is not that the provider has changed their WSDL; I was locking to the wrong version of Wasabi when retesting 2.3.0 (I wasn't being explicit so was getting a Wasabi more up-to-date than was previously available). Here follows a small matrix of my tests:

wasabi  savon  2.2.0  2.3.0  2.3.1  2.3.2
3.1.0          WORKS  --     --     --
3.2.0          --     FAILS  FAILS  --
3.2.1          --     FAILS  FAILS  --
3.2.2          --     WORKS  WORKS  WORKS

where -- means I couldn't lock that combination.

So yes, it appears to work on Savon 2.3.2 (with Wasabi 3.2.2).

tjarratt commented 10 years ago

@tiredpixel -- thank you so much for confirming this (and for making a matrix of versions with pass / fail).

My hope is that now that this issue is resolved, we can move on to fixing the other regressions in Savon 2.3.x and make it even better!