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/*

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

stella’s picture

securepages.module (line 39)
/*
if ($cache) {
return;
}
*/

worked for me!

stella’s picture

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

stella’s picture

Status: Active » Needs review
FileSize
346 bytes

This patch seems to fix it for me. No guarantees, will see how it goes.

mrfelton’s picture

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

tekante’s picture

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

tekante’s picture

FileSize
1.18 KB

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

tekante’s picture

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

YK85’s picture

subscribing

phreestilr’s picture

subscribing

pcave’s picture

FileSize
406 bytes

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

jaybee1001’s picture

Hi,

Is this patch addressing core page-caching, or will it also address the issue when using Boost?

Thanks

JB

tekante’s picture

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

AlexisWilke’s picture

Hi 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

MadtownLems’s picture

This 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!

MadtownLems’s picture

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

AlexisWilke’s picture

MadtownLems,

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

MadtownLems’s picture

AlexisWilke,

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!

AlexisWilke’s picture

I 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

MadtownLems’s picture

This 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. :|

AlexisWilke’s picture

I 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?

jmuessig’s picture

subscribe

jmuessig’s picture

I tried patch #10 and got into a redirect loop when navigating the admin menus - fwiw.

levelos’s picture

Priority: Normal » Critical

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

phil_esq.’s picture

subscribe

tekante’s picture

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

AlexisWilke’s picture

tekante,

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

johnthomas00’s picture

subscribe

neilnz’s picture

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

Fortyhands’s picture

Hello!

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.

DannyPfeiffer’s picture

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

kenwest’s picture

I applied the patch supplied by @pcave at #10 and this solved my redirect problem:

Set up

  • page 'nonsecure' should be served with http and 'secure' with https
  • both pages have menu items

To reproduce problem

  • starting on home page in http and clicking on 'nonsecure' - OK - 'nonsecure' served from cache with http
  • then clicking on 'secure' - OK - 'secure' served in https (hook_boot called once in http then a second time in https; securepages_redirect set the first time, stops loop second time, and is unset correctly in hook_init)
  • then clicking on 'nonsecure' - PROBLEM - 'nonsecure' served in http, except securepages_redirect is left set
  • then clicking on 'secure' - PROBLEM - 'secure' served in http, since securepages_redirect was set and the redirect could not occur

@pcave's patch saves the day by unsetting securepages_redirect once it's been finished with

Jaapx’s picture

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

Robert Castelo’s picture

FileSize
830 bytes

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

bcmiller0’s picture

subscribe

sreynen’s picture

I 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?

AlexisWilke’s picture

sreynen,

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

sreynen’s picture

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

Robert Castelo’s picture

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

sreynen’s picture

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

Robert Castelo’s picture

@sreynen have you tried my patch #33 ?

Fixed the problem for me in all cases.

kenwest’s picture

