Page MenuHomePhorge
Files F585111

No OneTemporary
Actions

Size
12 KB
Referenced Files
None
Subscribers
None

View Options

diff --git a/includes/session/SessionBackend.php b/includes/session/SessionBackend.php
index 4b9cb4911e6..de18e5052af 100644
--- a/includes/session/SessionBackend.php
+++ b/includes/session/SessionBackend.php
@@ -29,6 +29,7 @@ use MediaWiki\HookContainer\HookContainer;
use MediaWiki\HookContainer\HookRunner;
use MediaWiki\MainConfigNames;
use MediaWiki\MediaWikiServices;
+use MediaWiki\Request\FauxRequest;
use MediaWiki\Request\WebRequest;
use MediaWiki\User\User;
use MWRestrictions;
@@ -229,11 +230,11 @@ final class SessionBackend {
* @param int $index
*/
public function deregisterSession( $index ) {
- unset( $this->requests[$index] );
- if ( !$this->shutdown && !count( $this->requests ) ) {
+ if ( !$this->shutdown && count( $this->requests ) <= 1 ) {
$this->save( true );
$this->provider->getManager()->deregisterSessionBackend( $this );
}
+ unset( $this->requests[$index] );
}
/**
@@ -295,6 +296,11 @@ final class SessionBackend {
$this->autosave();
+ $this->logSessionWrite( [
+ 'old_session_id' => $oldId,
+ 'action' => 'delete',
+ 'reason' => 'ID reset',
+ ] );
// Delete the data for the old session ID now
$this->store->delete( $this->store->makeKey( 'MWSession', $oldId ) );
}
@@ -364,6 +370,10 @@ final class SessionBackend {
$this->forcePersist = true;
$this->metaDirty = true;
+ $this->logSessionWrite( [
+ 'action' => 'delete',
+ 'reason' => 'unpersist',
+ ] );
// Delete the session data, so the local cache-only write in
// self::save() doesn't get things out of sync with the backend.
$this->store->delete( $this->store->makeKey( 'MWSession', (string)$this->id ) );
@@ -761,6 +771,7 @@ final class SessionBackend {
}
}
+ $persistenceChangeType = $this->persistenceChangeType;
$this->forcePersist = false;
$this->persistenceChangeType = null;
@@ -791,6 +802,18 @@ final class SessionBackend {
}
}
+ if ( $this->persist ) {
+ $this->logSessionWrite( [
+ 'remember' => $metadata['remember'],
+ 'metaDirty' => $this->metaDirty,
+ 'dataDirty' => $this->dataDirty,
+ 'persistenceChangeType' => $persistenceChangeType ?? '',
+ 'action' => 'write',
+ 'reason' => 'save',
+ ] );
+
+ }
+
$flags = $this->persist ? 0 : CachedBagOStuff::WRITE_CACHE_ONLY;
$this->store->set(
$this->store->makeKey( 'MWSession', (string)$this->id ),
@@ -882,4 +905,30 @@ final class SessionBackend {
] );
}
+ /**
+ * @param array $data Additional log context. Should have at least the following keys:
+ * - action: 'write' or 'delete'.
+ * - reason: why the write happened
+ * @see SessionManager::logSessionWrite()
+ */
+ private function logSessionWrite( array $data = [] ): void {
+ $id = $this->getId();
+ $user = $this->getUser()->isAnon() ? '<anon>' : $this->getUser()->getName();
+ // No great way to find out what request SessionBackend is being called for, but it's
+ // rare to have multiple ones which are significantly different, and even rarer for the
+ // first of those not to be the real one.
+ $request = reset( $this->requests );
+ // Don't require $this->requests to be non-empty for unit tests, as it's hard to enforce
+ if ( $request === false && defined( 'MW_PHPUNIT_TEST' ) ) {
+ $request = new FauxRequest();
+ }
+ $this->logger->info( 'Session store: {action} for {reason}', $data + [
+ 'id' => $id,
+ 'provider' => get_class( $this->getProvider() ),
+ 'user' => $user,
+ 'clientip' => $request->getIP(),
+ 'userAgent' => $request->getHeader( 'user-agent' ),
+ ] );
+ }
+
}
diff --git a/includes/session/SessionManager.php b/includes/session/SessionManager.php
index e15ffb59f25..998b6efe96f 100644
--- a/includes/session/SessionManager.php
+++ b/includes/session/SessionManager.php
@@ -560,6 +560,10 @@ class SessionManager implements SessionManagerInterface {
// "fail" is just return false.
if ( $info->forceUse() && $blob !== false ) {
$failHandler = function () use ( $key, &$info, $request ) {
+ $this->logSessionWrite( $request, $info, [
+ 'type' => 'delete',
+ 'reason' => 'loadSessionInfo fail',
+ ] );
$this->store->delete( $key );
return $this->loadSessionInfoFromStore( $info, $request );
};
@@ -574,9 +578,10 @@ class SessionManager implements SessionManagerInterface {
if ( $blob !== false ) {
// Double check: blob must be an array, if it's saved at all
if ( !is_array( $blob ) ) {
- $this->logger->warning( 'Session "{session}": Bad data', [
- 'session' => $info->__toString(),
- ] );
+ $this->logSessionWrite( $request, $info, [
+ 'type' => 'delete',
+ 'reason' => 'bad data',
+ ], LogLevel::WARNING );
$this->store->delete( $key );
return $failHandler();
}
@@ -585,9 +590,10 @@ class SessionManager implements SessionManagerInterface {
if ( !isset( $blob['data'] ) || !is_array( $blob['data'] ) ||
!isset( $blob['metadata'] ) || !is_array( $blob['metadata'] )
) {
- $this->logger->warning( 'Session "{session}": Bad data structure', [
- 'session' => $info->__toString(),
- ] );
+ $this->logSessionWrite( $request, $info, [
+ 'type' => 'delete',
+ 'reason' => 'bad data structure',
+ ], LogLevel::WARNING );
$this->store->delete( $key );
return $failHandler();
}
@@ -602,9 +608,10 @@ class SessionManager implements SessionManagerInterface {
!array_key_exists( 'userToken', $metadata ) ||
!array_key_exists( 'provider', $metadata )
) {
- $this->logger->warning( 'Session "{session}": Bad metadata', [
- 'session' => $info->__toString(),
- ] );
+ $this->logSessionWrite( $request, $info, [
+ 'type' => 'delete',
+ 'reason' => 'bad metadata',
+ ], LogLevel::WARNING );
$this->store->delete( $key );
return $failHandler();
}
@@ -614,12 +621,11 @@ class SessionManager implements SessionManagerInterface {
if ( $provider === null ) {
$newParams['provider'] = $provider = $this->getProvider( $metadata['provider'] );
if ( !$provider ) {
- $this->logger->warning(
- 'Session "{session}": Unknown provider ' . $metadata['provider'],
- [
- 'session' => $info->__toString(),
- ]
- );
+ $this->logSessionWrite( $request, $info, [
+ 'type' => 'delete',
+ 'reason' => 'unknown provider',
+ 'provider' => $metadata['provider'],
+ ], LogLevel::WARNING );
$this->store->delete( $key );
return $failHandler();
}
@@ -1096,6 +1102,36 @@ class SessionManager implements SessionManagerInterface {
}
}
+ /**
+ * @param WebRequest $request
+ * @param SessionInfo $info
+ * @param array $data Additional log context. Should have at least the following keys:
+ * - type: 'write' or 'delete'.
+ * - reason: why the write happened
+ * @param string $level PSR LogLevel constant (defaults to info)
+ * @throws MWException
+ * @see SessionBackend::logSessionWrite()
+ */
+ private function logSessionWrite(
+ WebRequest $request,
+ SessionInfo $info,
+ array $data,
+ string $level = LogLevel::INFO
+ ): void {
+ $user = ( !$info->getUserInfo() || $info->getUserInfo()->isAnon() )
+ ? '<anon>'
+ : $info->getUserInfo()->getName();
+ $this->logger->log( $level, 'Session store: {action} for {reason}', $data + [
+ 'action' => $data['type'],
+ 'reason' => $data['reason'],
+ 'id' => $info->getId(),
+ 'provider' => $info->getProvider() ? get_class( $info->getProvider() ) : 'null',
+ 'user' => $user,
+ 'clientip' => $request->getIP(),
+ 'userAgent' => $request->getHeader( 'user-agent' ),
+ ] );
+ }
+
// endregion -- end of Internal methods
}
diff --git a/tests/phpunit/includes/session/PHPSessionHandlerTest.php b/tests/phpunit/includes/session/PHPSessionHandlerTest.php
index 5e930ee30d7..d024b39efee 100644
--- a/tests/phpunit/includes/session/PHPSessionHandlerTest.php
+++ b/tests/phpunit/includes/session/PHPSessionHandlerTest.php
@@ -158,6 +158,7 @@ class PHPSessionHandlerTest extends MediaWikiIntegrationTestCase {
$this->assertSame( [
[ LogLevel::DEBUG, 'SessionManager using store MediaWiki\Tests\Session\TestBagOStuff' ],
[ LogLevel::WARNING, 'Something wrote to $_SESSION!' ],
+ [ LogLevel::INFO, 'Session store: {action} for {reason}' ],
], $logger->getBuffer() );
// Screw up $_SESSION so we can tell the difference between "this
diff --git a/tests/phpunit/includes/session/SessionManagerTest.php b/tests/phpunit/includes/session/SessionManagerTest.php
index 4ee9159888d..4409d5cc49a 100644
--- a/tests/phpunit/includes/session/SessionManagerTest.php
+++ b/tests/phpunit/includes/session/SessionManagerTest.php
@@ -58,6 +58,7 @@ class SessionManagerTest extends MediaWikiIntegrationTestCase {
$this->logger = new TestLogger( false, static function ( $m ) {
return ( str_starts_with( $m, 'SessionBackend ' )
|| str_starts_with( $m, 'SessionManager using store ' )
+ || str_starts_with( $m, 'Session store: ' )
// These were added for T264793 and behave somewhat erratically, not worth testing
|| str_starts_with( $m, 'Failed to load session, unpersisting' )
|| preg_match( '/^(Persisting|Unpersisting) session (for|due to)/', $m )
@@ -1086,14 +1087,14 @@ class SessionManagerTest extends MediaWikiIntegrationTestCase {
$this->store->setRawSession( $id, true );
$this->assertFalse( $loadSessionInfoFromStore( $info ) );
$this->assertSame( [
- [ LogLevel::WARNING, 'Session "{session}": Bad data' ],
+ [ LogLevel::WARNING, 'Session store: {action} for {reason}' ],
], $logger->getBuffer() );
$logger->clearBuffer();
$this->store->setRawSession( $id, [ 'data' => [] ] );
$this->assertFalse( $loadSessionInfoFromStore( $info ) );
$this->assertSame( [
- [ LogLevel::WARNING, 'Session "{session}": Bad data structure' ],
+ [ LogLevel::WARNING, 'Session store: {action} for {reason}' ],
], $logger->getBuffer() );
$logger->clearBuffer();
@@ -1101,21 +1102,21 @@ class SessionManagerTest extends MediaWikiIntegrationTestCase {
$this->store->setRawSession( $id, [ 'metadata' => $metadata ] );
$this->assertFalse( $loadSessionInfoFromStore( $info ) );
$this->assertSame( [
- [ LogLevel::WARNING, 'Session "{session}": Bad data structure' ],
+ [ LogLevel::WARNING, 'Session store: {action} for {reason}' ],
], $logger->getBuffer() );
$logger->clearBuffer();
$this->store->setRawSession( $id, [ 'metadata' => $metadata, 'data' => true ] );
$this->assertFalse( $loadSessionInfoFromStore( $info ) );
$this->assertSame( [
- [ LogLevel::WARNING, 'Session "{session}": Bad data structure' ],
+ [ LogLevel::WARNING, 'Session store: {action} for {reason}' ],
], $logger->getBuffer() );
$logger->clearBuffer();
$this->store->setRawSession( $id, [ 'metadata' => true, 'data' => [] ] );
$this->assertFalse( $loadSessionInfoFromStore( $info ) );
$this->assertSame( [
- [ LogLevel::WARNING, 'Session "{session}": Bad data structure' ],
+ [ LogLevel::WARNING, 'Session store: {action} for {reason}' ],
], $logger->getBuffer() );
$logger->clearBuffer();
@@ -1125,7 +1126,7 @@ class SessionManagerTest extends MediaWikiIntegrationTestCase {
$this->store->setRawSession( $id, [ 'metadata' => $tmp, 'data' => [] ] );
$this->assertFalse( $loadSessionInfoFromStore( $info ) );
$this->assertSame( [
- [ LogLevel::WARNING, 'Session "{session}": Bad metadata' ],
+ [ LogLevel::WARNING, 'Session store: {action} for {reason}' ],
], $logger->getBuffer() );
$logger->clearBuffer();
}
@@ -1163,7 +1164,7 @@ class SessionManagerTest extends MediaWikiIntegrationTestCase {
] );
$this->assertFalse( $loadSessionInfoFromStore( $info ) );
$this->assertSame( [
- [ LogLevel::WARNING, 'Session "{session}": Unknown provider Bad' ],
+ [ LogLevel::WARNING, 'Session store: {action} for {reason}' ],
], $logger->getBuffer() );
$logger->clearBuffer();
@@ -1543,6 +1544,7 @@ class SessionManagerTest extends MediaWikiIntegrationTestCase {
$this->assertFalse( $this->store->getSession( $id ) );
$this->assertSame( [
[ LogLevel::WARNING, 'Session "{session}": User token mismatch' ],
+ [ LogLevel::INFO, 'Session store: {action} for {reason}' ],
], $logger->getBuffer() );
$logger->clearBuffer();
}

File Metadata

Mime Type
text/x-diff
Expires
Sat, Jul 5, 5:32 AM (11 h, 55 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
227533
Default Alt Text
(12 KB)

Event Timeline