Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

SessionManager: Don't save non-persisted sessions to backend storage

Browse source 
This introduces an in-process cache (using a HashBagOStuff) for session
data, and only saves to the external cache when the session is
persisted.

Bug: T125267
Change-Id: Ie161e0f7522cd68515b060ad8cf8c151b7198b0b
This commit is contained in:
Brad Jorsch 2016-01-29 18:02:11 -05:00 committed by BryanDavis
parent 982869d6b0
commit fcdd643a46
7 changed files with 93 additions and 37 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

37
includes/session/SessionBackend.php
View file

@ -65,7 +65,9 @@ final class SessionBackend {
private $dataHash = null;
/** @var BagOStuff */
private $store;
private $tempStore;
/** @var BagOStuff */
private $permStore;
/** @var LoggerInterface */
private $logger;
@ -97,12 +99,14 @@ final class SessionBackend {
/**
* @param SessionId $id Session ID object
* @param SessionInfo $info Session info to populate from
* @param BagOStuff $store Backend data store
* @param BagOStuff $tempStore In-process data store
* @param BagOStuff $permstore Backend data store for persisted sessions
* @param LoggerInterface $logger
* @param int $lifetime Session data lifetime in seconds
*/
public function __construct(
SessionId $id, SessionInfo $info, BagOStuff $store, LoggerInterface $logger, $lifetime
SessionId $id, SessionInfo $info, BagOStuff $tempStore, BagOStuff $permStore,
LoggerInterface $logger, $lifetime
) {
$phpSessionHandling = \RequestContext::getMain()->getConfig()->get( 'PHPSessionHandling' );
$this->usePhpSessionHandling = $phpSessionHandling !== 'disable';
@ -121,7 +125,8 @@ final class SessionBackend {
$this->id = $id;
$this->user = $info->getUserInfo() ? $info->getUserInfo()->getUser() : new User;
$this->store = $store;
$this->tempStore = $tempStore;
$this->permStore = $permStore;
$this->logger = $logger;
$this->lifetime = $lifetime;
$this->provider = $info->getProvider();
@ -130,7 +135,14 @@ final class SessionBackend {
$this->forceHTTPS = $info->forceHTTPS();
$this->providerMetadata = $info->getProviderMetadata();
$blob = $store->get( wfMemcKey( 'MWSession', (string)$this->id ) );
$key = wfMemcKey( 'MWSession', (string)$this->id );
$blob = $tempStore->get( $key );
if ( $blob === false ) {
$blob = $permStore->get( $key );
if ( $blob !== false ) {
$tempStore->set( $key, $blob );
}
}
if ( !is_array( $blob ) ||
!isset( $blob['metadata'] ) || !is_array( $blob['metadata'] ) ||
!isset( $blob['data'] ) || !is_array( $blob['data'] )
@ -229,7 +241,8 @@ final class SessionBackend {
$this->autosave();
// Delete the data for the old session ID now
$this->store->delete( wfMemcKey( 'MWSession', $oldId ) );
$this->tempStore->delete( wfMemcKey( 'MWSession', $oldId ) );
$this->permStore->delete( wfMemcKey( 'MWSession', $oldId ) );
}
}
@ -613,7 +626,7 @@ final class SessionBackend {
}
}
$this->store->set(
$this->tempStore->set(
wfMemcKey( 'MWSession', (string)$this->id ),
array(
'data' => $this->data,
@ -621,6 +634,16 @@ final class SessionBackend {
),
$metadata['expires']
);
if ( $this->persist ) {
$this->permStore->set(
wfMemcKey( 'MWSession', (string)$this->id ),
array(
'data' => $this->data,
'metadata' => $metadata,
),
$metadata['expires']
);
}
$this->metaDirty = false;
$this->dataDirty = false;

59
includes/session/SessionManager.php
View file

@ -55,7 +55,10 @@ final class SessionManager implements SessionManagerInterface {
private $config;
/** @var BagOStuff|null */
private $store;
private $tempStore;
/** @var BagOStuff|null */
private $permStore;
/** @var SessionProvider[] */
private $sessionProviders = null;
@ -159,16 +162,17 @@ final class SessionManager implements SessionManagerInterface {
$this->setLogger( \MediaWiki\Logger\LoggerFactory::getInstance( 'session' ) );
}
$this->tempStore = new \HashBagOStuff;
if ( isset( $options['store'] ) ) {
if ( !$options['store'] instanceof BagOStuff ) {
throw new \InvalidArgumentException(
'$options[\'store\'] must be an instance of BagOStuff'
);
}
$this->store = $options['store'];
$this->permStore = $options['store'];
} else {
$this->store = \ObjectCache::getInstance( $this->config->get( 'SessionCacheType' ) );
$this->store->setLogger( $this->logger );
$this->permStore = \ObjectCache::getInstance( $this->config->get( 'SessionCacheType' ) );
$this->permStore->setLogger( $this->logger );
}
register_shutdown_function( array( $this, 'shutdown' ) );
@ -202,7 +206,14 @@ final class SessionManager implements SessionManagerInterface {
// Test this here to provide a better log message for the common case
// of "no such ID"
$key = wfMemcKey( 'MWSession', $id );
if ( is_array( $this->store->get( $key ) ) ) {
$existing = $this->tempStore->get( $key );
if ( $existing === false ) {
$existing = $this->permStore->get( $key );
if ( $existing !== false ) {
$this->tempStore->set( $key, $existing );
}
}
if ( is_array( $existing ) ) {
$info = new SessionInfo( SessionInfo::MIN_PRIORITY, array( 'id' => $id, 'idIsSafe' => true ) );
if ( $this->loadSessionInfoFromStore( $info, $request ) ) {
$session = $this->getSessionFromInfo( $info, $request );
@ -240,7 +251,14 @@ final class SessionManager implements SessionManagerInterface {
}
$key = wfMemcKey( 'MWSession', $id );
if ( is_array( $this->store->get( $key ) ) ) {
$existing = $this->tempStore->get( $key );
if ( $existing === false ) {
$existing = $this->permStore->get( $key );
if ( $existing !== false ) {
$this->tempStore->set( $key, $existing );
}
}
if ( is_array( $existing ) ) {
throw new \InvalidArgumentException( 'Session ID already exists' );
}
}
@ -660,7 +678,13 @@ final class SessionManager implements SessionManagerInterface {
*/
private function loadSessionInfoFromStore( SessionInfo &$info, WebRequest $request ) {
$key = wfMemcKey( 'MWSession', $info->getId() );
$blob = $this->store->get( $key );
$blob = $this->tempStore->get( $key );
if ( $blob === false ) {
$blob = $this->permStore->get( $key );
if ( $blob !== false ) {
$this->tempStore->set( $key, $blob );
}
}
$newParams = array();
@ -668,7 +692,8 @@ final class SessionManager implements SessionManagerInterface {
// Sanity check: blob must be an array, if it's saved at all
if ( !is_array( $blob ) ) {
$this->logger->warning( "Session $info: Bad data" );
$this->store->delete( $key );
$this->tempStore->delete( $key );
$this->permStore->delete( $key );
return false;
}
@ -677,7 +702,8 @@ final class SessionManager implements SessionManagerInterface {
!isset( $blob['metadata'] ) || !is_array( $blob['metadata'] )
) {
$this->logger->warning( "Session $info: Bad data structure" );
$this->store->delete( $key );
$this->tempStore->delete( $key );
$this->permStore->delete( $key );
return false;
}
@ -692,7 +718,8 @@ final class SessionManager implements SessionManagerInterface {
!array_key_exists( 'provider', $metadata )
) {
$this->logger->warning( "Session $info: Bad metadata" );
$this->store->delete( $key );
$this->tempStore->delete( $key );
$this->permStore->delete( $key );
return false;
}
@ -702,7 +729,8 @@ final class SessionManager implements SessionManagerInterface {
$newParams['provider'] = $provider = $this->getProvider( $metadata['provider'] );
if ( !$provider ) {
$this->logger->warning( "Session $info: Unknown provider, " . $metadata['provider'] );
$this->store->delete( $key );
$this->tempStore->delete( $key );
$this->permStore->delete( $key );
return false;
}
} elseif ( $metadata['provider'] !== (string)$provider ) {
@ -893,7 +921,8 @@ final class SessionManager implements SessionManagerInterface {
$backend = new SessionBackend(
$this->allSessionIds[$id],
$info,
$this->store,
$this->tempStore,
$this->permStore,
$this->logger,
$this->config->get( 'ObjectCacheSessionExpiry' )
);
@ -970,7 +999,9 @@ final class SessionManager implements SessionManagerInterface {
do {
$id = wfBaseConvert( \MWCryptRand::generateHex( 40 ), 16, 32, 32 );
$key = wfMemcKey( 'MWSession', $id );
} while ( isset( $this->allSessionIds[$id] ) || is_array( $this->store->get( $key ) ) );
} while ( isset( $this->allSessionIds[$id] ) ||
is_array( $this->tempStore->get( $key ) ) || is_array( $this->permStore->get( $key ) )
);
return $id;
}
@ -980,7 +1011,7 @@ final class SessionManager implements SessionManagerInterface {
* @param PHPSessionHandler $handler
*/
public function setupPHPSessionHandler( PHPSessionHandler $handler ) {
$handler->setManager( $this, $this->store, $this->logger );
$handler->setManager( $this, $this->permStore, $this->logger );
}
/**

3
tests/phpunit/includes/session/CookieSessionProviderTest.php
View file

@ -363,6 +363,7 @@ class CookieSessionProviderTest extends MediaWikiTestCase {
'idIsSafe' => true,
) ),
$store,
$store,
new \Psr\Log\NullLogger(),
10
);
@ -449,6 +450,7 @@ class CookieSessionProviderTest extends MediaWikiTestCase {
'idIsSafe' => true,
) ),
new \EmptyBagOStuff(),
new \EmptyBagOStuff(),
new \Psr\Log\NullLogger(),
10
);
@ -553,6 +555,7 @@ class CookieSessionProviderTest extends MediaWikiTestCase {
'idIsSafe' => true,
) ),
$store,
$store,
new \Psr\Log\NullLogger(),
10
);

1
tests/phpunit/includes/session/ImmutableSessionProviderWithCookieTest.php
View file

@ -205,6 +205,7 @@ class ImmutableSessionProviderWithCookieTest extends MediaWikiTestCase {
'idIsSafe' => true,
) ),
new \EmptyBagOStuff(),
new \EmptyBagOStuff(),
new \Psr\Log\NullLogger(),
10
);

8
tests/phpunit/includes/session/PHPSessionHandlerTest.php
View file

@ -172,14 +172,6 @@ class PHPSessionHandlerTest extends MediaWikiTestCase {
$this->assertSame( $expect, $_SESSION );
}
// Test expiry
session_write_close();
ini_set( 'session.gc_divisor', 1 );
ini_set( 'session.gc_probability', 1 );
sleep( 3 );
session_start();
$this->assertSame( array(), $_SESSION );
// Re-fill the session, then test that session_destroy() works.
$_SESSION['AuthenticationSessionTest'] = $rand;
session_write_close();

12
tests/phpunit/includes/session/SessionBackendTest.php
View file

@ -59,7 +59,7 @@ class SessionBackendTest extends MediaWikiTestCase {
) );
$id = new SessionId( $info->getId() );
$backend = new SessionBackend( $id, $info, $this->store, $logger, 10 );
$backend = new SessionBackend( $id, $info, $this->store, $this->store, $logger, 10 );
$priv = \TestingAccessWrapper::newFromObject( $backend );
$priv->persist = false;
$priv->requests = array( 100 => new \FauxRequest() );
@ -87,7 +87,7 @@ class SessionBackendTest extends MediaWikiTestCase {
$id = new SessionId( $info->getId() );
$logger = new \Psr\Log\NullLogger();
try {
new SessionBackend( $id, $info, $this->store, $logger, 10 );
new SessionBackend( $id, $info, $this->store, $this->store, $logger, 10 );
$this->fail( 'Expected exception not thrown' );
} catch ( \InvalidArgumentException $ex ) {
$this->assertSame(
@ -103,7 +103,7 @@ class SessionBackendTest extends MediaWikiTestCase {
) );
$id = new SessionId( $info->getId() );
try {
new SessionBackend( $id, $info, $this->store, $logger, 10 );
new SessionBackend( $id, $info, $this->store, $this->store, $logger, 10 );
$this->fail( 'Expected exception not thrown' );
} catch ( \InvalidArgumentException $ex ) {
$this->assertSame( 'Cannot create session without a provider', $ex->getMessage() );
@ -118,7 +118,7 @@ class SessionBackendTest extends MediaWikiTestCase {
) );
$id = new SessionId( '!' . $info->getId() );
try {
new SessionBackend( $id, $info, $this->store, $logger, 10 );
new SessionBackend( $id, $info, $this->store, $this->store, $logger, 10 );
$this->fail( 'Expected exception not thrown' );
} catch ( \InvalidArgumentException $ex ) {
$this->assertSame(
@ -135,7 +135,7 @@ class SessionBackendTest extends MediaWikiTestCase {
'idIsSafe' => true,
) );
$id = new SessionId( $info->getId() );
$backend = new SessionBackend( $id, $info, $this->store, $logger, 10 );
$backend = new SessionBackend( $id, $info, $this->store, $this->store, $logger, 10 );
$this->assertSame( self::SESSIONID, $backend->getId() );
$this->assertSame( $id, $backend->getSessionId() );
$this->assertSame( $this->provider, $backend->getProvider() );
@ -157,7 +157,7 @@ class SessionBackendTest extends MediaWikiTestCase {
'idIsSafe' => true,
) );
$id = new SessionId( $info->getId() );
$backend = new SessionBackend( $id, $info, $this->store, $logger, 10 );
$backend = new SessionBackend( $id, $info, $this->store, $this->store, $logger, 10 );
$this->assertSame( self::SESSIONID, $backend->getId() );
$this->assertSame( $id, $backend->getSessionId() );
$this->assertSame( $this->provider, $backend->getProvider() );

10
tests/phpunit/includes/session/SessionManagerTest.php
View file

@ -103,7 +103,7 @@ class SessionManagerTest extends MediaWikiTestCase {
$manager = \TestingAccessWrapper::newFromObject( $this->getManager() );
$this->assertSame( $this->config, $manager->config );
$this->assertSame( $this->logger, $manager->logger );
$this->assertSame( $this->store, $manager->store );
$this->assertSame( $this->store, $manager->permStore );
$manager = \TestingAccessWrapper::newFromObject( new SessionManager() );
$this->assertSame( \RequestContext::getMain()->getConfig(), $manager->config );
@ -111,7 +111,7 @@ class SessionManagerTest extends MediaWikiTestCase {
$manager = \TestingAccessWrapper::newFromObject( new SessionManager( array(
'config' => $this->config,
) ) );
$this->assertSame( \ObjectCache::$instances['testSessionStore'], $manager->store );
$this->assertSame( \ObjectCache::$instances['testSessionStore'], $manager->permStore );
foreach ( array(
'config' => '$options[\'config\'] must be an instance of Config',
@ -301,6 +301,9 @@ class SessionManagerTest extends MediaWikiTestCase {
public function testGetSessionById() {
$manager = $this->getManager();
// Disable the in-process cache so our $this->store->setSession() takes effect.
\TestingAccessWrapper::newFromObject( $manager )->tempStore = new \EmptyBagOStuff;
try {
$manager->getSessionById( 'bad' );
$this->fail( 'Expected exception not thrown' );
@ -1083,6 +1086,9 @@ class SessionManagerTest extends MediaWikiTestCase {
$manager->setLogger( $logger );
$request = new \FauxRequest();
// Disable the in-process cache so our $this->store->setSession() takes effect.
\TestingAccessWrapper::newFromObject( $manager )->tempStore = new \EmptyBagOStuff;
// TestingAccessWrapper can't handle methods with reference arguments, sigh.
$rClass = new \ReflectionClass( $manager );
$rMethod = $rClass->getMethod( 'loadSessionInfoFromStore' );