Skip to content

Commit

Permalink
Merge pull request #4876 from michalsn/fix/database-session
Browse files Browse the repository at this point in the history 
Fix database session bug with timestamp in MySQLi
  • Loading branch information
@michalsn
michalsn authored Jun 27, 2021
2 parents 85cd690 + 5a70162 commit b156db1
Show file tree
Hide file tree
Showing 5 changed files with 183 additions and 7 deletions.
11 changes: 4 additions & 7 deletions system/Session/Handlers/DatabaseHandler.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,11 +198,10 @@ public function write($sessionID, $sessionData): bool
$insertData = [
'id' => $sessionID,
'ip_address' => $this->ipAddress,
'timestamp' => 'now()',
'data' => $this->platform === 'postgre' ? '\x' . bin2hex($sessionData) : $sessionData,
];

if (! $this->db->table($this->table)->insert($insertData)) {
if (! $this->db->table($this->table)->set('timestamp', 'now()', false)->insert($insertData)) {
return $this->fail();
}

Expand All @@ -218,15 +217,13 @@ public function write($sessionID, $sessionData): bool
$builder = $builder->where('ip_address', $this->ipAddress);
}

$updateData = [
'timestamp' => 'now()',
];
$updateData = [];

if ($this->fingerprint !== md5($sessionData)) {
$updateData['data'] = ($this->platform === 'postgre') ? '\x' . bin2hex($sessionData) : $sessionData;
}

if (! $builder->update($updateData)) {
if (! $builder->set('timestamp', 'now()', false)->update($updateData)) {
return $this->fail();
}

Expand Down Expand Up @@ -299,7 +296,7 @@ public function gc($maxlifetime): bool
$separator = $this->platform === 'postgre' ? '\'' : ' ';
$interval = implode($separator, ['', "{$maxlifetime} second", '']);

return $this->db->table($this->table)->delete("timestamp < now() - INTERVAL {$interval}") ? true : $this->fail();
return $this->db->table($this->table)->where("timestamp <", "now() - INTERVAL {$interval}", false)->delete() ? true : $this->fail();
}

//--------------------------------------------------------------------
Expand Down
29 changes: 29 additions & 0 deletions tests/_support/Database/Migrations/20160428212500_Create_test_tables.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,31 @@ public function up()
'ip' => ['type' => 'VARCHAR', 'constraint' => 100],
'ip2' => ['type' => 'VARCHAR', 'constraint' => 100],
])->createTable('ip_table', true);

// Database session table
if ($this->db->DBDriver === 'MySQLi') {
$this->forge->addField([
'id' => ['type' => 'VARCHAR', 'constraint' => 128, 'null' => false],
'ip_address' => ['type' => 'VARCHAR', 'constraint' => 45, 'null' => false],
'timestamp timestamp DEFAULT CURRENT_TIMESTAMP NOT NULL',
'data' => ['type' => 'BLOB', 'null' => false],
]);
$this->forge->addKey('id', true);
$this->forge->addKey('timestamp');
$this->forge->createTable('ci_sessions', true);
}

if ($this->db->DBDriver === 'Postgre') {
$this->forge->addField([
'id' => ['type' => 'VARCHAR', 'constraint' => 128, 'null' => false],
'ip_address inet NOT NULL',
'timestamp timestamptz DEFAULT CURRENT_TIMESTAMP NOT NULL',
"data bytea DEFAULT '' NOT NULL",
]);
$this->forge->addKey('id', true);
$this->forge->addKey('timestamp');
$this->forge->createTable('ci_sessions', true);
}
}

//--------------------------------------------------------------------
Expand All @@ -133,5 +158,9 @@ public function down()
$this->forge->dropTable('stringifypkey', true);
$this->forge->dropTable('without_auto_increment', true);
$this->forge->dropTable('ip_table', true);

if (in_array($this->db->DBDriver, ['MySQLi', 'Postgre'])) {
$this->forge->dropTable('ci_sessions', true);
}
}
}
18 changes: 18 additions & 0 deletions tests/_support/Database/Seeds/CITestSeeder.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,24 @@ public function run()
);
}

if ($this->db->DBDriver === 'MySQLi') {
$data['ci_sessions'][] = [
'id' => '1f5o06b43phsnnf8if6bo33b635e4p2o',
'ip_address' => '127.0.0.1',
'timestamp' => '2021-06-25 21:54:14',
'data' => '__ci_last_regenerate|i:1624650854;_ci_previous_url|s:40:\"http://localhost/index.php/home/index\";',
];
}

if ($this->db->DBDriver === 'Postgre') {
$data['ci_sessions'][] = [
'id' => '1f5o06b43phsnnf8if6bo33b635e4p2o',
'ip_address' => '127.0.0.1',
'timestamp' => '2021-06-25 21:54:14.991403+02',
'data' => '\x' . bin2hex('__ci_last_regenerate|i:1624650854;_ci_previous_url|s:40:\"http://localhost/index.php/home/index\";'),
];
}

foreach ($data as $table => $dummy_data) {
$this->db->table($table)->truncate();

Expand Down
4 changes: 4 additions & 0 deletions tests/system/Database/Live/MetadataTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ protected function setUp(): void
$prefix . 'without_auto_increment',
$prefix . 'ip_table',
];

