JSession enhancements

[ghost]

Fixes for memcache session store.
Fix[change] for none session store
Enhancement for JSession to add external session storage definitions
Unit tests for enhancements

Needs

Regression test for memcache and none fixes

[realityking]

Actually the sever configuration may have changed this to something else. Maybe we should provide JSessionStorageFiles and rename this to JSessionStorageDefault?

[ghost]

The problem with the current implementation is that IF the server configuration has changed it to something else, then this action breaks the server.

For example, assume that the save_handler is set to memcache:
Then session.save_path would be: sess1:11211

If you change session.save_handler to files then sessions break because the file path sess1:11211 does not exist.

If you set the save_handler to files, then you must ALSO set the save_path to something reasonable. To do one and not the other is broken.

I choose to go the route of not doing either since there is not a good way to determine a reasonable file path.

[realityking]

That's a really good point. I wonder if we can find a solution for this.

[ghost]

I think just removing the offending line will solve the issue. It's my guess that the only reason Joomla has this line in it is because in the ancient past there was a line somewhere in the primary JSession class which called ini_set('session.save_handler', 'somevalue') in order to redirect the session saves to itself - so as a fallback SessionNone was created which would set it back to the 'default' value - where default was assumed to be file. When we went to using session_set_save_handler, this one line of code was not removed due to an oversight.

At least, I can think of no reason WHY we would want to set session.save_handler to files today. If it was originally files, it remains files and the path is correct. If it was something else, then the save_path is likely not pointing to a directory so setting it to files will break session handling.

In either case, if SessionNone is a fallback for no other working session method, there is no need to override the default ini variable - either PHP is configured correctly and sessions will work, or it is not.

I also suspect it's an oversight mainly because it only affects the 1% of people who are using a session handling extension for PECL such as APC or Memcache and have changed their configuration. So it just never gets fixed because the number of people it affects is miniscule.

[realityking]

The Memcache(d) changes look fine. We should look into sharing some code between the Memcache and Memcached implementations.

I'm not a big fan of the "external session storage definitions" for now I think our API using a the abstract JSessionStorage is fine. Eventually (PHP 5.4ish time) I think this should be refactored towards a DI pattern where all we need is an (instantiated) SessionHandlerInterface. Making sure the handler is supported would then fall onto the application.

[ghost]

I have to close this request in the next day or so and open 2 new ones, one for memcache and one for none. Thanks to Ellin, I now know how to force a different session save handler without having to go through a JSession discovery process so that code is not needed.

[ghost]

Closing this commit, moved memcache fixes to 1836 #1836

[ghost]

moved sessionNone changes to their own pull request, 1839 #1839