Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: tangent Updated by: kbahey Status: patch 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)? kbahey Previous comments: ------------------------------------------------------------------------ February 14, 2005 - 02: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 - 04: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. -- View: http://drupal.org/node/17303 Edit: http://drupal.org/project/comments/add/17303

Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: tangent Updated by: kbahey Status: patch 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. kbahey Previous comments: ------------------------------------------------------------------------ February 14, 2005 - 02: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 - 04: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 - 09: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 - 13: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. -- View: http://drupal.org/node/17303 Edit: http://drupal.org/project/comments/add/17303

Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: tangent Updated by: tangent Status: patch 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 [1] until this issue is resolved positively. [1] http://drupal.org//node/17288 tangent Previous comments: ------------------------------------------------------------------------ February 14, 2005 - 02:51 : tangent Attachment: http://drupal.org/files/issues/settings-session.patch (1.71 KB) As discussed in this issue [2], 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. [2] http://drupal.org//node/2974 ------------------------------------------------------------------------ February 14, 2005 - 04: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 - 09: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 - 13: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 - 15: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 - 16: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 [3] 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. [3] http://www.php.net/manual/en/ini.php#ini.list -- View: http://drupal.org/node/17303 Edit: http://drupal.org/project/comments/add/17303

Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: tangent Updated by: tangent Status: patch 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. tangent Previous comments: ------------------------------------------------------------------------ February 14, 2005 - 02: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 - 04: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 - 09: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 - 13: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 - 15: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 - 16: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 - 02: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 - 03: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. -- View: http://drupal.org/node/17303 Edit: http://drupal.org/project/comments/add/17303

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

Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: tangent Updated by: tangent Status: patch Will do. I was planning to look into addressing the concerns of the previous poster with respect to the session functions first. I'll submit an update as soon as I can. tangent Previous comments: ------------------------------------------------------------------------ February 14, 2005 - 02: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 - 04: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 - 09: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 - 13: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 - 15: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 - 16: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 - 02: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 - 03: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 - 04: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 - 16:55 : Dries I guess this is a good thing. Unless someone objects, I'll commit this to HEAD. ------------------------------------------------------------------------ February 17, 2005 - 14:40 : Anonymous 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. ------------------------------------------------------------------------ February 17, 2005 - 18:07 : Dries tangent: patch complains about your patch being malformed. I tried fixing it but to no avail. Could you try resubmitting your patch? -- View: http://drupal.org/node/17303 Edit: http://drupal.org/project/comments/add/17303

Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: tangent Updated by: kbahey Status: patch +1 for this patch as it is. The setting use_only_cookies is a must. Some time ago my host changed to PHP SuExec (not mod_php) and I started seeing ?PHPSESSID=blah in the URLs, and had to put it manually in .htaccess. This way we are sure that Drupal handles this explicitly in a way that makes do things our way not how the defaults are set at the hosting server. Good work! kbahey Previous comments: ------------------------------------------------------------------------ February 14, 2005 - 02: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 - 04: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 - 09: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 - 13: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 - 15: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 - 16: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 - 02: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 - 03: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 - 04: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 - 16:55 : Dries I guess this is a good thing. Unless someone objects, I'll commit this to HEAD. ------------------------------------------------------------------------ February 17, 2005 - 14:40 : Anonymous 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. ------------------------------------------------------------------------ February 17, 2005 - 18:07 : Dries tangent: patch complains about your patch being malformed. I tried fixing it but to no avail. Could you try resubmitting your patch? ------------------------------------------------------------------------ February 17, 2005 - 18:14 : tangent Will do. I was planning to look into addressing the concerns of the previous poster with respect to the session functions first. I'll submit an update as soon as I can. ------------------------------------------------------------------------ February 18, 2005 - 01:23 : tangent Attachment: http://drupal.org/files/issues/settings-session6.patch (6.8 KB) Here's an updated patch. 1. I removed the unnecessary "session.save_handler" setting. 2. I uncommented the "session.use_only_cookies" and "session.use_trans_sid" settings to force cookie based (as opposed to url based) sessions by default. This seems like the most recommended method but perhaps there is a reason it was not in there before? 3. I did NOT replace the ini_set function with the setting specific functions because I'm uncertain what the implications are and I cannot find any documentation that corroborates the anonymous posters arguments. The ini_set function works on my shared server environment so if the anonymous poster (please log in next time so we know who is commenting) cannot clarify his/her concerns then this patch may be final. -- View: http://drupal.org/node/17303 Edit: http://drupal.org/project/comments/add/17303

Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: tangent Updated by: tangent Status: patch Not sure how that got added as enabled but good catch. tangent Previous comments: ------------------------------------------------------------------------ February 14, 2005 - 02: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 - 04: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 - 09: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 - 13: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 - 15: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 - 16: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 - 02: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 - 03: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 - 04: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 - 16:55 : Dries I guess this is a good thing. Unless someone objects, I'll commit this to HEAD. ------------------------------------------------------------------------ February 17, 2005 - 14:40 : Anonymous 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. ------------------------------------------------------------------------ February 17, 2005 - 18:07 : Dries tangent: patch complains about your patch being malformed. I tried fixing it but to no avail. Could you try resubmitting your patch? ------------------------------------------------------------------------ February 17, 2005 - 18:14 : tangent Will do. I was planning to look into addressing the concerns of the previous poster with respect to the session functions first. I'll submit an update as soon as I can. ------------------------------------------------------------------------ February 18, 2005 - 01:23 : tangent Attachment: http://drupal.org/files/issues/settings-session6.patch (6.8 KB) Here's an updated patch. 1. I removed the unnecessary "session.save_handler" setting. 2. I uncommented the "session.use_only_cookies" and "session.use_trans_sid" settings to force cookie based (as opposed to url based) sessions by default. This seems like the most recommended method but perhaps there is a reason it was not in there before? 3. I did NOT replace the ini_set function with the setting specific functions because I'm uncertain what the implications are and I cannot find any documentation that corroborates the anonymous posters arguments. The ini_set function works on my shared server environment so if the anonymous poster (please log in next time so we know who is commenting) cannot clarify his/her concerns then this patch may be final. ------------------------------------------------------------------------ February 18, 2005 - 09:15 : kbahey +1 for this patch as it is. The setting use_only_cookies is a must. Some time ago my host changed to PHP SuExec (not mod_php) and I started seeing ?PHPSESSID=blah in the URLs, and had to put it manually in .htaccess. This way we are sure that Drupal handles this explicitly in a way that makes do things our way not how the defaults are set at the hosting server. Good work! ------------------------------------------------------------------------ February 18, 2005 - 13:45 : Dries Great patch. Committed to HEAD. Thanks. ------------------------------------------------------------------------ February 19, 2005 - 07:15 : Goba Attachment: http://drupal.org/files/issues/Drupal.fix-trans-sid.patch (1.04 KB) Turning on /use only cookies/, but also turning on /trans sid/ is not logical, since only cookies will be observed for session id information, but session IDs will still be added to the URLs on the page. The attached patch makes trans sid 0, and aligns the setting values. -- View: http://drupal.org/node/17303 Edit: http://drupal.org/project/comments/add/17303