Impossible to log in with default PHP settings due to cookie lifetime [#846330]

By default, PHP has its session cookie lifetime set to 0 (http://hu.php.net/manual/en/session.configuration.php#ini.session.cookie...). That means, that the cookie expires when the browser is closed. If your Drupal environment runs on such an out-of-the box PHP setup, the latest session code will set a session that (instead of expiring when the browser is closed), expires right when it it set, and Drupal will not get it back on the next request.

This is because Drupal now uses this code to set the cookie:

    $params = session_get_cookie_params();
    setcookie(session_name(), session_id(), REQUEST_TIME + $params['lifetime'], $params['path'], $params['domain'], $params['secure'], $params['httponly']);

So if the lifetime is 0 as is the default, the REQUEST_TIME is the expiration time, so the browser will expire the cookie immediately. We should consider the case when this is 0 and pass on 0 consequently. The attached patch implements that.

Comment	File	Size	Author
#8	drupal.session-lifetime-wrap-ya.8.patch	1.13 KB	sun
#8
#4	846330_session_expire_cookie.patch	1.52 KB	JacobSingh
#4
	expire-cookie.patch	1.29 KB	Gábor Hojtsy

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

JacobSingh CreditAttribution: JacobSingh commented 6 July 2010 at 14:02

Status:

Needs review

» Reviewed & tested by the community

This looks obviously broken to me and the patches seems correct.

Comment #2

Damien Tournoud CreditAttribution: Damien Tournoud commented 6 July 2010 at 15:00

Priority:

Critical

» Normal

Clearly not critical. The standard default.settings.php file has:

ini_set('session.cookie_lifetime', 2000000);

Otherwise, that's definitely an oversight that needs fixing.

Comment #3

Dries CreditAttribution: Dries commented 7 July 2010 at 01:03

Status:

Reviewed & tested by the community

» Needs work

Let's add a simple code comment, please.

Comment #4

JacobSingh CreditAttribution: JacobSingh commented 7 July 2010 at 01:23

Status:

Needs work

» Needs review

File	Size
846330_session_expire_cookie.patch	1.52 KB

How's this?

Comment #5

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 7 July 2010 at 10:42

Status:

Needs review

» Reviewed & tested by the community

Looks good to me.

Comment #6

Dries CreditAttribution: Dries commented 7 July 2010 at 13:52

Status:

Reviewed & tested by the community

» Fixed

Thanks. Committed to CVS HEAD.

Comment #7

sun

German

Karlsruhe

CreditAttribution: sun commented 7 July 2010 at 14:30

+++ includes/session.inc
@@ -309,7 +309,10 @@ function drupal_session_regenerate() {
+    // If the session cookie lifetime is set, the session will expire $params['lifetime'] seconds from the current request.
+    // If it is not set, it will expire when the browser is closed.

Heavily exceeds 80 chars.

40 critical left. Go review some!