wsdl2go icon indicating copy to clipboard operation
wsdl2go copied to clipboard

Broken SOAP body

Open miadabrin opened this issue 7 years ago • 24 comments

According to this commit you have removed the body from the soap messages:

0e4e8dcdce5ccec6c5a3e437026b1577e26a971d

It has basically broken the whole functionality over old SOAP servers

I think it should be fixed somehow

miadabrin avatar Jan 06 '18 09:01 miadabrin

Don't know to which part of the commit you are referring to in particular, but the body was not removed. That would (probably) break any kind of request and/or response.

kernle32dll avatar Jan 08 '18 09:01 kernle32dll

@Kernle32DLL like this one

req := &Envelope{
  		EnvelopeAttr: c.Envelope,
  		NSAttr:       c.Namespace,
 +		TNSAttr:      c.ThisNamespace,
  		XSIAttr:      XSINamespace,
  		Header:       c.Header,
 -		Body:         Body{Message: in},
 +		Body:         in,
  	}

and it did break our requests (they had no soap body inside the envelop). I had to fork your repo and bring it back to the time before this commit so it would work

miadabrin avatar Jan 08 '18 12:01 miadabrin

@miadabrin The body was moved, look e.g. here.

If this broke your requests, you are mixing an updated version of the client (this repo) with an outdated version of your generated bindings. Rebuild your bindings, and you should be fine.

kernle32dll avatar Jan 08 '18 13:01 kernle32dll

@Kernle32DLL I had tested that . here is the wsdl we are working with https://bpm.shaparak.ir/pgwchannel/services/pgw?wsdl I tested the whole request payload before sending and it had no envelope body. maybe you can tell what is wrong with it. To be Specific we were calling BpPayRequest

miadabrin avatar Jan 09 '18 06:01 miadabrin

@Kernle32DLL Please note that it was working before ( I just had added the namespace name before the elements inside the generated model code but other than that it was working fine)

miadabrin avatar Jan 09 '18 06:01 miadabrin

@miadabrin cannot verify. I did test BpPayRequest, and the send request looks like this:

<SOAP-ENV:Envelope xmlns:SOAP-ENV="http://schemas.xmlsoap.org/soap/envelope/" xmlns:ns="http://interfaces.core.sw.bps.com/" xmlns:tns="http://interfaces.core.sw.bps.com/" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"><SOAP-ENV:Body><parameters><terminalId>123</terminalId></parameters></SOAP-ENV:Body></SOAP-ENV:Envelope>

Like I said - it looks like you are mixing a newer version of wsdl2go (the client) and the generated bindings (by the cli tool). Use go get -u github.com/fiorix/wsdl2go to get the latest version, and then re-generate your bindings (you should go:generate them anyway, to prevent such problems of inconsistency).

kernle32dll avatar Jan 09 '18 13:01 kernle32dll

@Kernle32DLL I did go get -u github.com/fiorix/wsdl2go already and then regenerated the code. Maybe I have missed something. I will retry and put the results here.

miadabrin avatar Jan 09 '18 14:01 miadabrin

I just tested this and it does render the body correctly.

The one thing I noticed, however, is that scalar types in struct are generated as pointers. That is definitely wrong. Have you seen this too @Kernle32DLL ?

// BpPayRequest was auto-generated from WSDL.
type BpPayRequest struct {
        TerminalId     *int64  `xml:"terminalId,omitempty" json:"terminalId,omitempty" yaml:"terminalId,omitempty"`
        UserName       *string `xml:"userName,omitempty" json:"userName,omitempty" yaml:"userName,omitempty"`
        UserPassword   *string `xml:"userPassword,omitempty" json:"userPassword,omitempty" yaml:"userPassword,omitempty"`
        OrderId        *int64  `xml:"orderId,omitempty" json:"orderId,omitempty" yaml:"orderId,omitempty"`
        Amount         *int64  `xml:"amount,omitempty" json:"amount,omitempty" yaml:"amount,omitempty"`
        LocalDate      *string `xml:"localDate,omitempty" json:"localDate,omitempty" yaml:"localDate,omitempty"`
        LocalTime      *string `xml:"localTime,omitempty" json:"localTime,omitempty" yaml:"localTime,omitempty"`
        AdditionalData *string `xml:"additionalData,omitempty" json:"additionalData,omitempty" yaml:"additionalData,omitempty"`
        CallBackUrl    *string `xml:"callBackUrl,omitempty" json:"callBackUrl,omitempty" yaml:"callBackUrl,omitempty"`
        PayerId        *int64  `xml:"payerId,omitempty" json:"payerId,omitempty" yaml:"payerId,omitempty"`
}

