Hey folks, so I spent the last few hours of my Thursday figuring this one out. I collect CC payments AND PayPal, so I have mixed SSL mode on my site, and use this module.

The problem I had was two-fold. First, I noticed all PayPal sales on my site would not show up in Google Analytics, whereas CC payments through authorize.net would show up just fine. Second, after investigating further, I found that all PayPal sales were getting redirected to '/cart' instead of '/cart/checkout/complete' .

Why would this happen? I looked through the uc_paypal code and found out the point where it failed on completing the txn. Did a little debugging and found that when it hits that page, it doesn't have the SSESS ID, just the SESS ID. UC Paypal stores the order ID in the secure session, meaning its linked to the SSESS ID. This clue led me to watch the network trace in Chrome as I finished a PayPal order. Ubercart SSL was redirecting https://www.example.com/uc_paypal/wps/complete/9286/ to http, because this URL is not included in the SSL include paths by default. This made the payment completion happen on http, which lacks the secure session, which holds the order ID variable. After looking through this modules code I came up with an easy fix.

Here's the fix, I apologize for having bad code presentation standards, I'm new to posting in the drupal community, though I've been coding drupal for about 2 years. In uc_ssl/uc_ssl.module (version 7.x-1.1) find line 126 with the function uc_ssl_exclude_ssl_switch_paths(). Add this line of code into the $paths array, around line 129:

	'PayPal WPS redirect fix' => '/uc_paypal/*'

This makes uc_ssl ignore all paypal requests, meaning if they are sent to http (for non ssl checkouts) it will NOT switch back to https, but if you are doing https checkouts for PayPal (since you also collect CC or for customer reassurance) then it will NOT switch ssl requests back to http and break the workflow. Does this make sense? Does anyone else have this issue? This should probably be patched to make this module work better with uc_paypal in mixed SSL mode, as uc_paypal is a core part of Ubercart. Thoughts?

This immediately fixed my issue and redirects people properly to /cart/checkout/complete (in https) after returning from PayPal. I am still having issues with the 10-second delay PayPal insists on after completion. I'm not sure how to resolve it and have found PayPal support to be poor at best. But for those who wait or click the link back, it works perfectly, tracks in Google Analytics, etc.

Let me know if you have any questions. This should probably be an option in the module OR possibly be excluded from switching either way by default.

Comments

swensor’s picture

swensor commented

I also made a helper module that fixes ONLY this issue:

File uc_ssl_wps_fix.info:

name = Ubercart SSL PayPal WPS Fix
description = Fixes a frustrating bug in Ubercart SSL that breaks google analytics and proper redirection to cart/checkout/complete
core = 7.x
package = Ubercart - extra

files[] = uc_ssl_wps_fix.module
; Information added by drupal.org packaging script on 2011-02-25
version = "7.x-1.x-dev"
core = "7.x"
project = "uc_ssl_wps_fix"
datestamp = "1298620153"

File uc_ssl_wps_fix.module:

<?php

/**
 * @file
 * Ubercart SSL WPS Fix module file
 */

function uc_ssl_wps_fix_exclude_ssl_switch_paths()
{
   $paths = array(
	'PayPal WPS redirect fix' => '/uc_paypal/*'
   );
   return $paths;	
}

One thought, on my site I actually used uc_ssl_wps_fix_include_ssl_paths() instead of exclude_ssl_switch_paths() since I am ALWAYS wanting this to happen in SSL mode. I'm not sure what is most proper for patching this module (uc_ssl) but this fixer module will apply the patch on top, since uc_ssl has the hooks. So you can have it force HTTPS using include_ssl_paths or you can have it not care and not force any redirects with exclude_ssl_switch_paths() . Anyways, questions/comments welcome!

crystaldawn’s picture

crystaldawn commented
Status: Needs review » Closed (works as designed)

This is why there is a hook for changing the redirect urls. This module only supports BASIC ssl support for ubercart and drupal commerce. It does not try to cover every possible combination of contrib modules (such as uc_paypal for example). This would make for a good contrib module for those that use paypal. Its small, doesnt do much, but you get to maintain a module on drupal.org ;) A link to it would be useful if it's something you've already put up.

