VERY obscure bug just waiting to bite someone....

James Noyes jnoyes-horde@retrogeeks.com
Thu, 21 Feb 2002 01:45:40 -1000


Hi all,
   I stumbled across a bug while exploring some bleeding-edge territory
today.

   The capsulized story is that the "@start_session()" on line 90 of
imp/login.php (it's line 90 of mine, at least) needs to removed.  Removing
it does not break any existing functionality, but leaving it in leaves
open the very strong possibility of future broken functionality.  This is
true even though "session_destroy()" is called on the line prior.  Trust
me.  :)  You can make the change and stop right here unless you want to
read on for the obnoxiously long, gory details.

The long version:
   I was experimenting with user-defined PHP Session Managers.  I was sick
of tons of sess_whatever files clogging up my /tmp and never going away,
and was wondering if MySQL-based sessioning could be done.  Yes, it CAN be
done, and I found several examples available of how to do it.  After
activating MySQL sessioning, everything seemed fine - until I attempted to
log out of Horde/IMP.  Clicking "Log out" presented me with a "Document
contained no data".  Since it didn't do that before I switched to a MySQL
Session Manager, I assumed the SM was causing the issue.
    I added a few error_log()'s to login.php and was able to determine
that when the MySQL Session Manager was active, no code after the
"@start_session()" on line 90 ever executed.  The page halted at that
point, and the user was presented with the "Document contains no data"
dialog.   A combination of things was making this happen.  First, some of
the MySQL Session Manager code I had looked at was actually DELETING a
session entry from the database when session_destroy() was called.  Be
careful if you implement a MySQL Session Manager - this is NOT correct
behavior! session_destroy() is supposed to destroy the DATA in a session,
not the session itself - see
http://www.php.net/manual/en/function.session-destroy.php for specifics.
This is the reason those files hang around in /tmp - the session still
exists, it's just empty.  It's actually YOUR responsibility to clean them
up.  Fixing this wrong behavior in the MySQL Session Manager did not
eliminate the"Document contains.." problem, however.
   There is an additional quirk to PHP Sessions, that is likely a bug (I'm
reporting this issue to the PHP folks, we'll see what they say).  Calling
a start_session() on a session that has been session_destroy()'ed while
using the built-in file-based Session Manager generates an error, but
*continues to parse the rest of the page*.  Calling the same
start_session() function after a session_destroy() while using a
user-defined PHP Session Manager not only generates an error, but stops
any further processing of the page at that point.  By trying various
third-party PHP Session Managers (ones that used DBM, MySQL, Oracle,
Sybase and shared memory for tracking) I determined that ALL user-defined
session managers apparently behave this way.   So it seems that if the
@session_start on line 90 is left as is, anyone who switches to a
user-defined Session Manager will bump into the "Document contains no
data" problem.  Now, if the PHP folks decide this is a bug and make
third-party Session Managers behave like the built-in one and continue on,
this problem will of course quietly go away.  But if instead the PHP folks
decide the third-party Session Manager behavior is right, and change the
built-in one so that it stops parsing on an error - suddenly EVERYONE will
start seeing the "Document contains no data" issue, which could get ugly. 
The easiest way to avoid both of these possible future problems is to just
get rid of the session_start() altogether, especially since it seems
totally unnecessary.  Without it, I am able to use either built-in PHP
Sessions or MySQL Sessions with no ill effects.

   And if anyone wants my WORKING MySQL Sessioning code, I'll be making it
available as soon as I get it cleaned up and packaged along with the
required sql table definition and documentation of the required php.ini
changes.  The best part is that it will work for ALL of your PHP sessions,
not just Horde/IMP!

Cheers,
James Noyes
(jnoyes-horde@retrogeeks.com)