if (in_array($this->db->DBDriver, ['MySQLi', 'Postgre'], true)) {
$this->expectedTables[] = $prefix . 'ci_sessions';
}
}

public function testListTables()
Expand Down
128 changes: 128 additions & 0 deletions tests/system/Session/Handlers/DatabaseHandlerTest.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
<?php

namespace CodeIgniter\Session\Handlers;

use CodeIgniter\Test\CIUnitTestCase;
use CodeIgniter\Test\DatabaseTestTrait;
use CodeIgniter\Test\ReflectionHelper;
use Config\App as AppConfig;
use Config\Database as DatabaseConfig;

class DatabaseHandlerTest extends CIUnitTestCase
{
use DatabaseTestTrait, ReflectionHelper;

protected $refresh = true;

protected $seed = 'Tests\Support\Database\Seeds\CITestSeeder';

protected function setUp(): void
{
parent::setUp();

if (! in_array(config(DatabaseConfig::class)->tests['DBDriver'], ['MySQLi', 'Postgre'], true)) {
$this->markTestSkipped('Database Session Handler requires database driver to be MySQLi or Postgre');
}
}

protected function getInstance($options = [])
{
$defaults = [
'sessionDriver' => 'CodeIgniter\Session\Handlers\DatabaseHandler',
'sessionCookieName' => 'ci_session',
'sessionExpiration' => 7200,
'sessionSavePath' => 'ci_sessions',
'sessionMatchIP' => false,
'sessionTimeToUpdate' => 300,
'sessionRegenerateDestroy' => false,
'cookieDomain' => '',
'cookiePrefix' => '',
'cookiePath' => '/',
'cookieSecure' => false,
'cookieSameSite' => 'Lax',
];

$config = array_merge($defaults, $options);
$appConfig = new AppConfig();

foreach ($config as $key => $c) {
$appConfig->$key = $c;
}

return new DatabaseHandler($appConfig, '127.0.0.1');
}

public function testOpen()
{
$handler = $this->getInstance();
$this->assertTrue($handler->open('ci_sessions', 'ci_session'));
}

public function testReadSuccess()
{
$handler = $this->getInstance();
$expected = '__ci_last_regenerate|i:1624650854;_ci_previous_url|s:40:\"http://localhost/index.php/home/index\";';
$this->assertSame($expected, $handler->read('1f5o06b43phsnnf8if6bo33b635e4p2o'));

$this->assertTrue($this->getPrivateProperty($handler, 'rowExists'));
$this->assertSame('1483201a66afd2bd671e4a67dc6ecf24', $this->getPrivateProperty($handler, 'fingerprint'));
}

public function testReadFailure()
{
$handler = $this->getInstance();
$this->assertSame('', $handler->read('123456b43phsnnf8if6bo33b635e4321'));

$this->assertFalse($this->getPrivateProperty($handler, 'rowExists'));
$this->assertSame('d41d8cd98f00b204e9800998ecf8427e', $this->getPrivateProperty($handler, 'fingerprint'));
}

public function testWriteInsert()
{
$handler = $this->getInstance();

$this->setPrivateProperty($handler, 'lock', true);

$data = '__ci_last_regenerate|i:1624650854;_ci_previous_url|s:40:\"http://localhost/index.php/home/index\";';
$this->assertTrue($handler->write('555556b43phsnnf8if6bo33b635e4444', $data));

$this->setPrivateProperty($handler, 'lock', false);

$row = $this->db->table('ci_sessions')
->getWhere(['id' => '555556b43phsnnf8if6bo33b635e4444'])
->getRow();

$this->assertGreaterThan(time() - 100, strtotime($row->timestamp));
$this->assertSame('1483201a66afd2bd671e4a67dc6ecf24', $this->getPrivateProperty($handler, 'fingerprint'));
}

public function testWriteUpdate()
{
$handler = $this->getInstance();

$this->setPrivateProperty($handler, 'sessionID', '1f5o06b43phsnnf8if6bo33b635e4p2o');
$this->setPrivateProperty($handler, 'rowExists', true);

$lockSession = $this->getPrivateMethodInvoker($handler, 'lockSession');
$lockSession('1f5o06b43phsnnf8if6bo33b635e4p2o');

$data = '__ci_last_regenerate|i:1624650854;_ci_previous_url|s:40:\"http://localhost/index.php/home/index\";';
$this->assertTrue($handler->write('1f5o06b43phsnnf8if6bo33b635e4p2o', $data));

$releaseLock = $this->getPrivateMethodInvoker($handler, 'releaseLock');
$releaseLock();

$row = $this->db->table('ci_sessions')
->getWhere(['id' => '1f5o06b43phsnnf8if6bo33b635e4p2o'])
->getRow();

$this->assertGreaterThan(time() - 100, strtotime($row->timestamp));
$this->assertSame('1483201a66afd2bd671e4a67dc6ecf24', $this->getPrivateProperty($handler, 'fingerprint'));
}

public function testGC()
{
$handler = $this->getInstance();
$this->assertTrue($handler->gc(3600));
}
}

0 comments on commit b156db1

Please sign in to comment.