crystaldawn’s picture

crystaldawn commented

This is why there is a hook for changing the redirect urls. This module only supports BASIC ssl support for ubercart and drupal commerce. It does not try to cover every possible combination of contrib modules (such as uc_paypal for example). This would make for a good contrib module for those that use paypal. Its small, doesnt do much, but you get to maintain a module on drupal.org ;) A link to it would be useful if it's something you've already put up.

swensor’s picture

swensor commented

crystaldawn, thanks for the feedback. I disagree, here's why.

UC_Paypal is NOT A CONTRIB MODULE, it is a part of ubercart core. It's included in every installation of ubercart. Furthermore, PayPal is an extremely popular payment solution that many stores offer.

You should really patch the module. The problem is extremely difficult to identify, and then finding my feeble contrib module to fix it would be even moreso. You're just one line of code away from making your module work as designed with a CORE part of ubercart. I don't see why you wouldn't. Your point about supporting BASIC ssl "support" is IMO just an excuse. It's well within the spirit of this module to make redirects work with a popular payment method that is PART OF UBERCART.

Furthermore, adding ANOTHER module is arduous. Its yet another module site admins must identify, read about and install. It adds two more files that your Drupal site has to run EVERYTIME you render a freaking page! Not that it's a huge performance hit, but it makes no sense unless absolutely necessary.

Is patching a module that hard? It would beckon the 800+ users of your module to upgrade and fix this problem whether they know about it or not. You'd be improving user experience on hundreds of live websites, practically overnight. Why not?!

If you insist on leaving this flaw in your module, then I guess I'll post a tweaker module to fix your module for other people. In any case, I hope this post has been useful to those more experienced in patching modules, etc.

Pardon my criticism, and thanks for this module, it's been quite useful. Cheers!

swensor’s picture

swensor commented
Status: Closed (works as designed) » Needs review
crystaldawn’s picture

crystaldawn commented

So while your request is not in any way unreasonable or complicated, it is something I cannot test for myself and thus I cant actually "support" it. This is why it's not in the module. If I cannot test it myself, I dont add it and I leave it up to contribs who DO have access to XYZ gateway to do that for me. UC is a very large project and the authors have access to ALL of the supported gateways (many moons ago uc_paypal use to be in a dir called 'contrib' within UC which has since changed because the contrib authors are now also on the UC team). I dont have that luxury. Let's say something in paypal changes that breaks your fix and makes it much harder to figure out down the road and you're no longer around. I wouldnt have the ability to fix it, but because it's in uc_ssl, people would be looking to me to provide a fix. The best person to provide such a fix would be someone who actually has the ability to test the gateway in question (paypal in your case).

I hope that makes it more clear as to why gateway modules are preferred over putting support directly in this one. Sure, the module for Paypal may only be 1 line, but it does a couple things. It helps you establish that you are a module contributor and gives you something very simple to maintain as your first module. Gateway modules for uc_ssl are excellent starter modules for those just getting into Drupal and I can see that you dont yet have any public modules and are relatively new (6 months'ish). This would be a good opportunity to help contribute what you can, even if its just a small contribution. No contribution is to small. And actually, the smaller it is, the better it will run and the less prone to errors it will be which results in better stability for all.

crystaldawn’s picture

crystaldawn commented

BTW one thing I forgot to ask is, if you would like to patch the module yourself, that is also an option. I am not opposed to co-authors who can support specific features such as adding a gateway. All I would need to know is your developer username. You get that when you create your first public module. But I know you dont have one, yet, so I dont think that I could simply add you. You'll need a public module first before I can add you as a maintainer as well. Then you could add the small change yourself and help support it.

swensor’s picture

swensor commented

Sorry for the delay, haven't been as active the past year w/ career changes. I would love to patch this module, as you said I don't have any public modules and probably won't for a while. Unless you think I could add this tiny module as a helper module. What do you think?

And I want to apologize if I came off rash, I was just pulling my hair out over this one in kind of a unique use-case scenario. Thanks for the module, it's great!!