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

Broken SOAP body #90

Open
miadabrin opened this issue Jan 6, 2018 · 24 comments
Open

Broken SOAP body #90

miadabrin opened this issue Jan 6, 2018 · 24 comments

Comments

@miadabrin
Copy link

miadabrin commented Jan 6, 2018

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

0e4e8dc

It has basically broken the whole functionality over old SOAP servers

I think it should be fixed somehow

@kernle32dll
Copy link
Contributor

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.

@miadabrin
Copy link
Author

@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

@kernle32dll
Copy link
Contributor

@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.

@miadabrin
Copy link
Author

@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
Copy link
Author

@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)

@kernle32dll
Copy link
Contributor

@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).

@miadabrin
Copy link
Author

miadabrin commented Jan 9, 2018

@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.

@fiorix
Copy link
Owner

fiorix commented Jan 9, 2018

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"`
}

@kernle32dll
Copy link
Contributor

@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.

@miadabrin
Copy link
Author

miadabrin commented Mar 4, 2018

@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

@fiorix
Copy link
Owner

fiorix commented Mar 14, 2018

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

@shirshir
Copy link

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

@kernle32dll
Copy link
Contributor

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

@r1se
Copy link

r1se commented Mar 29, 2018

I have same issue, SOAP-Env body dissapear.

@shdpl
Copy link

shdpl commented Apr 30, 2018

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.

@fiorix
Copy link
Owner

fiorix commented May 3, 2018

cc @adriantam

@fiorix fiorix mentioned this issue Aug 8, 2018
@Tiinusen
Copy link

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

@kernle32dll
Copy link
Contributor

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 #90 (comment), your best course of action is to put that in go:generate.

Now, onto what @miadabrin said in #90 (comment) : 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.

@Tiinusen
Copy link

Kernle32DLL do you have any new information for us?

@kernle32dll
Copy link
Contributor

@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).

@miadabrin
Copy link
Author

@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.

@Tiinusen
Copy link

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

@Tiinusen
Copy link

Tiinusen commented Nov 8, 2018

@kernle32dll bump (how's it going?)

@FlipB
Copy link

FlipB commented May 22, 2019

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants