ezpublish-legacy icon indicating copy to clipboard operation
ezpublish-legacy copied to clipboard

Workaround to the get the customer order view working with email addresses that include a "+" character

Open pbek opened this issue 6 years ago • 13 comments

This is a continuation of the closed pull request https://github.com/ezsystems/ezpublish-legacy/pull/1199 with suggested changes from @andrerom.

pbek avatar Sep 20 '18 10:09 pbek

Travis fails with Fatal error: A database transaction in eZ Publish failed.. Since this problem already occured 3 days ago, I guess my pull request has nothing to do with it.

pbek avatar Sep 20 '18 10:09 pbek

I guess my pull request has nothing to do with it.

yeah that is not relevant here.

andrerom avatar Sep 20 '18 11:09 andrerom

It would be good to have steps to reproduce that issue. I want to test if that's related to: https://github.com/ezsystems/ezpublish-legacy/pull/1360

pkamps avatar Sep 20 '18 13:09 pkamps

@pkamps, please see: https://jira.ez.no/browse/EZP-24798

pbek avatar Sep 20 '18 13:09 pbek

OK, I will try if I can reproduce the issue. I honestly don't have a webshop site currently. Would you mind to test the patch in #1360 and see if it fixes the issue for you?

pkamps avatar Sep 20 '18 19:09 pkamps

@pkamps, I tested your pull request and it didn't solve the issue of showing customer information with urls like https://your-server/admin/shop/customerorderview/10/test%[email protected]

pbek avatar Sep 21 '18 06:09 pbek

@pbek You're right, the patch from #1360 is not enough to fix the issue. I see that you fixed the missing url encoding in the template - that was missing and is not a workaround. I don't think you need to work with a query parameter ?email=. It's probably cleaner to stick with the the ezp ordered parameter.

The additional URL decoding $Email = urldecode( $Email ); is the problem. It's already decoding the query string in: https://github.com/ezsystems/ezpublish-legacy/blob/master/lib/ezutils/classes/ezsys.php#L1184

So it's safe to just remove that line from the code base. Verdict: I suggest to add the url encoding in the template and remove the line $Email = urldecode( $Email ); from the module/view.

pkamps avatar Sep 21 '18 07:09 pkamps

Oh, and maybe you still need the patch from #1360 - but I'm not sure, there is a chance it works without that patch.

pkamps avatar Sep 21 '18 07:09 pkamps

@pkamps @pbek After some checking I think I found the root of the problem: We get $Param['Email'] from URI (eZURI), which is initated here: https://github.com/ezsystems/ezpublish-legacy/blob/master/kernel/private/classes/ezpkernelweb.php#L1083 using eZSys::requestURI().

eZSys::requestURI() is initaited using urlencode (https://github.com/ezsystems/ezpublish-legacy/blob/master/lib/ezutils/classes/ezsys.php#L1184), then passed to eZURI::instance( eZSys::requestURI()) which again uses urlencode (https://github.com/ezsystems/ezpublish-legacy/blob/master/lib/ezutils/classes/ezuri.php#L89), so we end up with email encoded twice.

So: test%2Btest%40example.com is becoming [email protected], and then test example.com.

But as I see this is exactly what https://github.com/ezsystems/ezpublish-legacy/pull/1360 should be fixing.

mateuszbieniek avatar Sep 21 '18 13:09 mateuszbieniek

That's how we'll fix the issue in our fork: https://github.com/mugoweb/ezpublish-legacy/pull/119

pkamps avatar Sep 21 '18 21:09 pkamps

@pkamps

That's how we'll fix the issue in our fork: mugoweb#119

that didn't seem to work for me, the urlencoded + didn't get across $Params

pbek avatar Sep 26 '18 07:09 pbek

I have to test it, but it seems like https://github.com/mugoweb/ezpublish-legacy/pull/119 alongside with https://github.com/ezsystems/ezpublish-legacy/pull/1360 seems like a good approach and should work.

mateuszbieniek avatar Sep 26 '18 07:09 mateuszbieniek

I would also like if my workaround with the extra get-parameter isn't needed any more.

pbek avatar Sep 26 '18 07:09 pbek