Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: tangent Updated by: Anonymous Status: patch Isn't the 'session.save_path' directive unnecessary and misleading? Because Drupal uses session.save_handler = user (our sessions are stored in the database), that option won't do anything. AFAIK, save_path only matters when using save_handler = files. Also, it appears that there are functions we might be able to use instead of ini_set (for shared servers that disable ini_set): session_cache_expire, session_cache_limiter, session_set_cookie_params, session_set_save_handler. session_cache_expire has a minimum version of 4.2.0, so that might be a problem for users of Drupal's minimum PHP version (4.1.0). Perhaps the ini_set would be better for that particular setting. I think this would be a worthwhile patch... and the more hardcoded PHP settings we remove from .htaccess, the better. Many shared hosts are using CGI-based PHP (rather than mod_php) and configure their settings in a separate place (not .htaccess), which forces end-users to manually copy over the settings they need. Using a runtime-based approach like this would eliminate that problem. Anonymous Previous comments: ------------------------------------------------------------------------ February 14, 2005 - 07:51 : tangent Attachment: http://drupal.org/files/issues/settings-session.patch (1.71 KB) As discussed in this issue [1], it would be desirable to move the session settings into /sites/default/settings.php so that subsites can have better control over them. One of the advantages of the site specific settings.php file is that it will never get overwritten during upgrades and having these settings here should prove to be more friendly. I have created a patch which moves most of the PHP session settings from .htaccess to /sites/default/settings.php with the exception of "session.auto_start" because it must not, as far as I know, be modified. I have also added 2 additional commented settings which I suspect are often needed as they were in my case. [1] http://drupal.org//node/2974 ------------------------------------------------------------------------ February 14, 2005 - 09:42 : Goba Please, if this gets committed, leave a comment line in the .htaccess file at least, indicating that other session related settings are located in settings.php, so those in the know of current Drupal behaviour will find their way around. ------------------------------------------------------------------------ February 14, 2005 - 14:56 : kbahey This patch is very important. It does what I wanted to do in the other issue pointed to: make Drupal as much as possible independant of the underlying PHP configuration. Can you please also add ini_set calls for these values to make sure PHPSESSID does not end up in the URL: php_flag session.use_trans_sid off php_flag session.use_cookies on php_flag session.use_only_cookies on Also, remove the line +ini_set('session.cookie_lifetime', 2000000); Since the other patch takes care of this value. Or should we make everything consistent and use just one way to do it (settings.php instead of session.inc)? ------------------------------------------------------------------------ February 14, 2005 - 18:04 : tangent I added a line for "session.use_only_cookies" but did not for the others because the values you specified are the defaults. The only reason they should need to be changed is if they are defined elsewhere. I had to disable "session.use_trans_sid" on my installation so perhaps my webhost enables this in php.ini or elsewhere but I'm not sure if the setting should be included. Anyone else have thoughts? I see no reason to change the cookie lifetime value from what it has always been. The site admin can change the value in settings.php if they want a different value. I suppose this eliminates the need to set the value elsewhere. ------------------------------------------------------------------------ February 14, 2005 - 20:14 : kbahey I think that your way of doing the session is better, because, as I said in the other thread, it transcends the cookie/session discussion, and makes Drupal independant of how PHP defaults are set (global php.ini, local php.ini, .htaccess, ...etc.), and regardless of how the ISP setup their PHP defaults. Regardless of whether what the defaults for parameters are, we need to set them, and be sure that for Drupal, they are set this way. So, go ahead and add everything there, with a comment where appropriate (e.g. for cookie_lifetime, say "This is the number of seconds to keep a user logged in without asking them for a password.", ...etc.) My only concern is this http://us4.php.net/manual/en/function.ini-set.php (see comment from 14 Sept 2004, and specially this http://us4.php.net/manual/en/ini.php#ini.list). Different PHP versions allow/disallow ini_set for various values. Supporting the various version idiosyncracies in the code with conditionals is a nightmare. ------------------------------------------------------------------------ February 14, 2005 - 21:06 : tangent Attachment: http://drupal.org/files/issues/settings-session2.patch (2.27 KB) The URL session management settings not mandatory for Drupal to function. Therefore they are optional. That is why I have included them commented out. Further, the comment that you referenced about the changeability of session.use_trans_sid seems to be misleading. The official documentation [2] states the following version compatibility. PHP_INI_ALL in PHP <= 4.2.3. PHP_INI_PERDIR in PHP <= 4-cvs. Available since PHP 4.0.3. I also don't think documenting each setting (like cookie lifetime) is necessary because I've added a reference in this patch to the settings documentation. If the admin is not familiar with all the settings they can go read up. [2] http://www.php.net/manual/en/ini.php#ini.list ------------------------------------------------------------------------ February 15, 2005 - 07:51 : tangent Attachment: http://drupal.org/files/issues/settings-session3.patch (7.13 KB) Here's an updated patch which adds a comment to .htaccess noting the relocation of session settings. I also added the settings.php discovery rules which are documented in bootstrap.inc to the file doxygen block since this info will likely be of more use to the admin here. Finally, it now seems silly to maintain a separate issue for updating the comments from # to c-style (unless this patch is refused) so I've included that change here and will close issue 17288 [3] until this issue is resolved positively. [3] http://drupal.org//node/17288 ------------------------------------------------------------------------ February 15, 2005 - 08:07 : tangent Attachment: http://drupal.org/files/issues/settings-session4.patch (7.1 KB) This patch changes the configuration rule instructions to use "sites/default" instead of a variable since that value is hardcoded and would be more confusing to end users. The instructions are also slightly reworded. Sorry for the noise. Hopefully this is the final patch. ------------------------------------------------------------------------ February 15, 2005 - 09:35 : tangent Attachment: http://drupal.org/files/issues/settings-session5.patch (6.99 KB) This patch removes a setting which crept in and probably shouldn't be there. ------------------------------------------------------------------------ February 16, 2005 - 21:55 : Dries I guess this is a good thing. Unless someone objects, I'll commit this to HEAD. -- View: http://drupal.org/node/17303 Edit: http://drupal.org/project/comments/add/17303