MDL-79890 core: Session handlers must implement SessionHandlerInterface

This commit is contained in:
Andrew Nicols 2023-11-07 13:41:16 +08:00
parent dfccb3f3c4
commit e734858e55
No known key found for this signature in database
GPG Key ID: 6D1E3157C8CFBF14
4 changed files with 126 additions and 127 deletions

View File

@ -24,7 +24,7 @@
namespace core\session;
defined('MOODLE_INTERNAL') || die();
use SessionHandlerInterface;
/**
* Database based session handler.
@ -33,7 +33,7 @@ defined('MOODLE_INTERNAL') || die();
* @copyright 2013 Petr Skoda {@link http://skodak.org}
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class database extends handler {
class database extends handler implements SessionHandlerInterface {
/** @var \stdClass $record session record */
protected $recordid = null;
@ -70,12 +70,7 @@ class database extends handler {
throw new exception('sessionhandlerproblem', 'error', '', null, 'Database does not support session locking');
}
$result = session_set_save_handler(array($this, 'handler_open'),
array($this, 'handler_close'),
array($this, 'handler_read'),
array($this, 'handler_write'),
array($this, 'handler_destroy'),
array($this, 'handler_gc'));
$result = session_set_save_handler($this);
if (!$result) {
throw new exception('dbsessionhandlerproblem', 'error');
}
@ -117,11 +112,11 @@ class database extends handler {
*
* {@see http://php.net/manual/en/function.session-set-save-handler.php}
*
* @param string $save_path
* @param string $session_name
* @param string $path
* @param string $name
* @return bool success
*/
public function handler_open($save_path, $session_name) {
public function open(string $path, string $name): bool {
// Note: we use the already open database.
return true;
}
@ -133,7 +128,7 @@ class database extends handler {
*
* @return bool success
*/
public function handler_close() {
public function close(): bool {
if ($this->recordid) {
try {
$this->database->release_session_lock($this->recordid);
@ -152,9 +147,9 @@ class database extends handler {
* {@see http://php.net/manual/en/function.session-set-save-handler.php}
*
* @param string $sid
* @return string
* @return string|false
*/
public function handler_read($sid) {
public function read(string $sid): string|false {
try {
if (!$record = $this->database->get_record('sessions', array('sid'=>$sid), 'id')) {
// Let's cheat and skip locking if this is the first access,
@ -221,17 +216,18 @@ class database extends handler {
* NOTE: Do not write to output or throw any exceptions!
* Hopefully the next page is going to display nice error or it recovers...
*
* @param string $sid
* @param string $session_data
* @param string $id
* @param string $data
* @return bool success
*/
public function handler_write($sid, $session_data) {
public function write(string $id, string $data): bool {
if ($this->failed) {
// Do not write anything back - we failed to start the session properly.
return false;
}
$sessdata = base64_encode($session_data); // There might be some binary mess :-(
// There might be some binary mess.
$sessdata = base64_encode($data);
$hash = sha1($sessdata);
if ($hash === $this->lasthash) {
@ -240,14 +236,17 @@ class database extends handler {
try {
if ($this->recordid) {
$this->database->set_field('sessions', 'sessdata', $sessdata, array('id'=>$this->recordid));
$this->database->set_field('sessions', 'sessdata', $sessdata, ['id' => $this->recordid]);
} else {
// This happens in the first request when session record was just created in manager.
$this->database->set_field('sessions', 'sessdata', $sessdata, array('sid'=>$sid));
$this->database->set_field('sessions', 'sessdata', $sessdata, ['sid' => $id]);
}
} catch (\Exception $ex) {
// Do not rethrow exceptions here, this should not happen.
error_log('Unknown exception when writing database session data : '.$sid.' - '.$ex->getMessage());
// phpcs:ignore moodle.PHP.ForbiddenFunctions.FoundWithAlternative
error_log(
"Unknown exception when writing database session data : {$id} - " . $ex->getMessage(),
);
}
return true;
@ -258,19 +257,19 @@ class database extends handler {
*
* {@see http://php.net/manual/en/function.session-set-save-handler.php}
*
* @param string $sid
* @param string $id
* @return bool success
*/
public function handler_destroy($sid) {
if (!$session = $this->database->get_record('sessions', array('sid'=>$sid), 'id, sid')) {
if ($sid == session_id()) {
public function destroy(string $id): bool {
if (!$session = $this->database->get_record('sessions', ['sid' => $id], 'id, sid')) {
if ($id == session_id()) {
$this->recordid = null;
$this->lasthash = null;
}
return true;
}
if ($this->recordid and $session->id == $this->recordid) {
if ($this->recordid && ($session->id == $this->recordid)) {
try {
$this->database->release_session_lock($this->recordid);
} catch (\Exception $ex) {
@ -280,7 +279,7 @@ class database extends handler {
$this->lasthash = null;
}
$this->database->delete_records('sessions', array('id'=>$session->id));
$this->database->delete_records('sessions', ['id' => $session->id]);
return true;
}
@ -290,16 +289,19 @@ class database extends handler {
*
* {@see http://php.net/manual/en/function.session-set-save-handler.php}
*
* @param int $ignored_maxlifetime moodle uses special timeout rules
* @param int $max_lifetime moodle uses special timeout rules
* @return bool success
*/
public function handler_gc($ignored_maxlifetime) {
// phpcs:ignore moodle.NamingConventions.ValidVariableName.VariableNameUnderscore
public function gc(int $max_lifetime): int|false {
// This should do something only if cron is not running properly...
if (!$stalelifetime = ini_get('session.gc_maxlifetime')) {
return true;
return false;
}
$params = array('purgebefore' => (time() - $stalelifetime));
$params = ['purgebefore' => (time() - $stalelifetime)];
$count = $this->database->count_records_select('sessions', 'userid = 0 AND timemodified < :purgebefore', $params);
$this->database->delete_records_select('sessions', 'userid = 0 AND timemodified < :purgebefore', $params);
return true;
return $count;
}
}

View File

@ -25,8 +25,7 @@
namespace core\session;
use RedisException;
defined('MOODLE_INTERNAL') || die();
use SessionHandlerInterface;
/**
* Redis based session handler.
@ -39,7 +38,7 @@ defined('MOODLE_INTERNAL') || die();
* @copyright 2016 Russell Smith
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class redis extends handler {
class redis extends handler implements SessionHandlerInterface {
/**
* Compressor: none.
*/
@ -204,12 +203,8 @@ class redis extends handler {
$this->connection = new \Redis();
$result = session_set_save_handler(array($this, 'handler_open'),
array($this, 'handler_close'),
array($this, 'handler_read'),
array($this, 'handler_write'),
array($this, 'handler_destroy'),
array($this, 'handler_gc'));
$result = session_set_save_handler($this);
if (!$result) {
throw new exception('redissessionhandlerproblem', 'error');
}
@ -288,11 +283,11 @@ class redis extends handler {
/**
* Update our session search path to include session name when opened.
*
* @param string $savepath unused session save path. (ignored)
* @param string $sessionname Session name for this session. (ignored)
* @param string $path unused session save path. (ignored)
* @param string $name Session name for this session. (ignored)
* @return bool true always as we will succeed.
*/
public function handler_open($savepath, $sessionname) {
public function open(string $path, string $name): bool {
return true;
}
@ -301,7 +296,7 @@ class redis extends handler {
*
* @return bool true on success. false on unable to unlock sessions.
*/
public function handler_close() {
public function close(): bool {
$this->lasthash = null;
try {
foreach ($this->locks as $id => $expirytime) {
@ -317,6 +312,7 @@ class redis extends handler {
return true;
}
/**
* Read the session data from storage
*
@ -325,7 +321,7 @@ class redis extends handler {
*
* @throws RedisException when we are unable to talk to the Redis server.
*/
public function handler_read($id) {
public function read(string $id): string|false {
try {
if ($this->requires_write_lock()) {
$this->lock_session($id);
@ -402,7 +398,7 @@ class redis extends handler {
* @param string $data session data
* @return bool true on write success, false on failure
*/
public function handler_write($id, $data) {
public function write(string $id, string $data): bool {
$hash = sha1(base64_encode($data));
@ -438,7 +434,7 @@ class redis extends handler {
* @param string $id the session id to destroy.
* @return bool true if the session was deleted, false otherwise.
*/
public function handler_destroy($id) {
public function destroy(string $id): bool {
$this->lasthash = null;
try {
$this->connection->del($id);
@ -454,11 +450,12 @@ class redis extends handler {
/**
* Garbage collect sessions. We don't we any as Redis does it for us.
*
* @param integer $maxlifetime All sessions older than this should be removed.
* @param integer $max_lifetime All sessions older than this should be removed.
* @return bool true, as Redis handles expiry for us.
*/
public function handler_gc($maxlifetime) {
return true;
// phpcs:ignore moodle.NamingConventions.ValidVariableName.VariableNameUnderscore
public function gc(int $max_lifetime): int|false {
return false;
}
/**
@ -601,7 +598,7 @@ class redis extends handler {
$rs = $DB->get_recordset('sessions', array(), 'id DESC', 'id, sid');
foreach ($rs as $record) {
$this->handler_destroy($record->sid);
$this->destroy($record->sid);
}
$rs->close();
}
@ -616,6 +613,6 @@ class redis extends handler {
return;
}
$this->handler_destroy($sid);
$this->destroy($sid);
}
}

View File

@ -562,7 +562,7 @@ class dml_read_slave_test extends \base_testcase {
$this->assertNull($DB->get_dbhwrite());
$session = new \core\session\database();
$session->handler_read('dummy');
$session->read('dummy');
$this->assertEquals(0, $DB->perf_get_reads_slave());
$this->assertTrue($DB->perf_get_reads() > 0);

View File

@ -113,24 +113,24 @@ class session_redis_test extends \advanced_testcase {
$sess = new \core\session\redis();
$sess->set_requires_write_lock(false);
$sess->init();
$this->assertSame('', $sess->handler_read('sess1'));
$this->assertTrue($sess->handler_close());
$this->assertSame('', $sess->read('sess1'));
$this->assertTrue($sess->close());
}
public function test_normal_session_start_stop_works() {
$sess = new \core\session\redis();
$sess->init();
$sess->set_requires_write_lock(true);
$this->assertTrue($sess->handler_open('Not used', 'Not used'));
$this->assertSame('', $sess->handler_read('sess1'));
$this->assertTrue($sess->handler_write('sess1', 'DATA'));
$this->assertTrue($sess->handler_close());
$this->assertTrue($sess->open('Not used', 'Not used'));
$this->assertSame('', $sess->read('sess1'));
$this->assertTrue($sess->write('sess1', 'DATA'));
$this->assertTrue($sess->close());
// Read the session again to ensure locking did what it should.
$this->assertTrue($sess->handler_open('Not used', 'Not used'));
$this->assertSame('DATA', $sess->handler_read('sess1'));
$this->assertTrue($sess->handler_write('sess1', 'DATA-new'));
$this->assertTrue($sess->handler_close());
$this->assertTrue($sess->open('Not used', 'Not used'));
$this->assertSame('DATA', $sess->read('sess1'));
$this->assertTrue($sess->write('sess1', 'DATA-new'));
$this->assertTrue($sess->close());
$this->assertSessionNoLocks();
}
@ -141,18 +141,18 @@ class session_redis_test extends \advanced_testcase {
$sess = new \core\session\redis();
$sess->init();
$this->assertTrue($sess->handler_write('sess1', 'DATA'));
$this->assertSame('DATA', $sess->handler_read('sess1'));
$this->assertTrue($sess->handler_close());
$this->assertTrue($sess->write('sess1', 'DATA'));
$this->assertSame('DATA', $sess->read('sess1'));
$this->assertTrue($sess->close());
if (extension_loaded('zstd')) {
$CFG->session_redis_compressor = \core\session\redis::COMPRESSION_ZSTD;
$sess = new \core\session\redis();
$sess->init();
$this->assertTrue($sess->handler_write('sess2', 'DATA'));
$this->assertSame('DATA', $sess->handler_read('sess2'));
$this->assertTrue($sess->handler_close());
$this->assertTrue($sess->write('sess2', 'DATA'));
$this->assertSame('DATA', $sess->read('sess2'));
$this->assertTrue($sess->close());
}
$CFG->session_redis_compressor = \core\session\redis::COMPRESSION_NONE;
@ -162,25 +162,25 @@ class session_redis_test extends \advanced_testcase {
$sess = new \core\session\redis();
$sess->init();
$sess->set_requires_write_lock(true);
$this->assertTrue($sess->handler_open('Not used', 'Not used'));
$this->assertSame('', $sess->handler_read('sess1'));
$this->assertTrue($sess->handler_write('sess1', 'DATA'));
$this->assertTrue($sess->handler_close());
$this->assertTrue($sess->open('Not used', 'Not used'));
$this->assertSame('', $sess->read('sess1'));
$this->assertTrue($sess->write('sess1', 'DATA'));
$this->assertTrue($sess->close());
// Sessions are not locked until they have been saved once.
$this->assertTrue($sess->handler_open('Not used', 'Not used'));
$this->assertSame('DATA', $sess->handler_read('sess1'));
$this->assertTrue($sess->open('Not used', 'Not used'));
$this->assertSame('DATA', $sess->read('sess1'));
$sessblocked = new \core\session\redis();
$sessblocked->init();
$sessblocked->set_requires_write_lock(true);
$this->assertTrue($sessblocked->handler_open('Not used', 'Not used'));
$this->assertTrue($sessblocked->open('Not used', 'Not used'));
// Trap the error log and send it to stdOut so we can expect output at the right times.
$errorlog = tempnam(sys_get_temp_dir(), "rediserrorlog");
$this->iniSet('error_log', $errorlog);
try {
$sessblocked->handler_read('sess1');
$sessblocked->read('sess1');
$this->fail('Session lock must fail to be obtained.');
} catch (\core\session\exception $e) {
$this->assertStringContainsString("Unable to obtain lock for session id sess1", $e->getMessage());
@ -189,9 +189,9 @@ class session_redis_test extends \advanced_testcase {
$this->assertStringContainsString('Cannot obtain session lock for sid: sess1', file_get_contents($errorlog));
}
$this->assertTrue($sessblocked->handler_close());
$this->assertTrue($sess->handler_write('sess1', 'DATA-new'));
$this->assertTrue($sess->handler_close());
$this->assertTrue($sessblocked->close());
$this->assertTrue($sess->write('sess1', 'DATA-new'));
$this->assertTrue($sess->close());
$this->assertSessionNoLocks();
}
@ -199,8 +199,8 @@ class session_redis_test extends \advanced_testcase {
$sess = new \core\session\redis();
$sess->init();
$sess->set_requires_write_lock(true);
$this->assertTrue($sess->handler_open('Not used', 'Not used'));
$this->assertTrue($sess->handler_destroy('sess-destroy'));
$this->assertTrue($sess->open('Not used', 'Not used'));
$this->assertTrue($sess->destroy('sess-destroy'));
$this->assertSessionNoLocks();
}
@ -208,10 +208,10 @@ class session_redis_test extends \advanced_testcase {
$sess = new \core\session\redis();
$sess->init();
$sess->set_requires_write_lock(true);
$this->assertTrue($sess->handler_open('Not used', 'Not used'));
$this->assertSame('', $sess->handler_read('sess-destroy'));
$this->assertTrue($sess->handler_destroy('sess-destroy'));
$this->assertTrue($sess->handler_close());
$this->assertTrue($sess->open('Not used', 'Not used'));
$this->assertSame('', $sess->read('sess-destroy'));
$this->assertTrue($sess->destroy('sess-destroy'));
$this->assertTrue($sess->close());
$this->assertSessionNoLocks();
}
@ -224,36 +224,36 @@ class session_redis_test extends \advanced_testcase {
$sess2->init();
// Initialize session 1.
$this->assertTrue($sess1->handler_open('Not used', 'Not used'));
$this->assertSame('', $sess1->handler_read('sess1'));
$this->assertTrue($sess1->handler_write('sess1', 'DATA'));
$this->assertTrue($sess1->handler_close());
$this->assertTrue($sess1->open('Not used', 'Not used'));
$this->assertSame('', $sess1->read('sess1'));
$this->assertTrue($sess1->write('sess1', 'DATA'));
$this->assertTrue($sess1->close());
// Initialize session 2.
$this->assertTrue($sess2->handler_open('Not used', 'Not used'));
$this->assertSame('', $sess2->handler_read('sess2'));
$this->assertTrue($sess2->handler_write('sess2', 'DATA2'));
$this->assertTrue($sess2->handler_close());
$this->assertTrue($sess2->open('Not used', 'Not used'));
$this->assertSame('', $sess2->read('sess2'));
$this->assertTrue($sess2->write('sess2', 'DATA2'));
$this->assertTrue($sess2->close());
// Open and read session 1 and 2.
$this->assertTrue($sess1->handler_open('Not used', 'Not used'));
$this->assertSame('DATA', $sess1->handler_read('sess1'));
$this->assertTrue($sess2->handler_open('Not used', 'Not used'));
$this->assertSame('DATA2', $sess2->handler_read('sess2'));
$this->assertTrue($sess1->open('Not used', 'Not used'));
$this->assertSame('DATA', $sess1->read('sess1'));
$this->assertTrue($sess2->open('Not used', 'Not used'));
$this->assertSame('DATA2', $sess2->read('sess2'));
// Write both sessions.
$this->assertTrue($sess1->handler_write('sess1', 'DATAX'));
$this->assertTrue($sess2->handler_write('sess2', 'DATA2X'));
$this->assertTrue($sess1->write('sess1', 'DATAX'));
$this->assertTrue($sess2->write('sess2', 'DATA2X'));
// Read both sessions.
$this->assertTrue($sess1->handler_open('Not used', 'Not used'));
$this->assertTrue($sess2->handler_open('Not used', 'Not used'));
$this->assertEquals('DATAX', $sess1->handler_read('sess1'));
$this->assertEquals('DATA2X', $sess2->handler_read('sess2'));
$this->assertTrue($sess1->open('Not used', 'Not used'));
$this->assertTrue($sess2->open('Not used', 'Not used'));
$this->assertEquals('DATAX', $sess1->read('sess1'));
$this->assertEquals('DATA2X', $sess2->read('sess2'));
// Close both sessions
$this->assertTrue($sess1->handler_close());
$this->assertTrue($sess2->handler_close());
$this->assertTrue($sess1->close());
$this->assertTrue($sess2->close());
// Read the session again to ensure locking did what it should.
$this->assertSessionNoLocks();
@ -265,19 +265,19 @@ class session_redis_test extends \advanced_testcase {
$sess->set_requires_write_lock(true);
// Initialize session 1.
$this->assertTrue($sess->handler_open('Not used', 'Not used'));
$this->assertSame('', $sess->handler_read('sess1'));
$this->assertTrue($sess->handler_write('sess1', 'DATA'));
$this->assertSame('', $sess->handler_read('sess2'));
$this->assertTrue($sess->handler_write('sess2', 'DATA2'));
$this->assertSame('DATA', $sess->handler_read('sess1'));
$this->assertSame('DATA2', $sess->handler_read('sess2'));
$this->assertTrue($sess->handler_destroy('sess2'));
$this->assertTrue($sess->open('Not used', 'Not used'));
$this->assertSame('', $sess->read('sess1'));
$this->assertTrue($sess->write('sess1', 'DATA'));
$this->assertSame('', $sess->read('sess2'));
$this->assertTrue($sess->write('sess2', 'DATA2'));
$this->assertSame('DATA', $sess->read('sess1'));
$this->assertSame('DATA2', $sess->read('sess2'));
$this->assertTrue($sess->destroy('sess2'));
$this->assertTrue($sess->handler_close());
$this->assertTrue($sess->close());
$this->assertSessionNoLocks();
$this->assertTrue($sess->handler_close());
$this->assertTrue($sess->close());
}
public function test_session_exists_returns_valid_values() {
@ -285,13 +285,13 @@ class session_redis_test extends \advanced_testcase {
$sess->init();
$sess->set_requires_write_lock(true);
$this->assertTrue($sess->handler_open('Not used', 'Not used'));
$this->assertSame('', $sess->handler_read('sess1'));
$this->assertTrue($sess->open('Not used', 'Not used'));
$this->assertSame('', $sess->read('sess1'));
$this->assertFalse($sess->session_exists('sess1'), 'Session must not exist yet, it has not been saved');
$this->assertTrue($sess->handler_write('sess1', 'DATA'));
$this->assertTrue($sess->write('sess1', 'DATA'));
$this->assertTrue($sess->session_exists('sess1'), 'Session must exist now.');
$this->assertTrue($sess->handler_destroy('sess1'));
$this->assertTrue($sess->destroy('sess1'));
$this->assertFalse($sess->session_exists('sess1'), 'Session should be destroyed.');
}
@ -301,10 +301,10 @@ class session_redis_test extends \advanced_testcase {
$sess = new \core\session\redis();
$sess->init();
$this->assertTrue($sess->handler_open('Not used', 'Not used'));
$this->assertTrue($sess->handler_write('sess1', 'DATA'));
$this->assertTrue($sess->handler_write('sess2', 'DATA'));
$this->assertTrue($sess->handler_write('sess3', 'DATA'));
$this->assertTrue($sess->open('Not used', 'Not used'));
$this->assertTrue($sess->write('sess1', 'DATA'));
$this->assertTrue($sess->write('sess2', 'DATA'));
$this->assertTrue($sess->write('sess3', 'DATA'));
$sessiondata = new \stdClass();
$sessiondata->userid = 2;
@ -318,9 +318,9 @@ class session_redis_test extends \advanced_testcase {
$sessiondata->sid = 'sess3';
$DB->insert_record('sessions', $sessiondata);
$this->assertNotEquals('', $sess->handler_read('sess1'));
$this->assertNotEquals('', $sess->read('sess1'));
$sess->kill_session('sess1');
$this->assertEquals('', $sess->handler_read('sess1'));
$this->assertEquals('', $sess->read('sess1'));
$this->assertEmpty($this->redis->keys($this->keyprefix.'sess1.lock'));