More robust Mahara session handler

[Aaron Wells]

I've recently encountered two issues from clients who are on PHP 5.3 and have upgraded to Mahara 15.04+ and had serious bugs relating to session management. These seem to be caused by the changes we made in Bug 1532028 (commit 55a8deb8cbd2 ) to provide non-blocking sessions. Our fix relied on some undocumented

By default, PHP uses blocking sessions. That is, when you call session_start(), PHP puts a lock on the session store for your session ID. This lock is released when you call session_write_close(), session_destroy(), or your script ends. While the lock is in place, any other script attempting to access the same session ID will wait until the lock is released. This gets in the way if you want to run multiple AJAX scripts simultaneously which use the session (like the ones we use for loading blocks on a page), or to use AJAX scripts while a long-running script is running in the background (like for the CSV upload status bar).

We worked around this by opening and then closing the session multiple times during a page load. When the session is first launched, we call session_start() to load $_SESSION, then immediately call session_write_close(). We do this again when we want to write to the session, destroy the session, or regenerate the session ID.

However, there's an underlying bug in PHP that causes the session cookie to be sent out multiple times if you call session_start() and/or session_write_close() multiple times during a page load. (See https://bugs.php.net/bug.php?id=31455 and https://bugs.php.net/bug.php?id=38104 ). So, we tried to work around this by adding a function that clears out these duplicate page headers.

Anyway, since the duplicate page headers are a bug, our fix relied on undocumented behavior of PHP. It appears this behavior may not be correct in some distributions of the PHP binary, particularly in PHP 5.3.3. So, some changes to how we do things may be necessary to avoid major session problems for sites still on PHP 5.3. Additionally, an investigation of Mahara's session handler during the course of addressing these bugs, has shown me some fairly big design flaws in how we are currently handling sessions. So it will probably save us some pain down the line if we rewrite the auth/session.php code to be more robust.

[Aaron Wells (u-aaronw)]

My rough outline of how this should go:

0. Well, option 1 would be to avoid using PHP's built-in session handler system at all. (i.e. http://php.net/manual/en/book.session.php ). Since the PHP session system was *not* built to support non-blocking sessions, we may get the best results by building our own from scratch. There is nothing magic about sessions; it's just a cookie with an ID, and a data store that links serialized data to that ID. However, to avoid the pitfalls of reinventing the wheel, it would be best if we could re-use an existing session library. For instance the "Illuminate" session package, which is based on Laravel and does not use $_SESSION: https://packagist.org/packages/illuminate/session . We can't use that one specifically, though, because it needs PHP 5.5.9

Now, assuming that we *aren't* going to replace $_SESSION entirely, here's what I would recommend:

1. Replace $SESSION's "is_live()" method with two new statuses: "is_initiated()", and "is_open()". The "is_initiated()" method would indicate that we've called session_start() at least once, and hence $_SESSION is populated with data. The "is_open()" would mean that we've called session_start() and have not yet called session_write_close() or session_destroy(), which means that changes to $_SESSION will be recorded in the session store. We'd need to track these statuses ourselves through calls to the $SESSION singleton, because PHP provides no reliable way of detecting them. (PHP 5.4+ does have a session_status() method, but it is only equivalent to my "is_open()" method.)

2. User our knowledge of $SESSION->is_initiated() to make it so $SESSION->get() doesn't call session_start() or session_write_close(), thus reducing the number of duplicate session cookies generated.

3. Rewrite clear_duplicate_cookies() to use "header_remove()" instead of "header_remove('Set-Cookie')". The no-params version removes all headers (which we'd need to record first and add back), while the one-param version only removes the named type of header. It appears that in PHP 5.3 the duplicate session headers are not removed by the one-param version, but they are removed by the no-params version. Both forms remove the duplicate session headers, in later PHP versions.

4. Have $SESSION record the session's ID when the session is first initiated, or regenerated, and use that knowledge to remove any incorrect session cookies in clear_duplicate_cookies(). Currently we remove any non-unique cookies; but it seems this can cause problems when you regenerate the session ID. Both the old and new session ID may then be sent out in the response, and it's uncertain which the browser will use on its next request.

5. Stop using the session to send messages to the user on the next page request. This causes a lot of unpredictability and unreproduceability in session bugs, by changing when the session is initiated. Instead, write error messages to a database table, and fetch them using an "UPDATE... WHERE..." statement to ensure atomicity, and use a second ID cookie alongside the session cookie, to tie IDs to users. This would also reduce our need to create sessions for logged-out users.

6. Move the sessio...