Installing version 6.x-1.9 resolves the issue I was having (#31)

jmuessig’s picture

Patch #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?

jmuessig’s picture

OK, 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

dbassendine’s picture

Version: 6.x-1.8 » 6.x-1.9

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

dbassendine’s picture

FileSize
1.51 KB

Here'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

drupalsteve’s picture

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

chicodasilva’s picture

I have the same problem. Is there any update on this issues?

Reevescorp’s picture

Upgraded 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

AlexisWilke’s picture

Reevescorp,

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

Reevescorp’s picture

My 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/*

Reevescorp’s picture

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

AlexisWilke’s picture

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

Reevescorp’s picture

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

AlexisWilke’s picture

From 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

Reevescorp’s picture

You're right!

Doing a netbeans xdebug and moving a watch around in securepages.module -

Its dying here

  if (function_exists('module_invoke_all')) {
    foreach (module_implements('exit') as $module) {
      if ($module != 'devel') {
        module_invoke($module, 'exit');
      }
    }
  }

Getting tired... will walk through some more of it after a few hours of sleep.

Reevescorp’s picture

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

  $file = fopen("c:\\temp\\modules.txt", "w");
  if (function_exists('module_invoke_all')) {
    foreach (module_implements('exit') as $module) {
      if ($module != 'devel') {
        fwrite($file,$module . " ");
        module_invoke($module, 'exit');
      }
    }
  }
  else {
    bootstrap_invoke_all('exit');
  }
  fclose($file);

Reevescorp’s picture

Update on the statistics bug:

Traced the problem down to this section of the statistics module

if ((variable_get('statistics_enable_access_log', 0)) && (module_invoke('throttle', 'status') == 0)) {
    // Log this page access.
    db_query("INSERT INTO {accesslog} (title, path, url, hostname, uid, sid, timer, timestamp) values('%s', '%s', '%s', '%s', %d, '%s', %d, %d)", strip_tags(drupal_get_title()), $_GET['q'], referer_uri(), ip_address(), $user->uid, session_id(), timer_read('page'), time());     
    
 }

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.

if ((variable_get('statistics_enable_access_log', 0)) && (module_invoke('throttle', 'status') == 0)) {
    // Log this page access.
    //db_query("INSERT INTO {accesslog} (title, path, url, hostname, uid, sid, timer, timestamp) values('%s', '%s', '%s', '%s', %d, '%s', %d, %d)", strip_tags(drupal_get_title()), $_GET['q'], referer_uri(), ip_address(), $user->uid, session_id(), timer_read('page'), time());     
    db_query("INSERT INTO {accesslog} (title, path, url, hostname, uid, sid, timer, timestamp) values('%s', '%s', '%s', '%s', %d, '%s', %d, %d)", 'test', $_GET['q'], referer_uri(), ip_address(), $user->uid, session_id(), timer_read('page'), time());     
  }

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.

AlexisWilke’s picture

The 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

Reevescorp’s picture

Well,

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.

 $file = fopen("c:\\temp\\statistics.txt", "w");
  if ((variable_get('statistics_enable_access_log', 0)) && (module_invoke('throttle', 'status') == 0)) {
    // Log this page access.
    //db_query("INSERT INTO {accesslog} (title, path, url, hostname, uid, sid, timer, timestamp) values('%s', '%s', '%s', '%s', %d, '%s', %d, %d)", strip_tags(drupal_get_title()), $_GET['q'], referer_uri(), ip_address(), $user->uid, session_id(), timer_read('page'), time());     
      fwrite($file,strip_tags(drupal_get_title()) . " ");
      fwrite($file,drupal_get_title() . "\n ");
    db_query("INSERT INTO {accesslog} (title, path, url, hostname, uid, sid, timer, timestamp) values('%s', '%s', '%s', '%s', %d, '%s', %d, %d)", 'test', $_GET['q'], referer_uri(), ip_address(), $user->uid, session_id(), timer_read('page'), time());     
  }
  fclose($file);
Reevescorp’s picture

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

jrust’s picture

Subscribe. Patch from #33 worked for me.

drupalsteve’s picture

I manually applied the patch from #33 to version 6.x-1.9; it seems to be working so far.

adr_p’s picture

FileSize
1.96 KB

Here's my solution for described problems. While I was testing the module behavior I found problems like:

  1. The same address being redirected back and forth to http and https (e.g. user/[uid]/edit); depending which protocol we type in address bar we're redirected to the other [see http://drupal.org/node/846962].
  2. Pages incorrectly served via httpd.
  3. Pages incorrectly served via http.

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.

grendzy’s picture

I'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.

adr_p’s picture

I 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?

AlexisWilke’s picture

Isn'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

drupalsteve’s picture

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

mstrelan’s picture

subscribe

mrfelton’s picture

Patch updated to work with latest git code, and to apply cleanly through drush make.

grendzy’s picture

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

yub_yub’s picture

subscribe

msumme’s picture

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

msumme’s picture

Also, 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.

function securepages_should_be_secure($path) {
  /**
   * Check to see if the page matches the current settings
   */
  $secure = variable_get('securepages_secure', 1);
  $pages = variable_get('securepages_pages', "node/add*\nnode/*/edit\nuser/*\nadmin*");
  $ignore = variable_get('securepages_ignore', "*/autocomplete/*\n*/ajax/*");

  if ($ignore) {
    $regexp = '/^('. preg_replace(array('/(\r\n?|\n)/', '/\\\\\*/', '/(^|\|)\\\\<front\\\\>($|\|)/'), array('|', '.*', '\1'. preg_quote(variable_get('site_frontpage', 'node'), '/') .'\2'), preg_quote($ignore, '/')) .')$/';
    if (preg_match($regexp, $path)) {
      return securepages_is_secure() ? true : false;
    }
  }
  if ($pages) {
    $regexp = '/^('. preg_replace(array('/(\r\n?|\n)/', '/\\\\\*/', '/(^|\|)\\\\<front\\\\>($|\|)/'), array('|', '.*', '\1'. preg_quote(variable_get('site_frontpage', 'node'), '/') .'\2'), preg_quote($pages, '/')) .')$/';
    $result = preg_match($regexp, $path);
    if (function_exists('drupal_get_path_alias')) {
      $path_alias = drupal_get_path_alias($path);
      $result |= preg_match($regexp, $path_alias);
    }
    return !($secure xor $result) ? true : false;
  }
  else {
    return (bool)(!$secure);
  }

}

