From 2deb8f4176193486356fbda9061c6c62463d76fe Mon Sep 17 00:00:00 2001 From: Simon Gow Date: Tue, 27 Nov 2018 16:40:15 +1300 Subject: [PATCH 1/4] Resolve Duplicate Headers Ensure only a single Set-Cookie header is returned from Session once we have data to save. Include backwards compatibility for PHP56 --- src/Control/Session.php | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/src/Control/Session.php b/src/Control/Session.php index 917fea78c..7f00c7bd4 100644 --- a/src/Control/Session.php +++ b/src/Control/Session.php @@ -300,6 +300,7 @@ class Session // If the session cookie is already set, then the session can be read even if headers_sent() = true // This helps with edge-case such as debugging. + $data = []; if (!session_id() && (!headers_sent() || !empty($_COOKIE[ini_get('session.name')]))) { if (!headers_sent()) { session_set_cookie_params($timeout, $path, $domain ?: null, $secure, true); @@ -319,36 +320,38 @@ class Session session_save_path($session_path); } - // If we want a secure cookie for HTTPS, use a seperate session name. This lets us have a - // seperate (less secure) session for non-HTTPS requests. Note that if this causes problems + // If we want a secure cookie for HTTPS, use a separate session name. This lets us have a + // separate (less secure) session for non-HTTPS requests // if headers_sent() is true then it's best to throw the resulting error rather than risk // a security hole. if ($secure) { session_name($this->config()->get('cookie_name_secure')); } - session_start(); + $sessionParameters = [ + "cookie_path" => $path, + "cookie_domain" => $domain ?: "", + "cookie_secure" => $secure, + "cookie_httponly" => true + ]; + + if ($timeout) { + $sessionParameters["cookie_lifetime"] = $timeout; + } + + session_start($sessionParameters); if (isset($_SESSION)) { // Initialise data from session store if present $data = $_SESSION; + // Merge in existing in-memory data, taking priority over session store data $this->recursivelyApply((array)$this->data, $data); - } else { - // Use in-memory data if the session is lazy started - $data = $this->data; } - $this->data = $data ?: []; - } else { - $this->data = []; } - // Modify the timeout behaviour so it's the *inactive* time before the session expires. - // By default it's the total session lifetime - if ($timeout && !headers_sent()) { - Cookie::set(session_name(), session_id(), $timeout/86400, $path, $domain ? $domain - : null, $secure, true); - } + // Save any modified session data back to the session store if present, otherwise initialise it to an array. + $this->data = $data; $this->started = true; } From 4eb6669c08aa096fc46679d80a3d4424bf93f06e Mon Sep 17 00:00:00 2001 From: Simon Gow Date: Mon, 3 Dec 2018 10:08:50 +1300 Subject: [PATCH 2/4] #8543 Resolve Duplicate Headers Put cookie_lifetime back into the session parameters. --- src/Control/Session.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Control/Session.php b/src/Control/Session.php index 7f00c7bd4..cd543109c 100644 --- a/src/Control/Session.php +++ b/src/Control/Session.php @@ -331,14 +331,11 @@ class Session $sessionParameters = [ "cookie_path" => $path, "cookie_domain" => $domain ?: "", + "cookie_lifetime" => $timeout ?: 0, "cookie_secure" => $secure, "cookie_httponly" => true ]; - if ($timeout) { - $sessionParameters["cookie_lifetime"] = $timeout; - } - session_start($sessionParameters); if (isset($_SESSION)) { From 1edfa4d956925dc02590a1eec404181ac4b6eef9 Mon Sep 17 00:00:00 2001 From: Simon Gow Date: Mon, 3 Dec 2018 12:28:21 +1300 Subject: [PATCH 3/4] #8543 Resolve Duplicate Headers - Replace session name lookup with function to also check secure cookies - Added timeout which defaults to 0 (same as PHP) - Removed php7 style of session_start from PR - moved session_start into headers sent block to prevent warnings. --- src/Control/Session.php | 44 +++++++++++++++++------------------------ 1 file changed, 18 insertions(+), 26 deletions(-) diff --git a/src/Control/Session.php b/src/Control/Session.php index cd543109c..5b8f25c28 100644 --- a/src/Control/Session.php +++ b/src/Control/Session.php @@ -301,43 +301,35 @@ class Session // If the session cookie is already set, then the session can be read even if headers_sent() = true // This helps with edge-case such as debugging. $data = []; - if (!session_id() && (!headers_sent() || !empty($_COOKIE[ini_get('session.name')]))) { + if (!session_id() && (!headers_sent() || $this->requestContainsSessionId($request))) { if (!headers_sent()) { - session_set_cookie_params($timeout, $path, $domain ?: null, $secure, true); + session_set_cookie_params($timeout ?: 0, $path, $domain ?: null, $secure, true); $limiter = $this->config()->get('sessionCacheLimiter'); if (isset($limiter)) { session_cache_limiter($limiter); } - // If headers are sent then we can't have a session_cache_limiter otherwise we'll get a warning + // Allow storing the session in a non standard location + if ($session_path) { + session_save_path($session_path); + } + + // If we want a secure cookie for HTTPS, use a separate session name. This lets us have a + // separate (less secure) session for non-HTTPS requests + // if headers_sent() is true then it's best to throw the resulting error rather than risk + // a security hole. + if ($secure) { + session_name($this->config()->get('cookie_name_secure')); + } + + session_start(); + } else { + // If headers are sent then we can't have a session_cache_limiter otherwise we'll get a warning session_cache_limiter(null); } - // Allow storing the session in a non standard location - if ($session_path) { - session_save_path($session_path); - } - - // If we want a secure cookie for HTTPS, use a separate session name. This lets us have a - // separate (less secure) session for non-HTTPS requests - // if headers_sent() is true then it's best to throw the resulting error rather than risk - // a security hole. - if ($secure) { - session_name($this->config()->get('cookie_name_secure')); - } - - $sessionParameters = [ - "cookie_path" => $path, - "cookie_domain" => $domain ?: "", - "cookie_lifetime" => $timeout ?: 0, - "cookie_secure" => $secure, - "cookie_httponly" => true - ]; - - session_start($sessionParameters); - if (isset($_SESSION)) { // Initialise data from session store if present $data = $_SESSION; From d01585cc98cd84bf45c0a9ac0dc24422d114077f Mon Sep 17 00:00:00 2001 From: Simon Gow Date: Fri, 7 Dec 2018 14:45:41 +1300 Subject: [PATCH 4/4] #8543 Resolve Duplicate Headers - fix linting --- src/Control/Session.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Control/Session.php b/src/Control/Session.php index 5b8f25c28..e2591d2c5 100644 --- a/src/Control/Session.php +++ b/src/Control/Session.php @@ -324,7 +324,6 @@ class Session } session_start(); - } else { // If headers are sent then we can't have a session_cache_limiter otherwise we'll get a warning session_cache_limiter(null); @@ -430,7 +429,7 @@ class Session } $var[] = $val; - $diffVar[sizeof($var)-1] = $val; + $diffVar[sizeof($var) - 1] = $val; } /**