Skip to content

Commit

Permalink
MDL-72283 caching: internationalize redis session handler error
Browse files Browse the repository at this point in the history
The 'unable to obtain session lock'-exception raised by the Redis
session handler is hardcoded in English and not all that useful
to the end user.

This change adds the error message to the lang/error.php and gives
the user further hints why the error might have occured and how it
could be fixed.

Signed-off-by: Daniel Ziegenberg <daniel@ziegenberg.at>
  • Loading branch information
ziegenberg committed May 23, 2022
1 parent 6c114e2 commit 0130924
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 4 deletions.
3 changes: 3 additions & 0 deletions lang/en/error.php
Expand Up @@ -532,6 +532,9 @@
$string['sessionipnomatch'] = 'Sorry, but your IP number seems to have changed from when you first logged in. This security feature prevents crackers stealing your identity while logged in to this site. Normal users should not be seeing this message - please ask the site administrator for help.';
$string['sessionipnomatch2'] = '<p>Sorry, but your IP number seems to have changed from when you first logged in. This security feature prevents crackers stealing your identity while logged in to this site. You may see this error if you use wireless networks or if you are roaming between different networks. Please ask the site administrator for more help.</p>
<p>If you want to continue please press F5 key to refresh this page.</p>';
$string['sessioncannotobtainlock'] = '<p>Unable to obtain lock for session id {$a->id} within {$a->acquiretimeout}.</p>
<p>It is likely that another page ({$a->whohaslock}) is still running in another browser tab, or it did not release the lock due to an error.</p>
<p>You can wait until the session lock timeout ({$a->lockexpire}) or you can restart your browser session. If this error persists, please notify the server administrator.</p>';
$string['shortnametaken'] = 'Short name is already used for another course ({$a})';
$string['sitepolicynotagreed'] = 'Site policy not agreed: <a href="{$a}">Click here to open the site policy.</a>';
$string['scheduledbackupsdisabled'] = 'Scheduled backups have been disabled by the server admin';
Expand Down
9 changes: 8 additions & 1 deletion lib/classes/session/redis.php
Expand Up @@ -479,7 +479,14 @@ protected function lock_session($id) {
// phpcs:ignore
error_log("Cannot obtain session lock for sid: $id within $this->acquiretimeout seconds. " .
"It is likely another page ($whohaslock) has a long session lock, or the session lock was never released.");
throw new exception("Unable to obtain session lock");
$acquiretimeout = format_time($this->acquiretimeout);
$lockexpire = format_time($this->lockexpire);
$a = (object)[
'id' => substr($id, 0, 10),
'acquiretimeout' => $acquiretimeout,
'whohaslock' => $whohaslock,
'lockexpire' => $lockexpire];
throw new exception("sessioncannotobtainlock", 'error', '', $a);
}

if ($this->time() < $startlocktime + 5) {
Expand Down
13 changes: 10 additions & 3 deletions lib/tests/session_redis_test.php
Expand Up @@ -46,6 +46,11 @@ class core_session_redis_testcase extends advanced_testcase {
protected $keyprefix = null;
/** @var $redis The current testing redis connection */
protected $redis = null;
/** @var int $acquiretimeout how long we wait for session lock in seconds when testing Redis */
protected $acquiretimeout = 1;
/** @var int $lockexpire how long to wait in seconds before expiring the lock when testing Redis */
protected $lockexpire = 70;


public function setUp(): void {
global $CFG;
Expand All @@ -72,8 +77,8 @@ public function setUp(): void {

// Set a very short lock timeout to ensure tests run quickly. We are running single threaded,
// so unless we lock and expect it to be there, we will always see a lock.
$CFG->session_redis_acquire_lock_timeout = 1;
$CFG->session_redis_lock_expire = 70;
$CFG->session_redis_acquire_lock_timeout = $this->acquiretimeout;
$CFG->session_redis_lock_expire = $this->lockexpire;

$this->redis = new Redis();
$this->redis->connect(TEST_SESSION_REDIS_HOST);
Expand Down Expand Up @@ -165,7 +170,9 @@ public function test_session_blocks_with_existing_session() {
$sessblocked->handler_read('sess1');
$this->fail('Session lock must fail to be obtained.');
} catch (\core\session\exception $e) {
$this->assertStringContainsString("Unable to obtain session lock", $e->getMessage());
$this->assertStringContainsString("Unable to obtain lock for session id sess1", $e->getMessage());
$this->assertStringContainsString('within 1 sec.', $e->getMessage());
$this->assertStringContainsString('session lock timeout (1 min 10 secs) ', $e->getMessage());
$this->assertStringContainsString('Cannot obtain session lock for sid: sess1', file_get_contents($errorlog));
}

Expand Down

0 comments on commit 0130924

Please sign in to comment.