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

SessionManager: Kill getPersistedSessionId()

Browse source 
It's not guaranteed that loadSessionFromStore() will succeed after
whatever alterations the SessionProvider might have made later in the
request.

So instead, let's make a new global object that stores the SessionId
of the persistent session that was loaded during Setup.php, if any. Then
we can check that when we need to know whether the session was
persisted.

Bug: T124468
Change-Id: I1e8e616c83b16aadd86b0a0a40826d40f6e8abe4
This commit is contained in:
Brad Jorsch 2016-01-22 14:47:33 -05:00
parent 92ba9fa33d
commit 43f904b51a
6 changed files with 17 additions and 54 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
includes/Setup.php
View file

@ -691,6 +691,11 @@ if ( !is_object( $wgAuth ) ) {
// Set up the session
$ps_session = Profiler::instance()->scopedProfileIn( $fname . '-session' );
/**
* @var MediaWiki\\Session\\SessionId|null $wgInitialSessionId The persistent
* session ID (if any) loaded at startup
*/
$wgInitialSessionId = null;
if ( !defined( 'MW_NO_SESSION' ) && !$wgCommandLineMode ) {
// If session.auto_start is there, we can't touch session name
if ( $wgPHPSessionHandling !== 'disable' && !wfIniGetBool( 'session.auto_start' ) ) {
@ -723,6 +728,10 @@ if ( !defined( 'MW_NO_SESSION' ) && !$wgCommandLineMode ) {
throw $ex;
}
if ( $session->isPersistent() ) {
$wgInitialSessionId = $session->getSessionId();
}
$session->renew();
if ( MediaWiki\Session\PHPSessionHandler::isEnabled() &&
( $session->isPersistent() || $session->shouldRememberUser() )

4
includes/WebRequest.php
View file

@ -686,8 +686,10 @@ class WebRequest {
* @return bool
*/
public function checkSessionCookie() {
global $wgInitialSessionId;
wfDeprecated( __METHOD__, '1.27' );
return SessionManager::singleton()->getPersistedSessionId( $this ) !== null;
return $wgInitialSessionId !== null &&
$this->getSession()->getId() === (string)$wgInitialSessionId;
}
/**

9
includes/session/SessionManager.php
View file

@ -178,15 +178,6 @@ final class SessionManager implements SessionManagerInterface {
$this->logger = $logger;
}
public function getPersistedSessionId( WebRequest $request ) {
$info = $this->getSessionInfoForRequest( $request );
if ( $info && $info->wasPersisted() ) {
return $info->getId();
} else {
return null;
}
}
public function getSessionForRequest( WebRequest $request ) {
$info = $this->getSessionInfoForRequest( $request );

15
includes/session/SessionManagerInterface.php
View file

@ -34,21 +34,6 @@ use WebRequest;
* @since 1.27
*/
interface SessionManagerInterface extends LoggerAwareInterface {
/**
* Fetch the persisted session ID in a request.
*
* Note this is not the same thing as whether the session associated with
* the request is currently persistent, as the session might have been
* first made persistent during this request.
*
* @param WebRequest $request
* @return string|null
* @throws \\OverflowException if there are multiple sessions tied for top
* priority in the request. Exception has a property "sessionInfos"
* holding the SessionInfo objects for the sessions involved.
*/
public function getPersistedSessionId( WebRequest $request );
/**
* Fetch the session for a request
*

8
includes/specials/SpecialUserlogin.php
View file

@ -1566,10 +1566,12 @@ class LoginForm extends SpecialPage {
* @return bool
*/
function hasSessionCookie() {
global $wgDisableCookieCheck;
global $wgDisableCookieCheck, $wgInitialSessionId;
return $wgDisableCookieCheck ||
SessionManager::singleton()->getPersistedSessionId( $this->getRequest() ) !== null;
return $wgDisableCookieCheck || (
$wgInitialSessionId &&
$this->getRequest()->getSession()->getId() === (string)$wgInitialSessionId
);
}
/**

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

@ -182,7 +182,6 @@ class SessionManagerTest extends MediaWikiTestCase {
$session = $manager->getSessionForRequest( $request );
$this->assertInstanceOf( 'MediaWiki\\Session\\Session', $session );
$this->assertSame( $idEmpty, $session->getId() );
$this->assertNull( $manager->getPersistedSessionId( $request ) );
// Both providers return info, picks best one
$request->info1 = new SessionInfo( SessionInfo::MIN_PRIORITY + 1, array(
@ -200,7 +199,6 @@ class SessionManagerTest extends MediaWikiTestCase {
$session = $manager->getSessionForRequest( $request );
$this->assertInstanceOf( 'MediaWiki\\Session\\Session', $session );
$this->assertSame( $id2, $session->getId() );
$this->assertSame( $id2, $manager->getPersistedSessionId( $request ) );
$request->info1 = new SessionInfo( SessionInfo::MIN_PRIORITY + 2, array(
'provider' => $provider1,
@ -217,7 +215,6 @@ class SessionManagerTest extends MediaWikiTestCase {
$session = $manager->getSessionForRequest( $request );
$this->assertInstanceOf( 'MediaWiki\\Session\\Session', $session );
$this->assertSame( $id1, $session->getId() );
$this->assertSame( $id1, $manager->getPersistedSessionId( $request ) );
// Tied priorities
$request->info1 = new SessionInfo( SessionInfo::MAX_PRIORITY, array(
@ -246,18 +243,6 @@ class SessionManagerTest extends MediaWikiTestCase {
$this->assertContains( $request->info1, $ex->sessionInfos );
$this->assertContains( $request->info2, $ex->sessionInfos );
}
try {
$manager->getPersistedSessionId( $request );
$this->fail( 'Expcected exception not thrown' );
} catch ( \OverFlowException $ex ) {
$this->assertStringStartsWith(
'Multiple sessions for this request tied for top priority: ',
$ex->getMessage()
);
$this->assertCount( 2, $ex->sessionInfos );
$this->assertContains( $request->info1, $ex->sessionInfos );
$this->assertContains( $request->info2, $ex->sessionInfos );
}
// Bad provider
$request->info1 = new SessionInfo( SessionInfo::MAX_PRIORITY, array(
@ -276,15 +261,6 @@ class SessionManagerTest extends MediaWikiTestCase {
$ex->getMessage()
);
}
try {
$manager->getPersistedSessionId( $request );
$this->fail( 'Expcected exception not thrown' );
} catch ( \UnexpectedValueException $ex ) {
$this->assertSame(
'Provider1 returned session info for a different provider: ' . $request->info1,
$ex->getMessage()
);
}
// Unusable session info
$this->logger->setCollect( true );
@ -304,7 +280,6 @@ class SessionManagerTest extends MediaWikiTestCase {
$session = $manager->getSessionForRequest( $request );
$this->assertInstanceOf( 'MediaWiki\\Session\\Session', $session );
$this->assertSame( $id2, $session->getId() );
$this->assertSame( $id2, $manager->getPersistedSessionId( $request ) );
$this->logger->setCollect( false );
// Unpersisted session ID
@ -321,7 +296,6 @@ class SessionManagerTest extends MediaWikiTestCase {
$this->assertSame( $id1, $session->getId() );
$session->persist();
$this->assertTrue( $session->isPersistent(), 'sanity check' );
$this->assertNull( $manager->getPersistedSessionId( $request ) );
}
public function testGetSessionById() {