Skip to content

Commit

Permalink
[HttpFoundation][FrameworkBundle] fix support for samesite in session…
Browse files Browse the repository at this point in the history
… cookies
  • Loading branch information
fabpot authored and nicolas-grekas committed Feb 6, 2020
1 parent 7a2ab8c commit 4d440be
Show file tree
Hide file tree
Showing 10 changed files with 192 additions and 38 deletions.
59 changes: 59 additions & 0 deletions Session/SessionUtils.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <[email protected]>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\HttpFoundation\Session;

/**
* Session utility functions.
*
* @author Nicolas Grekas <[email protected]>
* @author Rémon van de Kamp <[email protected]>
*
* @internal
*/
final class SessionUtils
{
/**
* Find the session header amongst the headers that are to be sent, remove it, and return
* it so the caller can process it further.
*/
public static function popSessionCookie($sessionName, $sessionId)
{
$sessionCookie = null;
$sessionCookiePrefix = sprintf(' %s=', urlencode($sessionName));
$sessionCookieWithId = sprintf('%s%s;', $sessionCookiePrefix, urlencode($sessionId));
$otherCookies = [];
foreach (headers_list() as $h) {
if (0 !== stripos($h, 'Set-Cookie:')) {
continue;
}
if (11 === strpos($h, $sessionCookiePrefix, 11)) {
$sessionCookie = $h;

if (11 !== strpos($h, $sessionCookieWithId, 11)) {
$otherCookies[] = $h;
}
} else {
$otherCookies[] = $h;
}
}
if (null === $sessionCookie) {
return null;
}

header_remove('Set-Cookie');
foreach ($otherCookies as $h) {
header($h, false);
}

return $sessionCookie;
}
}
50 changes: 22 additions & 28 deletions Session/Storage/Handler/AbstractSessionHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

namespace Symfony\Component\HttpFoundation\Session\Storage\Handler;

use Symfony\Component\HttpFoundation\Session\SessionUtils;

