- Set $wgPHPSessionHandling = 'disable' in your LocalSettings
- Find any tests for SessionManager or other session-adjacent features that fail under that configuration
- If the test primarily tests the PHPSessionHandler class, make it pass using $this->overrideConfigValue( ... )
- If the test is more generic, find a way to make it pass regardless of the config, or split it into two tests (one with $wgPHPSessionHandling set to 'disable', another with it set to 'enable')
Description
Details
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | JTweed-WMF | T398814 WE5.1.1 Session storage protection | |||
| Resolved | JTweed-WMF | T400365 Support multiple session stores | |||
| Resolved | matmarex | T362324 Disable PHPSessionHandler in Wikimedia production | |||
| Resolved | Hokwelum | T400667 Make all SessionManager tests pass with PHPSessionHandler disabled |
Event Timeline
So I tried doing this locally by adding $wgPHPSessionHandling = 'disable'; to my localsettings and running: ./vendor/bin/phpunit tests/phpunit/includes/session/.
it returned:
OK (138 tests, 1280 assertions)
Yeah, my bad. I actually saw the error in warn mode, not disable mode:
1) MediaWiki\Tests\Session\SessionBackendTest::testResetIdOfGlobalSession Use of $_SESSION was deprecated in MediaWiki 1.27. [Called from session_write_close in (internal function)] /vagrant/mediawiki/includes/debug/MWDebug.php:386 /vagrant/mediawiki/includes/debug/MWDebug.php:357 /vagrant/mediawiki/includes/debug/MWDebug.php:238 /vagrant/mediawiki/includes/GlobalFunctions.php:787 /vagrant/mediawiki/includes/session/PHPSessionHandler.php:331 /vagrant/mediawiki/includes/session/SessionBackend.php:271 /vagrant/mediawiki/tests/phpunit/includes/session/SessionBackendTest.php:1014
Apparently the test overrides the enable flag (so it doesn't break when you disable PHP session handling) but doesn't override the warn flag.
But just to be sure, you could check the normal CI pipeline: create a patch where you change the default value of $wgPHPSessionHandling to disable, label it as a test patch (we usually prefix the commit message with something like [DNM] or [POC]), create an empty CentralAuth patch that depends on it (I think CentralAuth is the only Wikimedia extension that heavily interacts with session handling), upload them to Gerrit and see if they pass CI.
Change #1180221 had a related patch set uploaded (by Hokwelum; author: Hokwelum):
[mediawiki/core@master] [DNM] set $wgPHPSessionHandling to disable
Change #1180225 had a related patch set uploaded (by Hokwelum; author: Hokwelum):
[mediawiki/extensions/CentralAuth@master] [DNM] test disabled $wgPHPSessionHandling
From the test patch:
There were 2 failures: 1) MediaWiki\Tests\Api\ApiLoginTest::testDeprecatedUserLogin with data set "Bot passwords enabled" (true) Failed asserting that two arrays are identical. --- Expected +++ Actual @@ @@ Array &0 ( 'result' => 'Success' - 'lguserid' => 1 - 'lgusername' => 'TestUser 31a86d.27e' + 'lguserid' => 2 + 'lgusername' => 'UTSysop' ) /workspace/src/tests/phpunit/includes/api/ApiLoginTest.php:110 2) MediaWiki\Tests\Api\ApiLoginTest::testDeprecatedUserLogin with data set "Bot passwords disabled" (false) Failed asserting that two arrays are identical. --- Expected +++ Actual @@ @@ Array &0 ( 'result' => 'Success' - 'lguserid' => 1 - 'lgusername' => 'TestUser 31a86d.280' + 'lguserid' => 2 + 'lgusername' => 'UTSysop' ) /workspace/src/tests/phpunit/includes/api/ApiLoginTest.php:110
That's a scary error. I guess an existing session is somehow being reused in ApiTestCase::doApiRequest()?
To replicate the errors, I had to do $this->overrideConfigValue( CAMainConfigNames::CentralAuthEnableSul3, false ); in APiLoginTest.
Yes, I can reproduce it locally. I was testing with CentralAuth not being installed, since it interferes with many of the core auth/session tests (since it intentionally changes the behavior that they are testing).
Change #1182208 had a related patch set uploaded (by Hokwelum; author: Hokwelum):
[mediawiki/core@master] Fix failing test when PHPSessionHandler is set to disable
Change #1182208 merged by jenkins-bot:
[mediawiki/core@master] Fix failing test when PHPSessionHandler is set to disable
For the record, I manually tested the same workflow as in the failing test with $wgPHPSessionHandling = 'disable' (logging in with the legacy action=login API while logged in as another user) and it works correctly.
I did some debugging and it looks like ApiLogin reads out the user data from a different Session object than the one which AuthManager writes to. I haven't dug deep enough to figure out why this happens (probably some incomplete mocking/resetting somewhere), or why enabling PHPSessionHandler makes it work.
Change #1180225 abandoned by Hokwelum:
[mediawiki/extensions/CentralAuth@master] [DNM] test disabled $wgPHPSessionHandling
Reason:
Not needed!