From 5a70162102caff68d1b888959bf220e9ae619fc1 Mon Sep 17 00:00:00 2001 From: michalsn Date: Sat, 26 Jun 2021 11:00:32 +0200 Subject: [PATCH] Fix database session bug with timestamp in MySQLi --- system/Session/Handlers/DatabaseHandler.php | 11 +- .../20160428212500_Create_test_tables.php | 29 ++++ .../_support/Database/Seeds/CITestSeeder.php | 18 +++ tests/system/Database/Live/MetadataTest.php | 4 + .../Session/Handlers/DatabaseHandlerTest.php | 128 ++++++++++++++++++ 5 files changed, 183 insertions(+), 7 deletions(-) create mode 100644 tests/system/Session/Handlers/DatabaseHandlerTest.php diff --git a/system/Session/Handlers/DatabaseHandler.php b/system/Session/Handlers/DatabaseHandler.php index 647f73eb0da0..69c76ff3df73 100644 --- a/system/Session/Handlers/DatabaseHandler.php +++ b/system/Session/Handlers/DatabaseHandler.php @@ -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(); } @@ -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(); } @@ -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(); } //-------------------------------------------------------------------- diff --git a/tests/_support/Database/Migrations/20160428212500_Create_test_tables.php b/tests/_support/Database/Migrations/20160428212500_Create_test_tables.php index 7fdfbb0c3c34..629550cbfbdd 100644 --- a/tests/_support/Database/Migrations/20160428212500_Create_test_tables.php +++ b/tests/_support/Database/Migrations/20160428212500_Create_test_tables.php @@ -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); + } } //-------------------------------------------------------------------- @@ -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); + } } } diff --git a/tests/_support/Database/Seeds/CITestSeeder.php b/tests/_support/Database/Seeds/CITestSeeder.php index 45633244ac51..e633543414f7 100644 --- a/tests/_support/Database/Seeds/CITestSeeder.php +++ b/tests/_support/Database/Seeds/CITestSeeder.php @@ -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(); diff --git a/tests/system/Database/Live/MetadataTest.php b/tests/system/Database/Live/MetadataTest.php index 9480506a8780..4d5a08e235cb 100644 --- a/tests/system/Database/Live/MetadataTest.php +++ b/tests/system/Database/Live/MetadataTest.php @@ -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() diff --git a/tests/system/Session/Handlers/DatabaseHandlerTest.php b/tests/system/Session/Handlers/DatabaseHandlerTest.php new file mode 100644 index 000000000000..17a396a9b769 --- /dev/null +++ b/tests/system/Session/Handlers/DatabaseHandlerTest.php @@ -0,0 +1,128 @@ +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)); + } +}