/**
* This abstract session handler provides a generic implementation
* of the PHP 7.0 SessionUpdateTimestampHandlerInterface,
Expand All @@ -27,7 +29,7 @@ abstract class AbstractSessionHandler implements \SessionHandlerInterface, \Sess
private $igbinaryEmptyData;

/**
* {@inheritdoc}
* @return bool
*/
public function open($savePath, $sessionName)
{
Expand Down Expand Up @@ -62,7 +64,7 @@ abstract protected function doWrite($sessionId, $data);
abstract protected function doDestroy($sessionId);

/**
* {@inheritdoc}
* @return bool
*/
public function validateId($sessionId)
{
Expand All @@ -73,7 +75,7 @@ public function validateId($sessionId)
}

/**
* {@inheritdoc}
* @return string
*/
public function read($sessionId)
{
Expand All @@ -99,7 +101,7 @@ public function read($sessionId)
}

/**
* {@inheritdoc}
* @return bool
*/
public function write($sessionId, $data)
{
Expand All @@ -124,7 +126,7 @@ public function write($sessionId, $data)
}

/**
* {@inheritdoc}
* @return bool
*/
public function destroy($sessionId)
{
Expand All @@ -135,31 +137,23 @@ public function destroy($sessionId)
if (!$this->sessionName) {
throw new \LogicException(sprintf('Session name cannot be empty, did you forget to call "parent::open()" in "%s"?.', static::class));
}
$sessionCookie = sprintf(' %s=', urlencode($this->sessionName));
$sessionCookieWithId = sprintf('%s%s;', $sessionCookie, urlencode($sessionId));
$sessionCookieFound = false;
$otherCookies = [];
foreach (headers_list() as $h) {
if (0 !== stripos($h, 'Set-Cookie:')) {
continue;
}
if (11 === strpos($h, $sessionCookie, 11)) {
$sessionCookieFound = true;

if (11 !== strpos($h, $sessionCookieWithId, 11)) {
$otherCookies[] = $h;
}
$cookie = SessionUtils::popSessionCookie($this->sessionName, $sessionId);

/*
* We send an invalidation Set-Cookie header (zero lifetime)
* when either the session was started or a cookie with
* the session name was sent by the client (in which case
* we know it's invalid as a valid session cookie would've
* started the session).
*/
if (null === $cookie || isset($_COOKIE[$this->sessionName])) {
if (\PHP_VERSION_ID < 70300) {
setcookie($this->sessionName, '', 0, ini_get('session.cookie_path'), ini_get('session.cookie_domain'), filter_var(ini_get('session.cookie_secure'), FILTER_VALIDATE_BOOLEAN), filter_var(ini_get('session.cookie_httponly'), FILTER_VALIDATE_BOOLEAN));
} else {
$otherCookies[] = $h;
}
}
if ($sessionCookieFound) {
header_remove('Set-Cookie');
foreach ($otherCookies as $h) {
header($h, false);
$params = session_get_cookie_params();
unset($params['lifetime']);
setcookie($this->sessionName, '', $params);
}
} else {
setcookie($this->sessionName, '', 0, ini_get('session.cookie_path'), ini_get('session.cookie_domain'), filter_var(ini_get('session.cookie_secure'), FILTER_VALIDATE_BOOLEAN), filter_var(ini_get('session.cookie_httponly'), FILTER_VALIDATE_BOOLEAN));
}
}

Expand Down
42 changes: 36 additions & 6 deletions Session/Storage/NativeSessionStorage.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
namespace Symfony\Component\HttpFoundation\Session\Storage;

use Symfony\Component\HttpFoundation\Session\SessionBagInterface;
use Symfony\Component\HttpFoundation\Session\SessionUtils;
use Symfony\Component\HttpFoundation\Session\Storage\Handler\StrictSessionHandler;
use Symfony\Component\HttpFoundation\Session\Storage\Proxy\AbstractProxy;
use Symfony\Component\HttpFoundation\Session\Storage\Proxy\SessionHandlerProxy;
Expand Down Expand Up @@ -48,6 +49,11 @@ class NativeSessionStorage implements SessionStorageInterface
*/
protected $metadataBag;

/**
* @var string|null
*/
private $emulateSameSite;

/**
* Depending on how you want the storage driver to behave you probably
* want to override this constructor entirely.
Expand All @@ -67,6 +73,7 @@ class NativeSessionStorage implements SessionStorageInterface
* cookie_lifetime, "0"
* cookie_path, "/"
* cookie_secure, ""
* cookie_samesite, null
* entropy_file, ""
* entropy_length, "0"
* gc_divisor, "100"
Expand Down Expand Up @@ -94,9 +101,7 @@ class NativeSessionStorage implements SessionStorageInterface
* trans_sid_hosts, $_SERVER['HTTP_HOST']
* trans_sid_tags, "a=href,area=href,frame=src,form="
*
* @param array $options Session configuration options
* @param \SessionHandlerInterface|null $handler
* @param MetadataBag $metaBag MetadataBag
* @param AbstractProxy|\SessionHandlerInterface|null $handler
*/
public function __construct(array $options = [], $handler = null, MetadataBag $metaBag = null)
{
Expand Down Expand Up @@ -150,6 +155,13 @@ public function start()
throw new \RuntimeException('Failed to start the session');
}

if (null !== $this->emulateSameSite) {
$originalCookie = SessionUtils::popSessionCookie(session_name(), session_id());
if (null !== $originalCookie) {
header(sprintf('%s; SameSite=%s', $originalCookie, $this->emulateSameSite), false);
}
}

$this->loadSession();

return true;
Expand Down Expand Up @@ -215,6 +227,13 @@ public function regenerate($destroy = false, $lifetime = null)
// @see https://bugs.php.net/70013
$this->loadSession();

if (null !== $this->emulateSameSite) {
$originalCookie = SessionUtils::popSessionCookie(session_name(), session_id());
if (null !== $originalCookie) {
header(sprintf('%s; SameSite=%s', $originalCookie, $this->emulateSameSite), false);
}
}

return $isRegenerated;
}

Expand All @@ -223,6 +242,7 @@ public function regenerate($destroy = false, $lifetime = null)
*/
public function save()
{
// Store a copy so we can restore the bags in case the session was not left empty
$session = $_SESSION;

foreach ($this->bags as $bag) {
Expand All @@ -248,7 +268,11 @@ public function save()
session_write_close();
} finally {
restore_error_handler();
$_SESSION = $session;

// Restore only if not empty
if ($_SESSION) {
$_SESSION = $session;
}
}

$this->closed = true;
Expand Down Expand Up @@ -347,7 +371,7 @@ public function setOptions(array $options)

$validOptions = array_flip([
'cache_expire', 'cache_limiter', 'cookie_domain', 'cookie_httponly',
'cookie_lifetime', 'cookie_path', 'cookie_secure',
'cookie_lifetime', 'cookie_path', 'cookie_secure', 'cookie_samesite',
'entropy_file', 'entropy_length', 'gc_divisor',
'gc_maxlifetime', 'gc_probability', 'hash_bits_per_character',
'hash_function', 'lazy_write', 'name', 'referer_check',
Expand All @@ -360,6 +384,12 @@ public function setOptions(array $options)

foreach ($options as $key => $value) {
if (isset($validOptions[$key])) {
if ('cookie_samesite' === $key && \PHP_VERSION_ID < 70300) {
// PHP < 7.3 does not support same_site cookies. We will emulate it in
// the start() method instead.
$this->emulateSameSite = $value;
continue;
}
ini_set('url_rewriter.tags' !== $key ? 'session.'.$key : $key, $value);
}
}
Expand All @@ -381,7 +411,7 @@ public function setOptions(array $options)
* @see https://php.net/sessionhandlerinterface
* @see https://php.net/sessionhandler
*
* @param \SessionHandlerInterface|null $saveHandler
* @param AbstractProxy|\SessionHandlerInterface|null $saveHandler
*
* @throws \InvalidArgumentException
*/
Expand Down
3 changes: 2 additions & 1 deletion Tests/Session/Storage/Handler/Fixtures/storage.expected
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,11 @@ $_SESSION is not empty
write
destroy
close
$_SESSION is not empty
$_SESSION is empty
Array
(
[0] => Content-Type: text/plain; charset=utf-8
[1] => Cache-Control: max-age=0, private, must-revalidate
[2] => Set-Cookie: sid=deleted; expires=Thu, 01-Jan-1970 00:00:01 GMT; Max-Age=0; path=/; secure; HttpOnly
)
shutdown
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,6 @@ Array
[0] => Content-Type: text/plain; charset=utf-8
[1] => Cache-Control: max-age=10800, private, must-revalidate
[2] => Set-Cookie: abc=def
[3] => Set-Cookie: sid=deleted; expires=Thu, 01-Jan-1970 00:00:01 GMT; Max-Age=0; path=/; secure; HttpOnly
)
shutdown
16 changes: 16 additions & 0 deletions Tests/Session/Storage/Handler/Fixtures/with_samesite.expected
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
open
validateId
read
doRead:
read

write
doWrite: foo|s:3:"bar";
close
Array
(
[0] => Content-Type: text/plain; charset=utf-8
[1] => Cache-Control: max-age=0, private, must-revalidate
[2] => Set-Cookie: sid=random_session_id; path=/; secure; HttpOnly; SameSite=lax
)
shutdown
13 changes: 13 additions & 0 deletions Tests/Session/Storage/Handler/Fixtures/with_samesite.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?php

require __DIR__.'/common.inc';

use Symfony\Component\HttpFoundation\Session\Storage\NativeSessionStorage;

$storage = new NativeSessionStorage(['cookie_samesite' => 'lax']);
$storage->setSaveHandler(new TestSessionHandler());
$storage->start();

$_SESSION = ['foo' => 'bar'];

ob_start(function ($buffer) { return str_replace(session_id(), 'random_session_id', $buffer); });
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
open
validateId
read
doRead:
read
destroy
close
open
validateId
read
doRead:
read

write
doWrite: foo|s:3:"bar";
close
Array
(
[0] => Content-Type: text/plain; charset=utf-8
[1] => Cache-Control: max-age=0, private, must-revalidate
[2] => Set-Cookie: sid=random_session_id; path=/; secure; HttpOnly; SameSite=lax
)
shutdown
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?php

require __DIR__.'/common.inc';

use Symfony\Component\HttpFoundation\Session\Storage\NativeSessionStorage;

$storage = new NativeSessionStorage(['cookie_samesite' => 'lax']);
$storage->setSaveHandler(new TestSessionHandler());
$storage->start();

$_SESSION = ['foo' => 'bar'];

$storage->regenerate(true);

ob_start(function ($buffer) { return preg_replace('~_sf2_meta.*$~m', '', str_replace(session_id(), 'random_session_id', $buffer)); });
8 changes: 5 additions & 3 deletions Tests/Session/Storage/NativeSessionStorageTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class NativeSessionStorageTest extends TestCase
protected function setUp()
{
$this->iniSet('session.save_handler', 'files');
$this->iniSet('session.save_path', $this->savePath = sys_get_temp_dir().'/sf2test');
$this->iniSet('session.save_path', $this->savePath = sys_get_temp_dir().'/sftest');
if (!is_dir($this->savePath)) {
mkdir($this->savePath);
}
Expand Down Expand Up @@ -167,6 +167,10 @@ public function testCookieOptions()
'cookie_httponly' => false,
];

if (\PHP_VERSION_ID >= 70300) {
$options['cookie_samesite'] = 'lax';
}

$this->getStorage($options);
$temp = session_get_cookie_params();
$gco = [];
Expand All @@ -175,8 +179,6 @@ public function testCookieOptions()
$gco['cookie_'.$key] = $value;
}

unset($gco['cookie_samesite']);

$this->assertEquals($options, $gco);
}

Expand Down

0 comments on commit 4d440be

Please sign in to comment.