Destroying Sessions

Hi Glen,

Great bug report and research, thanks! :)

We've been battling with PHP sessions for a while.  The issues are the various web hosts servers delete them too soon, too late, or not at all.  Sometimes multiple websites on the same server use the same /tmp/ folder and erase each others session files when they shouldn't, and in this latest case that you found, sometimes we have developers using other 3rd party apps that use sessions, so we've had to stop destroying the entire session or we risk erasing data used by another web app on the same server.

In v2.51 we addressed some of the previous issues by bypass session altogether for login data and just using a cookie.

For the issues you've mentioned, those are bugs with the current version so I've applied the following patches.  Can you try them and let me know if they work for you?

In /lib/login_functions.php in the function user_logoff() I've added the code in red (just search for user_logoff):

function user_logoff($redirectUrl = '') {
  loginCookie_remove();                   // erase login cookie
  $GLOBALS['CURRENT_USER'] = false;       // clear user global

  // 2.52 - clear saved CMS session data
  if (isset($_SESSION['lastRequest'])) { unset($_SESSION['lastRequest']); }

  // redirect/refresh page



This will ensure that when users logoff their previous view states (stored in $_SESSION['lastRequest']) will be cleared including sort field, page number, keyword searches, etc.  Had this been in place you would have been able to logoff and login again to fix the issue.

Next, in /lib/menus/default/list_functions.php I've added the follow code (in red) - search for this text: _REQUEST from _SESSION 

  // Load last _REQUEST from _SESSION (current _REQUEST values override old ones)
  if (@$_SESSION['lastRequest'][$tableName] && !$isRelatedRecords && !@$_REQUEST['_ignoreSavedSearch']) {
    $sortByField        = @$_SESSION['lastRequest'][$tableName]['sortBy'];
    $invalidSortByField = $sortByField && !@$schema[$sortByField];
    if ($invalidSortByField) { unset($_SESSION['lastRequest'][$tableName]['sortBy']); } // v2.52 remove invalid sort by fields
    $_REQUEST += $_SESSION['lastRequest'][$tableName];
  };

Which will ignore previously saved invalid fieldnames so you won't be that error if the sequence of events re-occurs.

Can you give both of those a try and let me know if it resolves the issue for you? 

Thanks!

Hi Dave.

Dealing with the second item first:

  // Load last _REQUEST from _SESSION (current _REQUEST values override old ones)

Your code update does solve the problem.

As for the first issue:

function user_logoff($redirectUrl = '') {...

I am aware of this function and it was here that I discovered you had removed the session destruction.

I have also read in other recent threads you are having problems with zero byte sessions.

// 2.52 - clear saved CMS session data
  if (isset($_SESSION['lastRequest'])) { unset($_SESSION['lastRequest']); }

Your update removes the "'lastRequest'" session data but if no other session data exists it leaves behind a zero byte session which users seem to be having issues with.

Here are couple of suggestions that would help to get rid of the dreaded zero byte sessions.

In addition to your update continue to destroy the session if it is empty :

function user_logoff($redirectUrl = '') {
  loginCookie_remove();                   // erase login cookie
  $GLOBALS['CURRENT_USER'] = false;       // clear user global

  // 2.52 - clear saved CMS session data
  if (isset($_SESSION['lastRequest'])) { 
    unset($_SESSION['lastRequest']); 
    // If the session is empty after removing the CMSB 'lastRequest', then destroy it since there is no apparent value.
    if(!count($_SESSION)) session_destroy();
  }

  // redirect/refresh page

The other time a zero based session is created is when the login page is loaded.

So if you are not being redirected when you log off, the login screen appears but a zero byte session would also have been created earlier in the procedure. 

You could destroy the session at this point as well in lib/admin_functions.php

// if no logged in user
  if (!$CURRENT_USER) {

    // perform login screen maintenance actions - useful place to run common operations
    if (!$action) {
      createMissingSchemaTablesAndFields(); // create/update missing schemas, etc

      // show helpful messages
      if (!mysql_count('accounts')) { alert(t("There are no user accounts in the database.")); }
    }

    // show login screen if user not logged in
    if(!count($_SESSION)) session_destroy();  // destroy the newly created empty session
    showInterface('login.php', false);
    exit;
  }

If a user/visitor arrives at the login page for what ever reason a session will not exist until authenticated.

I've tested this in ver. 2.51 and my suggestions seem to work without any issues. 

We've been battling with PHP sessions for a while. The issues are the various web hosts servers delete them too soon, too late, or not at all. Sometimes multiple websites on the same server use the same /tmp/ folder and erase each others session files when they shouldn't, and in this latest case that you found, sometimes we have developers using other 3rd party apps that use sessions, so we've had to stop destroying the entire session or we risk erasing data used by another web app on the same server.

The difficulties with other "apps", "plugins" and "servers" makes me want to ask the question:  Are you trying to solve a problem that is "not" a CMSB problem?

The other thing I've noticed is a departure from the separation of CMSB and Plugins. 

As a user of CMSB it's my expectation that CMSB will be rock solid and stable without any "active" plugins. The current session issue makes we wonder if you are not adhering to any rules with respect to seperation of CMSB from other influences.  

Changes to your stable back end to solve a 3rd party issue I think is fundamentally wrong which, as is in the case of the current issue, has broken the back end.

For example, I've come across code in CMSB to solve/fix issues with your "Website Membership" plugin. Since the code relates to a specific plugin issue it should be in the plugin and accessed via a "Hook".

You as the developer of CMSB can add "hooks" where ever you like to address specific plugin issues for plugins you create.  Thus new versions of CMSB should only consist of new features and fixes in addition to new hooks to your already stable release.  Hooks theoretically will never break the back end.

Does IA have a design philosophy in this regard such as is the case in the philosophy of "separation of church and state" where the state is CMSB and the church is things like servers, apps and plugins?

This philosophy will ensure that CMSB can continue to be improved upon in its all ready feature rich state and remain stable. Specific issues as they relate to plugins can be isolated and worked on with breaking CMSB for those aren't using any particular plugin.

I think CMSB is fabulaous and the ability to perform customization via plugins is out standing. 

I am interested in knowing if IA has an official position on the relationship of CMSB to Plugins that I have talked about here. 

Hi Glen,

Thanks for your feedback and suggestions.  

We've actually got some code in 2.52 beta (not yet released) to remove 0-byte files on every page execution at the last moment.  If you'd like to give it a try, here's a link: 
http://www.interactivetools.com/forum/forum-posts.php?postNum=2229399#post2229399

If you're on 2.51 you should be able to just set $useThisBetaFeature = true; in /lib/init.php and new 0-byte session files won't be created by CMSB (but still will be by other domains on the same server or other apps, etc).

And regarding your questions about our philosophy and intent behind recent changes, here's some more info on that:

Are you trying to solve a problem that is "not" a CMSB problem?

In regards to the zero-byte php session files.  Yes,  it's default PHP behavior but they build up on our servers and fill up my backup files.  And I actually don't agree with the PHP developers rational for leaving them there, but get why they might (probably to reduce disk overhead).  It's part of a larger goal of making CMSB "play-nice" with the server it's on, and making it easier for developers by preventing possible problems from emerging, or making them aware of configuration issues that are already be present.  One example, if you set a custom PHP session folder and get tens of thousands of zero byte files, your FTP client might stall or crash on that folder, which is frustrating.

The other thing I've noticed is a departure from the separation of CMSB and Plugins. 

The goal is to keep these two things separate.  But we try to be pragmatic in our approach to solving problems.  So I know I might be rewriting a block of code later in the year, or can save a significant amount of time, or make something backwards compatible so users can upgrade easier, I may do that.   We're only now able to remove PHP4 compatibility code, at some future point we'd like to break backwards compatibility and get rid of a lot of workarounds, but as of now we're almost 100% seamlessly backward compatible to CMSB 1.00.  In the case of Website Membership I think the decision was either hard-code some backwards compatibility code or require everyone who upgrades CMSB or Website Membership to update all of their website membership pages so they keep working.

Changes to your stable back end to solve a 3rd party issue I think is fundamentally wrong which, as is in the case of the current issue, has broken the back end.

We do need to treat $_SESSION as a shared resource, at least within the domain, but I agree with your sentiment.  Sometimes we'll break things while trying to fix other things, and no one likes that so we try not to do it.  We're especially motivated not to do it too because everyone let's us know when we do!  The session_destroy() was actually removed because it was breaking other code on multiple users sites.  Finding a balance is sometimes tricky. 

Does IA have a design philosophy in this regard such as is the case in the philosophy of "separation of church and state" where the state is CMSB and the church is things like servers, apps and plugins?

Our first priority is always focusing on what's going to best serve our current and future user-base.   At the end of the day, CMSB needs to be fast and easy to build websites with.  And ideally over time it will get faster and easier, and allow people to do more and more things. Most of the users aren't going to care about design ideologies  they just want it to work and they don't really care much about where problems originate, and understandably so, they have a job to do.  So we're trying to do our best to support them in that.

Back in the day, we'd spend hours on the phone trying to help "broken" hosts fix their setups.  Nowadays if we can make a change to the software that reduces the amount of support calls we get, or makes a common configuration error obvious to a sysadmin, or works around a "bug/feature" with a large web host who won't change it, we generally just do it.   This saves time for both our users and us and let's us focus on building new features or improving other aspects of the software.

Some notable examples of this philosophy:

• /lib/init.php - _init_fixPreviewDnsDomains() - godaddy.com offers "preview" domains (*.previewdns.com) for customers whose DNS hasn't migrated yet.  To remind them not to use the preview domains for live sites they inject javascript into every page to launch a popup.  But their injected code also includes some javascript libraries and prevents jQuery from working breaking all the CMSB pages.  This workaround prevents that.
• /lib/menus/default/uploadForm_funcitons.php - _showLinks() - if networksolutions.com FTP sees this string ".addcslashes" they silently truncate the file, causing CMSB not to work at all.  I assume it's a virus scanner on their end getting a false positive or something, but it totally breaks CMSB.  We have copious comments and extra whitespace to bypass whatever filter on their end is causing that problem.
• /lib/admin_functions.js - IPowerWeb uses a Nginx / Varnish reverse proxy that sends a Content-Length: 0 header and 1 byte of content (the number zero "0") if you don't output anything, which we need to do sometimes for jquery ajax calls (return a blank page).  They swore up and down this was valid until I referenced the RFC that said it wasn't, then they just simply said they didn't want to change it because their former head technician did it and they were worried it might break something else if they did. :)

And then, as you've pointed out we've added some workaround for our own plugins to "fix" issues.  Generally I don't like doing any of that and strive for clean & simple code, but sometimes makes the most sense in the moment.  On plugins I do agree that they should be separate and the hooks should be generic.   So if we didn't do that in one place or another it's probably because after a lot of thought we couldn't think of a better alternative given the constraints of the situation.

There's actually an interesting article on Joel on Software about ugly vs clean code: 
http://www.joelonsoftware.com/articles/fog0000000069.html

So in short, if the "hacks" are beneficial enough, we might put them in there, but it is something we try and avoid and we make those decisions carefully on a case by case basis..  Having a clean separation between the CMS and the plugins is actually better for us internally as well, because it creates a solid plugin ecosystem that can be built on.  Which I think is mostly what we have now.

Hope that helps, cheers! 

Thanks for sharing Dave.

Actually, your response was so thorough I thought maybe you were feeling a need to be defensive.  That was not my intention and hope you did not interpret my questions or comments in that way.

I turned on the "useThisBetaFeature" and it "did" removed the zero byte sessions.

To you and your team, thanks for the great product.