Then redirect could be simpler given the above change.

function redirect() {
	if($_POST){
            // If something has been posted to here then ignore the rules.
	} else {
		$path = isset($_GET['q']) ? $_GET['q'] : '';
		$should_be_secure = (bool)securepages_should_be_secure($path);
		if($should_be_secure !== (bool)securepages_is_secure() ) {
                     //simply switch to what it should be if it isn't that yet.
			securepages_switch_ssl($should_be_secure);
			return;
		}	
	}
	if (securepages_is_secure()) {
		global $base_url;
    	$base_url = securepages_baseurl(true);
   }
}



I replaced securepages_goto with securepages_switch_ssl($on) :

It only does one thing... changes ssl to non-ssl or vice-versa

function securepages_switch_ssl($on) {
	
	$base = securepages_baseurl($on);
//	simply switch to/from ssl w/ no other changes
	$url = $base . $_SERVER['REQUEST_URI'];

	if(function_exists('module_invoke_all')) {
		foreach(module_implements('exit') as $module) {
			if($module != 'devel') {
				module_invoke($module, 'exit');
			}
		}
	}
	else {
		bootstrap_invoke_all('exit');
	}
	header('Location: '.$url);
	
	exit();
} 

With this approach, you don't really need to set a session variable at all, because it works in a stateless fashion.

shaisachs’s picture

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

shaisachs’s picture

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

seanr’s picture

Looks good, but a minor code formatting issue here:

-  header('Location: '. $url);
-
+  
+  header('Location: '.$url);
+  

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.

shaisachs’s picture

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

  1. Visit http://somesite.com/aaabbbccc
  2. hook_init fires, your module detects that aaabbbccc matches ([abc]+)+, and redirects to https
  3. Visit https://somesite.com/aaabbbccc
  4. hook_boot fires, but your module is not loaded; consequently aaabbbccc is considered non-secure, and redirects to http
  5. Visit http://somesite.com/aaabbbccc, and the loop continues forever

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.

grendzy’s picture

Status: Needs review » Needs work

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

grendzy’s picture

Please try the 6.x-2.x-dev release.

rickbydesign’s picture

How do I apply your patch?

grendzy’s picture

rickbydesign: 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.

ShaunDychko’s picture

Patch #69 works wonderfully so far. Other patches (such as #6, #10) didn't redirect after repeatedly entering http://example.com/user.

wiherek’s picture

Version: 6.x-1.9 » 7.x-1.x-dev

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

grendzy’s picture

Version: 7.x-1.x-dev » 6.x-2.x-dev
Status: Needs work » Fixed

I'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.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

tomswiggers’s picture

Status: Closed (fixed) » Active
FileSize
754 bytes

Hi,

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

grendzy’s picture

Status: Active » Closed (fixed)

Hi 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!

tomswiggers’s picture

Hi,

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