Possible bug in PayPal Express

[villagedefrance]

It has been reported in the Forum that an issue could exist in PayPal Express payment.
Reference: http://forum.villagedefrance.net/showthread.php?tid=14&pid=98#pid98

DavidB had no issues with it in v1.8.4 but PP Express has been updated since.
I will investigate but I am unable to test it.

@gob33 Do you have a working version? Any chance you could have a look at this for us?

[gob33]

I saw DavidB post.
I made some changes in the admin part, not the catalog part so i doubt i introduced a bug.
Reinstalling or changing API does nothing, better to see the log if transaction succeeded but not the return in shop.
Suspect the if() tests for redirection in catalog controller to be bad.
Unable to test in real even in sand box since they passed to TLS1.2
All Paypal payments in OC run approximatively, when there is problem noboby know where to look at.

[villagedefrance]

I possibly changed something after your last PR, something to do with "Token_id" if I remember correctly, but it has been a while ...

Would it be possible for you to create a new PR with your last known good version?
If I have made changes then they will be obvious to see and I will then remove them.

FYI, I have also published a patch with the old PP Express v1.8.4 version, but I don't really want to commit that if we can find an answer.

[gob33]

I visualy compared catalog controler PP Express v1.8.4 with mine current line by line but saw nothing important, the code is better in recent version dealing with possible functions failure.
My PPexpress version is same as 1.10.2
The improved log file should show more if it has not been deleted.
It is possible to test without Paypal by manualy simulating $response value in code.
Whereas:

2018-03-02 20:32:56 - PHP Notice: Undefined index: paypal in /var/www/web/catalog/controller/payment/pp_express.php on line 1355
2018-03-02 20:32:56 - PHP Notice: Undefined index: PAYERID in /var/www/web/catalog/controller/payment/pp_express.php on line 1360
2018-03-02 20:32:56 - PHP Notice: Undefined index: order_id in /var/www/web/catalog/controller/payment/pp_express.php on line 1363
2018-03-02 20:32:56 - PHP Notice: Undefined index: token in /var/www/web/catalog/controller/payment/pp_express.php on line 1366 

refer to $this->session->data['paypal'] not defined in function checkoutReturn().
A session problem ? Or one page checkout problem ?

[gob33]

I found a "$paypal_fee_fee" not defined in .tpl
The call() doesnt return false on curl error for catalog/model, it is different of admin/model call() for some reason ? Has it been overrided, i dont know, except these functions should be equal.

[villagedefrance]

Yes, "$paypal_fee_fee" is a "Total" extension, to add an extra fee for customers choosing any one of the Paypal payment options at Checkout. It only adds to the total amount to pay and shouldn't affect the processing.

I had a look at both call() functions in the Model files, Admin vs Catalog, and they are indeed different. Do you know which one is the good one and which one needs updating?

I am going to investigate on my side, comparing with OC main code.
Thank you for looking into this for us.

[villagedefrance]

OC v3.1.0.0b seems to be using 2 different version in Admin and in Catalog, 84 and 109 respectively.
No real other clues apart from that.

[gob33]

The good call() is in admin, it returns false when curl fails so that it is tested in controller
if ($response === false)........
Api version is unsafe, it varies with the country and even by Paypal merchant account.

If someone can explain me the real utility of RESEND button and call_data ??
Why resending saved data after a failure instead of re-cliking again on a CAPTURE / REFUND / VOID button ? Why they did that ??

[gob33]

I searched in closed pull requests and effectively I did a commit 331d107 for new Paypal Express but my changes in the catalog part seem have been all overwrited by old version.

[villagedefrance]

I don't remember making such big changes in the Catalog. I probably made some minor code standards adjustments but not changing an entire function like $call().

Any chance you could make a new PR? or send me your files so I can commit them back myself.

Regarding the RESEND button, yeah, that's weird because browser generally don't like that, and this is also potentially unsafe. No idea why it is in there.

[gob33]

I can do a new pull request starting from 331d107 and comparing but it will take some time as i go slowly on spare time.

[gob33]

For the moment i get an SSL error with github connexion
"error:1407742E:SSL routines:SSL23_GET_SERVER_HELLO:tlsv1 alert protocol version".
Unable to do something with that fucking github.

[gob33]

Solved by upgrading git for windows from 1.9.5 to 2.10.0. But I am limit of windows xp. Need win 7 but too long to prepare.

[villagedefrance]

Thank you for your efforts. they are much appreciated.
If you can't get Github working, you could just send me the files by email to commit, if that helps.

[gob33]

So i inspected PPExpress and only saw the catalog call() which should be rewritten as the admin call()
The rest seems ok using comparison tool.
I dont know how it has changed.

[gob33]

Seems recurring payment doesnt work in OC PPExpress: opencart/opencart#5084
Needs testing.

[gob33]

I have read the Paypal NVP code in OC deeply and i can tell that all errors reported by merchants do not surprise me. The code is approximative, not bullet proof, taken from a free extension which has not been updated since long time. The transaction view is incomprehensible with a formatRow() function comming from transaction search. The resend is idiot. They added a debug field to trace hazardous response. And nobody at OC cares about the code since 1.5.6 at least as it has not changed.