We currently have a site deployed with caching enabled. I have a secure pages rule to redirect user* (for secure sign on). For some reason it would NOT redirect for anon users. After some hacking around the following fix seems to resolve the problem:
securepages.module (line 39)
/*
if ($cache) {
return;
}
*/
I commented out the section of the securepages_boot() that exits the function if the page is cached and it now works. Could someone with more knowledge than myself on this module explain the reasoning for stopping the securepages_boot hook if the page is cached? Not trying to second guess, only solve my problem at hand.
Below are our config settings:
Drupal: 6.14/MySql 5.x/PHP 5.2.6
Securepages: 6.x-1.8
Caching Settings
Caching mode: Normal
Page Compression: Enabled
Block cache: Enabled
Optimize CSS: Enabled
Optimize Javascript files: Enabled
Secure pages settings:
Secure Pages: Enabled
Switch back to http: True
Secure/Non-secure filled out properly
Pages: (Make secure only the listed)
node/add*
node/*/edit
user
user*
admin*
Ignore Pages
*/autocomplete/*
*/ajax/*
Comment | File | Size | Author |
---|---|---|---|
#86 | securepages-cache-587000-86.patch | 754 bytes | tomswiggers |
#75 | securepages-removesession-addhook-587000-75.patch | 9.33 KB | shaisachs |
#74 | securepages-removesession-587000-74.patch | 8.04 KB | shaisachs |
#69 | 587000.69-securepages-cached-page-redirect.patch | 1.83 KB | mrfelton |
#63 | securepages.module.patch | 1.96 KB | adr_p |
Comments
Comment #1
stella CreditAttribution: stella commentedworked for me!
Comment #2
stella CreditAttribution: stella commentedI spoke too soon, it's still not working. I think it's because when the page is cached, the $_SESSION['securepages_redirect'] doesn't get unset when you navigate away,... maybe.
Comment #3
stella CreditAttribution: stella commentedThis patch seems to fix it for me. No guarantees, will see how it goes.
Comment #4
mrfelton CreditAttribution: mrfelton commentedHere is my version of the patch. I completely removed the implementation of hook_init, and ensure that the check is always run in hook_boot instead since that hook always runs regardless of cache. Seems to be working for me.
Comment #5
tekante CreditAttribution: tekante commentedThere are two issues at play here, I'm working on a patch but wanted to explain the issue for anyone else searching.
The securepages_redirect session variable appears to be present to prevent attempting to do a redirect lookup if you are coming from a redirect, in theory preventing a potential redirect loop is all I can determine. It was added in version 1.15.2.7.
This variable can get stuck set to true in the following case and allow you to view pages using the wrong scheme as long as they are cached and you request no uncached pages.
Here is the sequence that can cause the error:
Visit page http://example.com/page_at_secure_only, page is not cached and securepages redirects correctly, securepages_redirect is set to true at this point
Browser requests https://example.com/page_at_secure_only, page has become cached in the interim and is served from cache, securepages_redirect is not altered at this point as the page serving from cache preempts the call to securepages_init where the variable is unset.
User requests page http://example.com/other_secure_only_page, if this page is cached it will be served at the incorrect scheme as the redirect check is aborted in securepages_boot (if ($cache) { return; }) and securepages_init is never called due to Drupal aborting the request processing early to serve the cached page. securepage_redirect is still set to TRUE until a non cached page is requested and securepages_init has a chance to run and unset the variable.
A quick fix is to remove the cache check from securepages_boot and always do the redirect check in addition with removing the securepage_redirect session variable checking. This opens you up to a potential redirect loop in theory but I've been unable to imagine a case where securepages would both try to direct you to the secure url and then away from it with the same path. I'm working on a more complete fix and will submit a patch.
Comment #6
tekante CreditAttribution: tekante commentedPatch dealing with issues described in #5 attached.
Added a hook_exit implementation to ensure that the securepages_redirect session variable is cleared even if a page is served from cache and will ensure that the variable is cleared after any request which does not generate a redirect.
Comment #7
tekante CreditAttribution: tekante commentedAnother note for anyone having trouble, ensure you clear your cache tables any time you enable the module or make changes to your patterns since a page which is already cached is not checked to see if it should be redirected.
Comment #8
YK85 CreditAttribution: YK85 commentedsubscribing
Comment #9
phreestilr CreditAttribution: phreestilr commentedsubscribing
Comment #10
pcave CreditAttribution: pcave commentedThe 58700-6-securepages.patch comes close to fixing the issue, but don't think the securepages_redirected_this_request() check is necessary. If you were redirected, then $_SESSION['securepages_redirect'] will be set to true, so you can just unset the var. If you weren't redirected, it won't exist. The attached patch works for us.
Comment #11
jaybee1001 CreditAttribution: jaybee1001 commentedHi,
Is this patch addressing core page-caching, or will it also address the issue when using Boost?
Thanks
JB
Comment #12
tekante CreditAttribution: tekante commentedIt is my understanding that boost works by saving rendered pages as files and uses special htaccess rules so that if the page has already been created Drupal is never invoked at all. If this is the case then this patch does not address that issue. To make sure someone is at an https url with boost enabled you may need to add rewrite rules to the .htaccess file for which you can find documentation here: http://httpd.apache.org/docs/2.0/mod/mod_rewrite.html (see the HTTPS special variable for the RewriteCond directive)
Comment #13
AlexisWilke CreditAttribution: AlexisWilke commentedHi Tekante,
I have boost running on my secure site and it works just fine, without any patching.
The fact is that securepages will send anyone to the secure pages FIRST, then the pages will be created. Because of that, a non-secure version will never be cached by boost. If that happens, you have another problem.
Thank you.
Alexis
Comment #14
MadtownLems CreditAttribution: MadtownLems commentedThis seems like a major issue, and I'm experiencing it as well of course.
Are there plans to include it in a new release soon?
Thanks!
Comment #15
MadtownLems CreditAttribution: MadtownLems commentedI have applied the patch from #6, and cleared caches, but it doesn't seem to fix the issue.
I have it set to secure /user for logins.
when I visit the site home, it's not secure. perfect.
when i visit site.com/user - i get the secure. perfect.
when i go anywhere else on the site, it goes back to not secure - perfect.
however, if i go back to site.com/user - it is no longer secured. PROBLEM
the error disappears is i disable caching.
Comment #16
AlexisWilke CreditAttribution: AlexisWilke commentedMadtownLems,
It sounds to me that you check for user/login properly (which is where you are sent when the user is not yet logged in) but not /user itself. It may also be hidden (done internally.)
You may want to add "user" to the list of pages to make secure (at the bottom of the settings you have a way to do that.)
Thank you.
Alexis
Comment #17
MadtownLems CreditAttribution: MadtownLems commentedAlexisWilke,
Here are the pages I've made Secure:
user/*
user
admin*
Is there any way I can help debug this issue? It seems.... important :P
thanks!
Comment #18
AlexisWilke CreditAttribution: AlexisWilke commentedI just installed securepages on a site where I never used it and it works just fine, as you would expect. Actually, any time I go to /user it switches to HTTPS and I did not have to change the defaults.
node/add*
node/*/edit
user/*
admin*
I think that what happens is that you may be sent to user/<uid> internally, so user/* works in all cases.
Alexis
Comment #19
MadtownLems CreditAttribution: MadtownLems commentedThis is exactly how it works when page caching is disabled, yes. But with the core page caching set to Normal, I get the behavior I've been explaining. :|
Comment #20
AlexisWilke CreditAttribution: AlexisWilke commentedI see. I have that cache mode turned on too, but I also run with Boost. Could you try with boost as well and see whether the bug goes away?
Comment #21
jmuessig CreditAttribution: jmuessig commentedsubscribe
Comment #22
jmuessig CreditAttribution: jmuessig commentedI tried patch #10 and got into a redirect loop when navigating the admin menus - fwiw.
Comment #23
levelos CreditAttribution: levelos commentedExact same behavior as reported in #15. When visiting a secured page a second time, with normal page caching enabled, the page is NOT secured. Bumping up to critical for the obvious reasons. Some of these supposedly secure pages are collecting sensitive data, and it's unknowingly not secure.
Comment #24
phil_esq. CreditAttribution: phil_esq. commentedsubscribe
Comment #25
tekante CreditAttribution: tekante commented#15, #23 - I have been unable to replicate your described behavior.
Test details - Drupal 6.17 clean install, securepages 6.x-1.8 with patch from #6 as only additional module enabled, Drupal caching set to normal, default securepage settings except securepages set to redirect to http when no match
Initial request sequence - home page, /user, home page, /user
Second request sequence - home page, /user/login, home page, /user/login
Third request sequence - story page, /user, story page, /user
Anonymous user, redirected to secure at paths like /user/login in all cases even after several secure to non secure to secure to non secure sequences. Never redirected to secure for path of /user as expected with default settings.
Authenticated user, behavior same as anonymous user.
Modified securepage settings to add user to pages to make secure and repeated test sequence. Same behavior as previously but now /user also redirects to secure in all instances.
Please provide any information you can about your setup and the exact sequence of pages you visit to trigger the problem. Include anything you use that might affect caching (boost, memcached, varnish, squid, etc). A list of installed/enabled modules would be very helpful if you are testing against a non base install.
Comment #26
AlexisWilke CreditAttribution: AlexisWilke commentedtekante,
I can tell you that Boost is not a problem since I use it and never saw this problem.
I also have XCache running which compiles and caches PHP code, not the output.
However, memcached, vanish and squid could indeed be problems if they don't respect the expected HTTP nocache behavior.
Thank you.
Alexis
Comment #27
johnthomas00 CreditAttribution: johnthomas00 commentedsubscribe
Comment #28
neilnz CreditAttribution: neilnz commentedBoost users may be interested in my solution, to pre-rewrite all the links instead of redirecting. This means that there's absolute URLs in your Boost cache that go to the right places, so a bootstrap isn't required to get them on to HTTPS (or HTTP).
This is only useful if either you want to have some boost-cached HTTPS pages, or you want to switch them back to HTTP from HTTPS, but the link goes to HTTPS and the web server doesn't know better than to leave them on HTTPS for Boost (unless you configure it not to serve from boost for HTTPS requests).
Anyway, my patch is #893840: Support to pre-rewrite links to the correct protocol instead of redirect (patch) and needs the url_alter module to work.
Comment #29
Fortyhands CreditAttribution: Fortyhands commentedHello!
I too am having this issue.
When "Normal" Caching is turned on, I can visit the login page "/user" without being redirected to https.
When I disable caching altogether, this problem does not occur.
Other pages, such as checkout and admin are always redirected to https as expected.
Happens under all browsers.
The strange thing is, that I can only get the problem to occur when I am visiting the login page via a link from the home page OR if I type in the url manually.
I do not have boost or any other caching mods installed. I do have hijack prevention module enabled, but have tested with and without and the issue persists.
Login toboggan is installed.
I have tested the latest Secure Pages module with the patch from #5 applied. I have also tested the dev version of the module - problem still occurs.
Tested "set to http when no matches found" on/off.
Secure Pages is set up for the following pages:
user
user/*
user/*/edit
admin
admin/*
cart/checkout
cart/checkout/review
cart/checkout/payment_details/*
cart/checkout/complete
uc_paypal/ipn/*
uc_paypal/wps/*
uc_paypal/wps/complete/*
cgi-bin/webscr
Should I just disable caching altogether for the time being? A bit of a bummer to have this crop up on a production site that collects credit card info. Caching disabled, still looking for a solution.
Comment #30
DannyPfeiffer CreditAttribution: DannyPfeiffer commentedSame issue here... When page caching is disabled, https is intermittently properly redirected on the /user/* paths. Store checkout paths seem to always work properly.
Disabling page cache altogether fixes the problem.
I'm also not using boost, but AM using login toboggan.
Comment #31
kenwest CreditAttribution: kenwest commentedI applied the patch supplied by @pcave at #10 and this solved my redirect problem:
Set up
To reproduce problem
@pcave's patch saves the day by unsetting securepages_redirect once it's been finished with
Comment #32
Jaapx CreditAttribution: Jaapx commentedAlso having the problem that http://mysite.com/user is not redirected to https://mysite.com/user I trried the patches #6 and #10 but these didn't work for me.
My installation: D6.19 and the most rececent 6.1x-1.x dev of secure pages. Normal caching is on, no special caching modules like boost installed.
When the cache is cleared and I logged off, the first login with http://mysite.com/user redirects to https://mysite.com/user. Login's that follow immediately the first don't redirect anymore. A login after a while however does redirect to https.
It seems to me that Drupal stores http://mysite.com/user in cache. When this form is cached it is delivered to the user whithout invoking the module Secure Pages. A specific exclusion of http://mysite.com/user from caching could be a solution but I do not know how to realise that without patching Drupal core.
Comment #33
Robert Castelo CreditAttribution: Robert Castelo commentedSame problem here as defined in #31 but none of the patches above solved it for me.
Cause:
Once $_SESSION['securepages_redirect'] is set it never gets unset, and so no evaluation happens to determine if subsequent pages should be secure or insecure.
Setup:
Drupal 6.20
Normal Cache: enabled
Solution:
Instead of just setting $_SESSION['securepages_redirect'] as a flag that a redirect has happened, we store the path that the redirect is for. When the user goes to other pages, even if the flag has not been cleared we can check it against the current path and clear it if there's no match, triggering an evaluation for secure or insecure.
Comment #34
bcmiller0 CreditAttribution: bcmiller0 commentedsubscribe
Comment #35
sreynen CreditAttribution: sreynen commentedI solved this by adding
unset($_SESSION['securepages_redirect']);
to the top of securepages.module. That doesn't seem to have broken anything else, but it effectively makes it not a session variable, so I can imagine that might break something I just haven't noticed. My question: why is securepages_redirect a session variable in the first place? Is there some reason that variable needs to persist between HTTP requests and couldn't just be a global variable within a single request?Comment #36
AlexisWilke CreditAttribution: AlexisWilke commentedsreynen,
I think it is necessary if you want to change the sub-domain name.
For instance I use http://www.m2osw.com/ as my main website URL. When you log in, you are sent to: http://secure.m2osw.com/
But I think that in this case the problem is that the user, after the log in, would be sent back to http://www.m2osw.com/ which is something I don't use.
If you don't switch sub-domains, I think you're good. But that's just my bare knowledge of the code. No guarantees... 8-)
Thank you.
Alexis
Comment #37
sreynen CreditAttribution: sreynen commented@AlexisWilke, that doesn't make sense to me. Even if you're redirecting to another domain, why would you need to maintain the value of securepages_redirect in $_SESSION after the redirect happens?
Comment #38
Robert Castelo CreditAttribution: Robert Castelo commented@sreynen $_SESSION['securepages_redirect'] is used to note that a redirect has happened so as to prevent another redirect on page reload and an endless loop.
Comment #39
sreynen CreditAttribution: sreynen commented@Robert Castelo, yeah, I figured that out when I ran into an endless loop. So now I'm back to pages that only sometimes redirect.
Comment #40
Robert Castelo CreditAttribution: Robert Castelo commented@sreynen have you tried my patch #33 ?
Fixed the problem for me in all cases.
Comment #41
kenwest CreditAttribution: kenwest commentedInstalling version 6.x-1.9 resolves the issue I was having (#31)
Comment #42
jmuessig CreditAttribution: jmuessig commentedPatch #33 worked for me with the latest version of securepages (1.9). I had to manually modify securepages.module, though, since the patch is not yet updated.
Patch #6 also worked for me in the past. Which is better?
Comment #43
jmuessig CreditAttribution: jmuessig commentedOK, here are my results for securepages 1.9.
Patch #6 (needs to be patched manually) still works great with normal cache on. No problems thus far after quite a bit of testing.
Patch #33 - seems to work at first, but has some issues. In IE, I kept going back and forth between my home page (http) and my cart (https). Eventually, I think IE was slow enough that I confused it and it went to my home page, but with https. After that point, it started serving all home page requests securely (instead of insecure, which it should do).
I was able to repeat this several times with patch #33 by adding an "s" to http on my home page. I then clicked on my cart (which should be secure and was) and then clicked on a link to my homepage, and sure enough, it was serving it as secure when it shouldn't have. It gave a "page not available" error when I manually entered http://homepage.com. Repeated on chrome as well as IE8.
In any case, patch #6 has worked for me for a long time, and continues to with the latest rev of securepages 1.9.
HTH
Comment #44
dbassendine CreditAttribution: dbassendine commentedI also tested the #6 patch against a clean install, and it worked well to resolve the $_SESSION['securepages_redirect'] issue. However I also encountered the issue from #15 on another build, and also couldn't reproduce it on a clean build as described by #25.
The steps required to reproduce the issue were:
1. clear caches
2. type in http://site.local
3. click through to http://site.local/user/login
* redirects to https://site.local/user/login (CORRECT)
* but, in the cache tables http://site.local/user/login is cached (instead of https://site.local/user/login on the clean build)
4. click back to https://site.local
* goes to https://site.local (INCORRECT: Problem 1)
* reset by going to http://site.local manually
5. click through to http://site.local/user/login manually
* goes to http://site.local/user/login (INCORRECT: Problem 2)
* securepages_boot finds http://site.local/user/login is cached and aborts instead of redirecting to https://site.local/user/login
I put some break points in core bootstrap.inc and found that the $base_url setting in settings.php is forcing the $base_root global variable to be http://. If $base_url is not set, the bootstrap checks for https and modifies the $base_root global accordingly. However, if $base_url is set in settings.php the https check is overridden.
This creates the two problems identified above:
* Problem 1: User clicks through to https://site.local. securepages_boot() calls page_get_cache(), which looks for $base_root and finds the http://site.local page cached (from cache_page table). It returns the cached page (on the https url), and doesn't redirect to http://site.local
* Problem 2: After the user loads https://site.local/user/login the first time (step 3), Drupal runs page_set_cache (common.inc) using $base_root (incorrectly using http) - so the page is cached as http://site.local/user/login. Then when the user comes back to click on a link to http://site.loca/user/login, securepages_boot check the cache and finds the cached copy of the http page, aborts and doesn't redirect to https://site.local/user/login
So: if you enable $base_url in settings.php, the steps above are reproducible on a clean install (d6.20, securepages 1.9 + patch#6). I'm working on a patch and will submit one below.
Comment #45
dbassendine CreditAttribution: dbassendine commentedHere's a patch to fix the $base_root variable problem described above.
I have added a function call to securepages_boot that corrects $base_root before i) the caches are checked by page_get_cache() (also in securepages_boot; and ii) before the page itself is cached. This is applied to both secured and unsecured pages - to cover the (albeit unlikely) case where the $base_url is set to https:// as well as http:// in settings.php.
The patch was created against 1.9 after patching with #6.
Hope that helps! David
Comment #46
drupalsteve CreditAttribution: drupalsteve commentedI am also experiencing this problem. Secure Pages works correctly with caching disabled, but is inconsistent in normal caching mode. For now, I'm forced to leave caching disabled to ensure my website safely processes secure transactions. Looking forward to an updated version of Secure Pages with a fix.
Comment #47
chicodasilva CreditAttribution: chicodasilva commentedI have the same problem. Is there any update on this issues?
Comment #48
Reevescorp CreditAttribution: Reevescorp commentedUpgraded to Drupal 6.19 > 6.20 and now secure pages is FUBAR. Didnt notice it at first because the admin account had already been connect securely to the site. If you load the site with https secure pages will work fine after that.... But if it is a new browser session the secure links like "log in" are http not https. And when you try to hit the http of a secure page its generating a php error "Fatal error: Call to undefined function user_access()" and the page remains blank unless you press the reload button then the page will reload in http format (not good for a store processing credit cards).
Once you hit the site in https... everything will work properly. So I resorted to hard coding my secured entry links to https. (crappy solution but at least its better than blank pages). I applied the patches in both #6 and #45.... no go.... (double checked the patch to insure operational) still no go. Site is operational with patch but still has same problem. Im not running cached pages since it interferes with ubercart shopping cart. I could do a rollback to 6.19 but I hate going backwards. Don't know what other information to provide so feel free to ask. Oh I also tried the dev version of secure pages... no go... same problem so reverted back to 6.x-1.9
Comment #49
AlexisWilke CreditAttribution: AlexisWilke commentedReevescorp,
Are you using SecurePages with that "strange" setting "Send people back to HTTP after they logged in"? If not, then your problem is probably something different. Just sayin'... I have an online store too and did not get any such problem (I'm using Übercart and Core Log In page.) However, I have problems because of Boost when I don't properly mark such and such page as non-boosted (non-cached.)
Thank you.
Alexis
Comment #50
Reevescorp CreditAttribution: Reevescorp commentedMy Secure page settings
X Enabled
X Switch back to http pages when there are no matches
Non-secure Base URL: http://www.signagemart.com
Secure Base URL: Secure Base URL: https://www.signagemart.com
X Make secure only the listed pages.
user
user/*
user/*/edit
users
users/*
users/*/edit
admin
admin/*
cart/checkout
cart/checkout/review
cart/checkout/payment_details/*
cart/checkout/complete
Ignore pages:
*/autocomplete/*
*/ajax/*
*/imagecache/*
image_captcha/*
Comment #51
Reevescorp CreditAttribution: Reevescorp commentedYou can still see the error on the site...
http://www.signagemart.com
Add something to the cart... click view cart on the top link (dont click checkout on the top link). From the view cart page click the orange checkout button. The screen will go blank. That button should be taking you to an https connection but for some reason its giving you an http link. Now from the blank page... click back... the cart will show back up... click the hard coded checkout at the top... the log in page appears as it should. But the secure url was a hard coded https:// link vs a simple /checkout link
This was all working fine for nearly year until I upgraded to 6.20
I'm only going to leave the site like this until early Monday morning.... then going to rollback to my 6.19 backups so I can go back to be operational for the work week.
Comment #52
AlexisWilke CreditAttribution: AlexisWilke commentedHmmm... I tested and got a WSOD indeed. Not following your exact steps, but none the less, got the white screen.
Do you see any PHP errors in your Apache logs when that happens? In most cases, that's where I find problems. Function does not exist, missing ; or ), etc.
Thank you.
Alexis
Comment #53
Reevescorp CreditAttribution: Reevescorp commentedYeah.... I get
PHP Fatal error: Call to undefined function user_access() in D:\Websites\Signagemart.com\includes\menu.inc on line 449
Believe that is happening because its trying to call the user_access function from the non secure url and cant find it.
What sucks is my development site doesnt have a secure setup.... And I need to put my production site back online with the 6.19 backups. Guess I could add one with a server generated certificate so could debug this problem.
Comment #54
AlexisWilke CreditAttribution: AlexisWilke commentedFrom what I can see the only place where it could happen is the 'exit'. securepages itself doesn't call user_access().
See the securepages_goto() function.
I would think that what's happening is a conflict with another module which thinks that everything was initialized when it wasn't (since you're still in the bootstrap level: DRUPAL_BOOTSTRAP_LATE_PAGE_CACHE)
Thank you.
Alexis
Comment #55
Reevescorp CreditAttribution: Reevescorp commentedYou're right!
Doing a netbeans xdebug and moving a watch around in securepages.module -
Its dying here
Getting tired... will walk through some more of it after a few hours of sleep.
Comment #56
Reevescorp CreditAttribution: Reevescorp commentedOk... after several hours of debugging...
It was dying on the statistics module. Disabled statistics.... problem went away. Haven't traced into statistics as of yet and that might not be the only module that it is having an issue with. I added code to the section to have it write a temp file so I could see what modules it was processing when it dies.
Going to do some more testing, if it doesnt die on any more modules, I'll set some watch points in statistics to see what it is choking on.
Comment #57
Reevescorp CreditAttribution: Reevescorp commentedUpdate on the statistics bug:
Traced the problem down to this section of the statistics module
Figured I see what part was causing it to die so I started replacing variables with hard coded strings... Sure enough... first one made the problem go away.
Now what possibly in strip_tags(drupal_get_title()) could be making secure pages crash?
Obviously this is a different error than the cache problem. Not sure if I should start a new ticket for it or not.
Comment #58
AlexisWilke CreditAttribution: AlexisWilke commentedThe title must be the culprit. If the page is a node and that node is not otherwise accessible then its title is securely hidden. The strip_tags() is a PHP call anyway. 8-)
Thank you for all your hard work.
Alexis
Comment #59
Reevescorp CreditAttribution: Reevescorp commentedWell,
This was all working before I upgraded... and there is nothing mysterious about the checkout page title. I am however running the page title module so possibly may be a conflict going on there. Ill put some code into the call to write the page title to a file both stripped and non... See what it is trying to write to the database... maybe the database write is choking on something weird being in the title and causing the whole thing to crap out.
Comment #60
Reevescorp CreditAttribution: Reevescorp commentedDefinitely something wrong calling the page title. Seems to call the title intermediately (but the title on the page always shows) Wondering if drupal_get_title() fails and calls a nul and you try to use the php strip_tags on a nul title if that would cause the crash. But then that should be showing up in the php error log and its not. Amazing how something as unrelated as a page title would be killing secure pages.
Comment #61
jrust CreditAttribution: jrust commentedSubscribe. Patch from #33 worked for me.
Comment #62
drupalsteve CreditAttribution: drupalsteve commentedI manually applied the patch from #33 to version 6.x-1.9; it seems to be working so far.
Comment #63
adr_p CreditAttribution: adr_p commentedHere's my solution for described problems. While I was testing the module behavior I found problems like:
The patch posted in #33 seems to solve the second and the third issue but not quite. It's not working when a user stubbornly tries to use https on 'regular' page - he will succeed after the second attempt.
The patch bellow appears to solve all the issues. Please don't hesitate to try it and post the results of your tests, including potential side effects.
Comment #64
grendzy CreditAttribution: grendzy commentedI'd be in favor of removing the use of $_SESSION - it's introduces a race condition, there's at least one reproducible bug in this issue, and it's not in the 7.x branch - but hardcoding exceptions for 2 other modules is going to be a hard sell.
Comment #65
adr_p CreditAttribution: adr_p commentedI agree that hardcoding module names is not a perfect way to get rid the $_SESSION variable or bugs, but in my opinion this solution is clearly better than the existing one. If I haven't missed any side effects it solves most (or maybe all?) of the redirection problems.
The biggest problem we have here is that at this step of drupal's bootstrap where hook_boot() is being called we don't have access to normal paths, we can only see a path in a form sent by the browser. Hence securepages implemented also hook_init(), where aliases are already resolved and all of the modules loaded. In order to remove hook_init() implementation (and $_SESSION variable as well) I've slightly overtook bootstrap process and forced path initialization. The problem is that modules that can affect this process aren't loaded yet. So this is why manual module loading is required: modules implementing e.g. hook_url_inbound_alter() or similar are needed at the moment.
If you hesitate about harcoding (well, I am) then maybe the solution would be to let other modules to decide if securepages should load them. For obvious reasons we can't do that by defining a hook, but other modules could do that in their info files, so securepages could parse it from time to time to keep internal registry up to date. It sure won't be easy to delegate responsibility, but I believe it's worth consideration. What do you think?
Comment #66
AlexisWilke CreditAttribution: AlexisWilke commentedIsn't the module weight there for that reason? I guess even just loading what necessary for the paths isn't enough then... but the weight is used in two places: to load modules at boot time, then to load the remaining modules once Drupal was booted. I think that having a hook_boot() is enough to get these other modules to load at boot time and that would certainly be enough to make sure they're there, wouldn't it?
It's kind of sad that this is required, but that's certainly would make it a lot easier than what you're talking about.
Thank you.
Alexis
Comment #67
drupalsteve CreditAttribution: drupalsteve commentedThis comment pertains to Ubercart, but may be relevant to viewers of this issue. I'm experiencing a problem when the caching mode is set to normal. I use Ubercart, Ubercart Add to Cart Tweaks, and Ubercart Variable Price to process transactions. When normal caching mode is enabled, the product price is not properly passed to the checkout page; the checkout page will ignore variable options and always use the default price. When I disable normal caching, it works fine. The problem persists with Secure Pages disabled, so I presume it must be an issue with one of the Ubercart modules that I am using when normal caching is enabled.
I'm using Drupal 6.22, MySQL 5.1.53, PHP 5.2.17, and Secure Pages 6.x-1.9 with the patch from comment #33 applied.
Comment #68
mstrelan CreditAttribution: mstrelan commentedsubscribe
Comment #69
mrfelton CreditAttribution: mrfelton commentedPatch updated to work with latest git code, and to apply cleanly through drush make.
Comment #70
grendzy CreditAttribution: grendzy commentedWould someone mind updating the issue summary and title? It's hard for me to review patches because I'm not really sure what's trying to be fixed. I'm pretty sure the issue has nothing at all to do with caching. I think there is, however, a race condition caused by trying to prevent redirect loops by storing stuff in $_SESSION.
Comment #71
yub_yub CreditAttribution: yub_yub commentedsubscribe
Comment #72
msumme CreditAttribution: msumme commentedConfirmed that the patch in #10 solved my problem as well.
I had behavior in #15 and #23.
It's the smartest solution, because that var should ALWAYS be unset on the end of a request so it can't be hanging around at the beginning of the next request in the session.
Further, it really shouldn't be in the session at all. It should just be a static var set in a class or in a function so it can't persist past one request.
Comment #73
msumme CreditAttribution: msumme commentedAlso, it would be easier to understand and not need a session variable in the first place if it just happened a little differently...
instead of securepages_match having the possibility of returning null, it should always return true/false.
To make that clear, i renamed it in the example here "securepages_should_be_secure", which returns a boolean
I haven't tested any of the following yet, but it shows an easier to understand structure.
Then redirect could be simpler given the above change.
I replaced securepages_goto with securepages_switch_ssl($on) :
It only does one thing... changes ssl to non-ssl or vice-versa
With this approach, you don't really need to set a session variable at all, because it works in a stateless fashion.
Comment #74
shaisachs CreditAttribution: shaisachs commentedIn the interest of moving the ball down the field, I've created a patch which follows the strategy in 73, does a bit of cleanup, and also applies the same idea to form_alter and link_alter for consistency's sake. It's not yet tested though.
Comment #75
shaisachs CreditAttribution: shaisachs commentedThe patch in 74 seems to be working relatively well. I've extended it to support a hook which other modules can implement in order to have more granular "input" on whether or not a page is secure, which addresses the issue in #217171: Add ability to secure Nodes based on Content Type, sort of. (Sorry to go a bit out of scope on this thread.)
Comment #76
seanrLooks good, but a minor code formatting issue here:
Original was correct. Note that for D7 and beyond, it'd actually be header('Location: ' . $url); but the original was correct for D6. There may be more, but that one in particular jumped out at me.
Also, haven't dug too deeply in the code, but watch out for hook_init vs hook_boot. Hook_boot gets executed before any caching, while hook_init fires afterward, so by moving that redirect call from boot to init, you may be introducing problems with caching. Be sure to test that very carefully, with both normal and aggressive caching.
Comment #77
shaisachs CreditAttribution: shaisachs commentedDuly noted on coding standards, thanks!
With regards to boot vs. init, the issue that caused me to remove the redirect in boot in #75 is that not all modules are loaded in boot. Therefore if you try to redirect in boot (and you are using the hook) you could get a false positive and cause an infinite redirect.
For example, suppose you have a module which wants to indicate that any path which matches the pattern ([abc]+)+ is secure, and that module does not load during boot but is only available in init; furthermore you want your site to redirect back to http on every other page. Since you can't include every possible path that should be secure in securepages_pages, you just let the should_be_secure hook handle the pattern matching. Then you try to visit a page like "aaabbbccc", and you get the following behavior.
Perhaps the best approach would be if hook_boot were to implement only a one-way redirect, i.e. hook_boot only redirects from http->https if the page you are visiting does in fact match the securepages_pages. I suppose that could still get messy in aggressive caching, though. In any case, perhaps we should bring this conversation to another thread.
In other news, the strategy in #73 (implemented in the patch in #74) has been tested for a little while now and seems to be working. I think removing SESSION dependency from this module altogether would probably be for the best, and it seems that this approach works pretty well.
Comment #78
grendzy CreditAttribution: grendzy commentedThis needs an issue summary, and an accurate title. FWIW, 7.x has already removed the use of $_SESSION and removed hook_boot. These changes are on the roadmap for 6.x-2.0.
Comment #79
grendzy CreditAttribution: grendzy commentedPlease try the 6.x-2.x-dev release.
Comment #80
rickbydesign CreditAttribution: rickbydesign commentedHow do I apply your patch?
Comment #81
grendzy CreditAttribution: grendzy commentedrickbydesign: http://drupal.org/patch/apply
Instead of patches, please try the 6.x-2.x release, which is expected to resolve a number of issues.
Comment #82
ShaunDychko CreditAttribution: ShaunDychko commentedPatch #69 works wonderfully so far. Other patches (such as #6, #10) didn't redirect after repeatedly entering http://example.com/user.
Comment #83
wiherek CreditAttribution: wiherek commentedWhat about the d7 version? My site doesn't redirect back to http from https pages. That is when I have page caching and boost enabled.
I tried doing the trick with giving secure pages a lower module weight than boost. That didn't work. Major performance issue.
Comment #84
grendzy CreditAttribution: grendzy commentedI'm closing this issue as fixed. The 6.x-2.x branch resolves the issues with $_SESSION.
Regarding boost, I think it's expected to not cache HTTPS responses per #516670: Boost should consider skipping SSL requests.. Boost won't have full interoperability with SSL until #1070048: New folder structure (which proposes adding the REQUEST_SCHEME to the cache key) is resolved.
Comment #86
tomswiggers CreditAttribution: tomswiggers commentedHi,
I have tested with branch 6.x-2.x and the issue isn't resolved.
Can you please apply this patch?
Thanks in advance,
Tom
Comment #87
grendzy CreditAttribution: grendzy commentedHi Tom,
This issue has such a long and confusing history, that I'd prefer it to remain closed. Please open a new issue if you are experiencing problems with the 6.x-2.x branch. Please make sure to follow the issue summary template, and in particular include detailed steps to reproduce your issue. Step one in your report should be install Drupal core from scratch. Any information you can provide about your server is useful too. Because of the difficulties in reproducing others' server environments, the level of detail needed is perhaps a bit higher than in other issue queues.
It would also help if you could explain why you believe the module should
unset($_SESSION['securepages_redirect'])
when the 6.x-2.x branch does not use sessions at all. Thanks!Comment #88
tomswiggers CreditAttribution: tomswiggers commentedHi,
Thanks for your answer.
Indeed the patch that I attached isn't valid when working with 6.x-2.x branch.
I created this patch upon 6.x-1.x branch.
I double checked my testings and now I can't reproduce the issue anymore. So I must have done something wrong the first time.
Sorry for the extra confusion.
Going forward with the 6.x-2.x branch when do you think that will be released?
Thanks,
Tom