Skip to content

Commit

Permalink
Fix session expiration on several drivers.
Browse files Browse the repository at this point in the history
  • Loading branch information
taylorotwell committed Jun 10, 2016
1 parent ecf782a commit 0831312
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 7 deletions.
16 changes: 14 additions & 2 deletions src/Illuminate/Session/CookieSessionHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Illuminate\Session;

use Carbon\Carbon;
use SessionHandlerInterface;
use Symfony\Component\HttpFoundation\Request;
use Illuminate\Contracts\Cookie\QueueingFactory as CookieJar;
Expand Down Expand Up @@ -56,15 +57,26 @@ public function close()
*/
public function read($sessionId)
{
return $this->request->cookies->get($sessionId) ?: '';
$value = $this->request->cookies->get($sessionId) ?: '';

if (! is_null($decoded = json_decode($value, true)) && is_array($decoded)) {
if (isset($decoded['expires']) && time() <= $decoded['expires']) {
return $decoded['data'];
}
}

return '';
}

/**
* {@inheritdoc}
*/
public function write($sessionId, $data)
{
$this->cookie->queue($sessionId, $data, $this->minutes);
$this->cookie->queue($sessionId, json_encode([
'data' => $data,
'expires' => Carbon::now()->addMinutes($this->minutes)->getTimestamp(),
]), $this->minutes);
}

/**
Expand Down
18 changes: 17 additions & 1 deletion src/Illuminate/Session/DatabaseSessionHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Illuminate\Session;

use Carbon\Carbon;
use SessionHandlerInterface;
use Illuminate\Database\ConnectionInterface;

Expand All @@ -21,6 +22,13 @@ class DatabaseSessionHandler implements SessionHandlerInterface, ExistenceAwareI
*/
protected $table;

/**
* The number of minutes the session should be valid.
*
* @var int
*/
protected $minutes;

/**
* The existence state of the session.
*
Expand All @@ -33,11 +41,13 @@ class DatabaseSessionHandler implements SessionHandlerInterface, ExistenceAwareI
*
* @param \Illuminate\Database\ConnectionInterface $connection
* @param string $table
* @param int $minutes
* @return void
*/
public function __construct(ConnectionInterface $connection, $table)
public function __construct(ConnectionInterface $connection, $table, $minutes)
{
$this->table = $table;
$this->minutes = $minutes;
$this->connection = $connection;
}

Expand All @@ -64,6 +74,12 @@ public function read($sessionId)
{
$session = (object) $this->getQuery()->find($sessionId);

if (isset($session->last_activity)) {
if ($session->last_activity < Carbon::now()->subMinutes($this->minutes)->getTimestamp()) {
return;
}
}

if (isset($session->payload)) {
$this->exists = true;

Expand Down
16 changes: 14 additions & 2 deletions src/Illuminate/Session/FileSessionHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Illuminate\Session;

use Carbon\Carbon;
use SessionHandlerInterface;
use Symfony\Component\Finder\Finder;
use Illuminate\Filesystem\Filesystem;
Expand All @@ -22,17 +23,26 @@ class FileSessionHandler implements SessionHandlerInterface
*/
protected $path;

/**
* The number of minutes the session should be valid.
*
* @var int
*/

This comment has been minimized.

Copy link
@itsdave

itsdave Jun 14, 2016

Can we set a default? We have a session handler which extends this, and due to the missing $minutes value the sessions were expiring as soon as they were created, silently!

protected $minutes;

/**
* Create a new file driven handler instance.
*
* @param \Illuminate\Filesystem\Filesystem $files
* @param string $path
* @param int $minutes
* @return void
*/
public function __construct(Filesystem $files, $path)
public function __construct(Filesystem $files, $path, $minutes)
{
$this->path = $path;
$this->files = $files;
$this->minutes = $minutes;
}

/**
Expand All @@ -57,7 +67,9 @@ public function close()
public function read($sessionId)
{
if ($this->files->exists($path = $this->path.'/'.$sessionId)) {
return $this->files->get($path);
if (filemtime($path) >= Carbon::now()->subMinutes($this->minutes)->getTimestamp()) {
return $this->files->get($path);
}
}

return '';
Expand Down
8 changes: 6 additions & 2 deletions src/Illuminate/Session/SessionManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,9 @@ protected function createNativeDriver()
{
$path = $this->app['config']['session.files'];

return $this->buildSession(new FileSessionHandler($this->app['files'], $path));
$lifetime = $this->app['config']['session.lifetime'];

return $this->buildSession(new FileSessionHandler($this->app['files'], $path, $lifetime));
}

/**
Expand All @@ -73,7 +75,9 @@ protected function createDatabaseDriver()

$table = $this->app['config']['session.table'];

return $this->buildSession(new DatabaseSessionHandler($connection, $table));
$lifetime = $this->app['config']['session.lifetime'];

return $this->buildSession(new DatabaseSessionHandler($connection, $table, $lifetime));
}

/**
Expand Down

1 comment on commit 0831312

@markvaneijk
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is probably a bug in this commit, because after updating to latest Laravel 5.2.37, I get on every page refresh an error:

SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry 'bdddd8f13d482b88406b1a10c4bd2bbaff0c694c' for key 'sessions_id_unique'

And on 5.2.36 no problems..

Please sign in to comment.