fiorix avatar Jan 09 '18 16:01 fiorix

@fiorix I don't know about the pointers. But I suspect the reason is correct omitting. I think to remember that primitives with their zero value (e.g. "", 0 or false) are removed on omitempty (which is wrong - 0 could be a valid value for example). But don't quote me on that.

kernle32dll avatar Jan 14 '18 23:01 kernle32dll

@fiorix well I tested the new version after all the variations in the code. It still is buggy

It sends this:

<SOAP-ENV:Envelope
	xmlns:SOAP-ENV="http://schemas.xmlsoap.org/soap/envelope/"
	xmlns:ns="https://bpm.shaparak.ir/pgwchannel/services/pgw"
	xmlns:tns="https://bpm.shaparak.ir/pgwchannel/services/pgw"
	xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
	<SOAP-ENV:Body>
		<parameters>
			<terminalId>?</terminalId>
			<userName>?</userName>
			<userPassword>?</userPassword>
			<orderId>?</orderId>
			<amount>?</amount>
			<localDate>?</localDate>
			<localTime>?</localTime>
	                <callBackUrl>?</callBackUrl>
		</parameters>
	</SOAP-ENV:Body>
</SOAP-ENV:Envelope>

instead of this:

<soapenv:Envelope xmlns:soapenv="http://schemas.xmlsoap.org/soap/envelope/" xmlns:int="http://interfaces.core.sw.bps.com/">
   <soapenv:Header/>
   <soapenv:Body>
      <int:bpPayRequest>
         <terminalId>?</terminalId>
         <userName>?</userName>
         <userPassword>?</userPassword>
         <orderId>?</orderId>
         <amount>?</amount>
         <localDate>?</localDate>
         <localTime>?</localTime>
         <additionalData>?</additionalData>
         <callBackUrl>?</callBackUrl>
         <payerId>?</payerId>
      </int:bpPayRequest>
   </soapenv:Body>
</soapenv:Envelope>

which as you can see there is a parameters key in there which is obviously wrong

miadabrin avatar Mar 04 '18 14:03 miadabrin

Hey being realistic here, I have little to no time to troubleshoot this but I'd be happy to review any PRs.

fiorix avatar Mar 14 '18 11:03 fiorix

I don't know about the pointers. But I suspect the reason is correct omitting. I think to remember that primitives with their zero value (e.g. "", 0 or false) are removed on omitempty (which is wrong - 0 could be a valid value for example). But don't quote me on that.

@Kernle32DLL Yes, you are right: https://stackoverflow.com/a/38487668

shirshir avatar Mar 26 '18 17:03 shirshir

@miadabrin True, I can verify that this is incorrect. I will investigate.

kernle32dll avatar Mar 27 '18 09:03 kernle32dll

I have same issue, SOAP-Env body dissapear.

r1se avatar Mar 29 '18 11:03 r1se

I believe i also got similar issue. I minimized to all relevant sections of code. So SOAP-ENV:Body seems to be overridden by generated type. Something like this fixes the problem.

shdpl avatar Apr 30 '18 12:04 shdpl

cc @adriantam

fiorix avatar May 03 '18 09:05 fiorix

Is there any status on this? pull requests? this issue halts development from integrating this library.

Tiinusen avatar Aug 24 '18 07:08 Tiinusen

First of all - sorry to keep everyone waiting. Its really bad that this issue and ensuing bugs are open for nearly a year now. Especially with me causing the problems in the first place.

