Skip to content

Commit

Permalink
Fixes #9438, #13740, #15037: Handle DB session callback custom fields…
Browse files Browse the repository at this point in the history
… before session closed
  • Loading branch information
lubosdz authored and samdark committed Mar 9, 2019
1 parent a8d4f85 commit 8bb334b
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 24 deletions.
1 change: 1 addition & 0 deletions framework/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ Yii Framework 2 Change Log
2.0.17 under development
------------------------

- Bug #9438, #13740, #15037: Handle DB session callback custom fields before session closed (lubosdz)
- Bug #16681: `ActiveField::inputOptions` were not used during some widgets rendering (GHopperMSK)
- Bug #17133: Fixed aliases rendering during help generation for a console command (GHopperMSK)
- Bug #17185: Fixed `AssetManager` timestamp appending when a file is published manually (GHopperMSK)
Expand Down
40 changes: 36 additions & 4 deletions framework/web/DbSession.php
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ class DbSession extends MultiFieldSession
*/
public $sessionTable = '{{%session}}';

/**
* @var array Session fields to be written into session table columns
* @since 2.0.17
*/
protected $fields = [];

/**
* Initializes the DbSession component.
Expand Down Expand Up @@ -136,6 +141,19 @@ public function regenerateID($deleteOldSession = false)
}
}

/**
* Ends the current session and store session data.
* @since 2.0.17
*/
public function close()
{
if ($this->getIsActive()) {
// prepare writeCallback fields before session closes
$this->fields = $this->composeFields();
YII_DEBUG ? session_write_close() : @session_write_close();
}
}

/**
* Session read handler.
* @internal Do not call this method directly.
Expand Down Expand Up @@ -169,14 +187,28 @@ public function writeSession($id, $data)
// exception must be caught in session write handler
// https://secure.php.net/manual/en/function.session-set-save-handler.php#refsect1-function.session-set-save-handler-notes
try {
$fields = $this->composeFields($id, $data);
$fields = $this->typecastFields($fields);
$this->db->createCommand()->upsert($this->sessionTable, $fields)->execute();
// ensure backwards compatability (fixed #9438)
if ($this->writeCallback && !$this->fields) {
$this->fields = $this->composeFields();
}
// ensure data consistency
if (!isset($this->fields['data'])) {
$this->fields['data'] = $data;
} else {
$_SESSION = $this->fields['data'];
}
// ensure 'id' and 'expire' are never affected by [[writeCallback]]
$this->fields = array_merge($this->fields, [
'id' => $id,
'expire' => time() + $this->getTimeout(),
]);
$this->fields = $this->typecastFields($this->fields);
$this->db->createCommand()->upsert($this->sessionTable, $this->fields)->execute();
$this->fields = [];
} catch (\Exception $e) {
Yii::$app->errorHandler->handleException($e);
return false;
}

return true;
}

Expand Down
29 changes: 9 additions & 20 deletions framework/web/MultiFieldSession.php
Original file line number Diff line number Diff line change
Expand Up @@ -89,30 +89,19 @@ public function getUseCustomStorage()

/**
* Composes storage field set for session writing.
* @param string $id session id
* @param string $data session data
* @param string $id Optional session id
* @param string $data Optional session data
* @return array storage fields
*/
protected function composeFields($id, $data)
protected function composeFields($id = null, $data = null)
{
$fields = [
'data' => $data,
];
if ($this->writeCallback !== null) {
$fields = array_merge(
$fields,
call_user_func($this->writeCallback, $this)
);
if (!is_string($fields['data'])) {
$_SESSION = $fields['data'];
$fields['data'] = session_encode();
}
$fields = $this->writeCallback ? call_user_func($this->writeCallback, $this) : [];
if ($id !== null) {
$fields['id'] = $id;
}
if ($data !== null) {
$fields['data'] = $data;
}
// ensure 'id' and 'expire' are never affected by [[writeCallback]]

This comment has been minimized.

Copy link
@thkillert

thkillert Apr 3, 2019

Removing this part caused some trouble in our live application as session id's and expire times were not preserved (e.g. on logins).

We currently added this code part in our writeCallback which solves the problem for us.
Just wanted to raise attention on that as the change was not clearly documented.
Maybe you removed this code by accident...or was it intended?

This comment has been minimized.

Copy link
@samdark

samdark Apr 3, 2019

Member

@thkillert would you please create an issue with details on how to reproduce it?

$fields = array_merge($fields, [
'id' => $id,
'expire' => time() + $this->getTimeout(),
]);
return $fields;
}

Expand Down
28 changes: 28 additions & 0 deletions tests/framework/web/session/AbstractDbSessionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use Yii;
use yii\db\Connection;
use yii\db\Query;
use yii\db\Migration;
use yii\web\DbSession;
use yiiunit\framework\console\controllers\EchoMigrateController;
use yiiunit\TestCase;
Expand Down Expand Up @@ -147,6 +148,33 @@ public function testWriteCustomField()
$this->assertSame('changed by callback data', $session->readSession('test'));
}

/**
* @depends testReadWrite
*/
public function testWriteCustomFieldWithUserId()
{
$session = new DbSession();
$session->open();
$session->set('user_id', 12345);

// add mapped custom column
$migration = new Migration;
$migration->addColumn($session->sessionTable, 'user_id', $migration->integer());

$session->writeCallback = function ($session) {
return ['user_id' => $session['user_id']];
};

// here used to be error, fixed issue #9438
$session->close();

// reopen & read session from DB
$session->open();
$loadedUserId = empty($session['user_id']) ? null : $session['user_id'];
$this->assertSame($loadedUserId, 12345);
$session->close();
}

protected function buildObjectForSerialization()
{
$object = new \stdClass();
Expand Down

0 comments on commit 8bb334b

Please sign in to comment.