I had another look into this, and cannot verify the body disappearing. As for that @shdpl wrote: This just establishes the old behavior, and in turn breaks RPC again. The playground is also wrong, as the presented input to "doRoundTrip" is not what the (current) encoder is generating. This is important! The key is this: The client (this is what sends the requests) was adjusted, to not include the body anymore. The encoder (this is what builds the binding files) was also adjusted to compensate for this. Or put differently: If you use a newer client with older bindings, you end up with no body, as neither client nor encoder add them. You can look at this playground, to see what the request actually looks like.

Key takeaway: Make absolutely sure, that you only use bindings with the version of this lib you created them with. Best course of action - use something like dep to fix the version. If you like to live dangerously, at least ensure that you regenerate your bindings before building. Like mentioned in https://github.com/fiorix/wsdl2go/issues/90#issuecomment-356278431, your best course of action is to put that in go:generate.

Now, onto what @miadabrin said in https://github.com/fiorix/wsdl2go/issues/90#issuecomment-370232935 : Indeed, wsdl2go builds a wrong request here (<parameters> instead of <int:bpPayRequest>). But why?

The encoder generates code in two ways: "regular" and "rpc" style. You can look here, on how the request must look for either. In short, RPC wraps the request parameters, with the name of the request.

In miadabrins case, the encoder builds "regular" code. But judging from what miadabrin said, and verifying it via SoapUI, it should be RPC-ish. So what is going on? If you looked at the url linked above, there is a very subtle configuration variation, called "Document/literal wrapped". In short, this is not RPC, but the request looks like RPC. And guess what - miadabrins wsdl is exactly this case.

Now, the problem is that I currently have no idea how to detect this "wrapped" style. Also, even if I could, the current RPC-style is not sufficient. If the wsdl would be detected as RPC, it would create the following request (note the <parameters>):

<SOAP-ENV:Envelope
	xmlns:SOAP-ENV="http://schemas.xmlsoap.org/soap/envelope/"
	xmlns:ns="https://bpm.shaparak.ir/pgwchannel/services/pgw"
	xmlns:tns="https://bpm.shaparak.ir/pgwchannel/services/pgw"
	xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
	<SOAP-ENV:Body>
		<ns1:bpPayRequest>
			<parameters>
				<terminalId>?</terminalId>
				<userName>?</userName>
				<userPassword>?</userPassword>
				<orderId>?</orderId>
				<amount>?</amount>
				<localDate>?</localDate>
				<localTime>?</localTime>
				<callBackUrl>?</callBackUrl>
			</parameters>
		</<ns1:bpDynamicPayRequest>
	</SOAP-ENV:Body>
</SOAP-ENV:Envelope>

So, next steps. I have to do some homework in regards to this "Document/literal wrapped" style. Now that I identified the root issue, this should be easy to fix. I will keep you guys posted.

kernle32dll avatar Aug 25 '18 16:08 kernle32dll

Kernle32DLL do you have any new information for us?

Tiinusen avatar Sep 10 '18 07:09 Tiinusen

@Tiinusen Yes. I think I identified how to correct this. I will attempt a fix this week, and will see how this goes. However, I currently lack a proper WSDL to test, as the WSDL linked by @miadabrin does not seem to be valid (anymore? - it returns an incomplete WSDL without the actual parameters).

kernle32dll avatar Sep 12 '18 16:09 kernle32dll

@Kernle32DLL if you look at the contents of the wsdl it imports another one at address https://bpm.shaparak.ir/pgwchannel/services/pgw?wsdl=IPaymentGateway.wsdl . This should be a good one to start with.

miadabrin avatar Sep 12 '18 16:09 miadabrin

@Kernle32DLL Did you succeed with your fix? Haven't seen anything in the commit log

Tiinusen avatar Sep 21 '18 11:09 Tiinusen

@Kernle32DLL bump (how's it going?)

Tiinusen avatar Nov 08 '18 12:11 Tiinusen

I ran into this issue.

Are there any in-progress PRs or branches i can try or are there any other SOAP implementations for Go I should consider?

FlipB avatar May 22 '19 11:05 FlipB