It would be great to have a staging server that uses KaskBagOStuff for its session management.
Description
Description
Details
Details
Customize query in gerrit
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | aaron | T88445 MediaWiki active/active datacenter investigation and work (tracking) | |||
| Resolved | Eevans | T206016 Create a service for session storage | |||
| Resolved | BPirkle | T215533 Enable use of session storage service in MediaWiki | |||
| Resolved | BPirkle | T222099 Staging release of RESTBagOStuff using Kask | |||
| Resolved | BPirkle | T227097 Make sure that we're taking CentralAuth into consideration for staging release | |||
| Resolved | BPirkle | T227696 OAuth extension uses session object store directly |
Event Timeline
Comment Actions
So, would the best way to do this be to configure test.wikipedia.org to use RESTBagOStuff and the production Kask service?
How do we get that going?
Comment Actions
What happens if we (in repository mediawiki-config):
- add kask-session and kask-transition to CommonSettings.php (per T224993)
- change the wgSessionCacheType section in InitialiseSettings.php to:
'wgSessionCacheType' => [ 'default' => 'redis_local', // declared in redis.php 'wikitech' => 'memcached-pecl', 'testwiki' => 'kask-transition', // change this to kask-session after the transition is complete ],
(I assume that test.wikipedia.org is "testwiki")
After confirming that much works, we could change testwiki to 'kask-session'. That would take testwiki through the same deployment stages we're planning for all the live wikis.
Comment Actions
Change 519432 had a related patch set uploaded (by BPirkle; owner: BPirkle):
[operations/mediawiki-config@master] Add kask session storage configuration. Use only on testwiki,
Comment Actions
Prospective configuration change pushed for review. Once this is merged and deployed, sessions for testwiki will be stored to both Redis and Kask, and will be continue to be read from Redis. (Other wikis will be unaffected and will continue to use only Redis.) Next steps will be:
- confirm sessions are being happily stored to Kask and that logs are clear
- wait a sufficient time period so that all testwiki sessions are in Kask ($wgObjectCacheSessionExpiry is currently 3600)
- push a config change to:
- change testwiki from 'kask-transition' to 'kask-session', thereby removing Redis and putting testwiki sessions solely in Kask
- change testwiki $wgObjectCacheSessionExpiry to match the Kask setting (if Kask uses something other than 3600)
- confirm sessions are happily functioning and logs are clear
Note: during the transition, testwiki will still be using a TTL ($wgObjectCacheSessionExpiry) value of 3600. The Kask TTL setting must therefore be at least 3600 seconds. If the Kask setting is longer, the transition will still be okay. But we will do more writes to Kask than we would with a longer settings, because MediaWiki will think the sessions need to be rewritten.
Comment Actions
I will work on getting the sessionstore cluster reconfigured for a TTL of 3600 (currently set to 86400).
Comment Actions
Change 519432 merged by jenkins-bot:
[operations/mediawiki-config@master] Add kask session storage configuration. Use only on testwiki,
Comment Actions
Change 526790 had a related patch set uploaded (by Urbanecm; owner: Urbanecm):
[operations/mediawiki-config@master] Fix a typo in CommonSettings.php
Comment Actions
Change 526790 merged by jenkins-bot:
[operations/mediawiki-config@master] Fix a typo in CommonSettings.php
Comment Actions
Mentioned in SAL (#wikimedia-operations) [2019-07-31T23:12:52Z] <urbanecm@deploy1001> Synchronized wmf-config/: SWAT: Add kask session storage configuration. Use only on testwiki, (ede989e, 862df8d, T222099) (duration: 00m 56s)
Comment Actions
Change 527120 had a related patch set uploaded (by BPirkle; owner: BPirkle):
[operations/mediawiki-config@master] Switch testwiki to read sessions from kask, with fallback to redis
Comment Actions
Change 527120 merged by jenkins-bot:
[operations/mediawiki-config@master] Switch testwiki to read sessions from kask, with fallback to redis
Comment Actions
Mentioned in SAL (#wikimedia-operations) [2019-08-01T18:06:10Z] <urbanecm@deploy1001> Synchronized wmf-config/CommonSettings.php: SWAT: 469c42d: Switch testwiki to read sessions from kask, with fallback to redis (T222099) (duration: 00m 55s)
Comment Actions
We have now completed two deploys that affect testwiki (only):
- use MultiWriteBagOStuff to write sessions to both redis and Kask, with reads from redis as primary, and kask as fallback
- use MultiWriteBagOStuff to write sessions to both redis and Kask, with reads from kask as primary, and redis as fallback
The final testwiki update will be to remove usage of MultiWriteBagOStuff and use only kask. This will fully move sessions off redis for testwiki.
Comment Actions
Change 528130 had a related patch set uploaded (by BPirkle; owner: BPirkle):
[operations/mediawiki-config@master] Switch testwiki to use kask (only) for sessions
Comment Actions
Change 528130 merged by jenkins-bot:
[operations/mediawiki-config@master] Switch testwiki to use kask (only) for sessions
Comment Actions
Mentioned in SAL (#wikimedia-operations) [2019-08-05T18:09:39Z] <urbanecm@deploy1001> Synchronized wmf-config/InitialiseSettings.php: SWAT: 254ecc1: Switch testwiki to use kask (only) for sessions (T222099) (duration: 00m 48s)
Comment Actions
We have completed all stages of the migration for testwiki. Testwiki is now using only kask for session storage, and is not using redis at all. Everything looks good from my end.
Comment Actions
Can't say for definite, but it seems this might be causing some session issues with our 2FA extension, MediaWiki-extensions-OATHAuth
See T231786 for the original task (2nd September) and T233146 for the current task
As far as we can see, it's only seemingly broken on testwiki (something put into session data doesn't come out the same). And may have been broken for a while, as I imagine enabling 2FA on testwiki isn't how most people would do it
The class in question going in is https://github.com/wikimedia/mediawiki-extensions-OATHAuth/blob/master/src/Key/TOTPKey.php and apparently comes out as an empty array...
While the code has undergone a lot of changes this year, it's mostly quietened down now, with the code in https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/OATHAuth/+/499183/ being live since July and working otherwise
Comment Actions
I think what I'm trying to say.. Is whatever seriali[sz]ation of objects Kask does... It seemingly doesn't like this object
Comment Actions
Pardon any ignorance here, but is this even stored in sessionstore? I know they share a common store elsewhere (Redis/mainstash), but AIUI, the migration to sessionstore (i.e. Kask) breaks sessions out from...well everything else.
I think what I'm trying to say.. Is whatever seriali[sz]ation of objects Kask does... It seemingly doesn't like this object
Kask doesn't do any serialization; The data is opaque, just bytes in and bytes out.
Comment Actions
The code goes down to
/** * Set a value in the session * @param string|int $key * @param mixed $value */ public function set( $key, $value ) { $data = &$this->backend->getData(); if ( !array_key_exists( $key, $data ) || $data[$key] !== $value ) { $data[$key] = $value; $this->backend->dirty(); } }
If we're adding something to the backends data...? Presumably?
Comment Actions
Kask doesn't, but RESTBagOStuff that's used to talk to Kask from MediaWiki does.
And rMWbf2d6d724074: Add additional configuation parameters to RESTBagOStuff switched RESTBagOStuff from using PHP serialization to JSON. And that will mangle objects.
>>> $key = MediaWiki\Extension\OATHAuth\Key\TOTPKey::newFromRandom();
=> MediaWiki\Extension\OATHAuth\Key\TOTPKey {#489}
>>> json_decode( json_encode( $key ), true );
=> []Comment Actions
Cheers Brad.
So, yeah, it's Kask (even if indirectly), and the way MW uses it is breaking this behaviour. It worked not using Kask. So the next question is does this just break OATHAuth? Or do other extensions put objects in and therefore will expect it to work?
It's certainly a breaking change as it stands
The docs say mixed, it doesn't say no objects. If they're not allowed, MW should let you know, and it should be documented. If they are allowed... Well, the json seriali[sz]ing needs to support it
Looking at most usages, it does seem that most are just passing bools, strings and some arrays.
/** * Set session data * * @note Prefer $this->getSession() instead if making multiple calls. * @param string $key Name of key in the session * @param mixed $data */ public function setSessionData( $key, $data ) { $this->getSession()->set( $key, $data ); } /** * Set a value in the session * @param string|int $key * @param mixed $value */ public function set( $key, $value ) { $data = &$this->backend->getData(); if ( !array_key_exists( $key, $data ) || $data[$key] !== $value ) { $data[$key] = $value; $this->backend->dirty(); } }
WebRequest::setSessionData()
Method
setSessionData
Found usages (44 usages found)
Delegate to another instance method (1 usage found)
mediawiki (1 usage found)
core/includes (1 usage found)
DerivativeRequest.php (1 usage found)
DerivativeRequest (1 usage found)
setSessionData (1 usage found)
70 $this->base->setSessionData( $key, $data );
Method call (43 usages found)
mediawiki (43 usages found)
extensions/BlueSpiceWhoIsOnline/src (2 usages found)
Tracer.php (2 usages found)
Tracer (2 usages found)
trace (2 usages found)
132 $context->getRequest()->setSessionData(
136 $context->getRequest()->setSessionData(
extensions/BreadCrumbs (2 usages found)
BreadCrumbsFunctions.php (2 usages found)
BreadCrumbsFunctions (2 usages found)
fnBreadCrumbsShowHook (2 usages found)
60 $request->setSessionData( "BreadCrumbs", $m_BreadCrumbs );
67 $request->setSessionData( "BreadCrumbs", $m_BreadCrumbs );
extensions/CentralAuth/includes (4 usages found)
CentralAuthHooks.php (4 usages found)
CentralAuthHooks (4 usages found)
doCentralLoginRedirect (3 usages found)
511 $request->setSessionData( 'CentralAuthDoEdgeLogin', true );
560 $request->setSessionData( 'CentralAuth:autologin:current-attempt', [
599 $request->setSessionData( 'CentralAuthDoEdgeLogin', true );
onBeforePageDisplay (1 usage found)
1145 $out->getRequest()->setSessionData( 'CentralAuthDoEdgeLogin', null );
extensions/CentralAuth/includes/specials (7 usages found)
SpecialCentralAutoLogin.php (2 usages found)
SpecialCentralAutoLogin (2 usages found)
execute (2 usages found)
357 $request->setSessionData( 'centralautologin-token', $token );
499 $request->setSessionData( 'CentralAuthDoEdgeLogin', true );
SpecialCentralLogin.php (2 usages found)
SpecialCentralLogin (2 usages found)
doLoginComplete (2 usages found)
254 $request->setSessionData( $skey, null );
285 $request->setSessionData( 'CentralAuthDoEdgeLogin', true );
SpecialMergeAccount.php (2 usages found)
SpecialMergeAccount (2 usages found)
addWorkingPassword (1 usage found)
188 $request->setSessionData( 'wsCentralAuthMigration', $data );
clearWorkingPasswords (1 usage found)
195 $request->setSessionData( 'wsCentralAuthMigration', $data );
SpecialSulRenameWarning.php (1 usage found)
SpecialSulRenameWarning (1 usage found)
execute (1 usage found)
76 $request->setSessionData( 'SulRenameWarning', null );
extensions/CentralNotice/includes/specials (1 usage found)
CentralNotice.php (1 usage found)
CentralNotice (1 usage found)
setCNSessionVar (1 usage found)
1615 $this->getRequest()->setSessionData( "centralnotice-{$variable}", $value );
extensions/DonationInterface/gateway_common (3 usages found)
GatewayPage.php (1 usage found)
GatewayPage (1 usage found)
overrideLogo (1 usage found)
513 $request->setSessionData( 'logoOverride', $storedOverride );
ResultSwitcher.php (1 usage found)
ResultSwitcher (1 usage found)
handleResultRequest (1 usage found)
42 $request->setSessionData( 'order_status', $sessionOrderStatus );
WmfFramework.mediawiki.php (1 usage found)
WmfFramework_Mediawiki (1 usage found)
setSessionValue (1 usage found)
58 RequestContext::getMain()->getRequest()->setSessionData( $key, $value );
extensions/FlaggedRevs/backend (1 usage found)
FlaggedRevsHooks.php (1 usage found)
FlaggedRevsHooks (1 usage found)
setSessionKey (1 usage found)
1272 $wgRequest->setSessionData( 'wsFlaggedRevsKey', $key );
extensions/GoogleAnalyticsTopPages/includes (1 usage found)
GoogleAnalyticsTopPages.body.php (1 usage found)
GoogleAnalyticsTopPages (1 usage found)
getData (1 usage found)
34 $request->setSessionData( 'service_token', $client->getAccessToken() );
extensions/GoogleLogin/includes (1 usage found)
GoogleLoginHooks.php (1 usage found)
GoogleLoginHooks (1 usage found)
onUserLogoutComplete (1 usage found)
16 $wgRequest->setSessionData( 'access_token', null );
extensions/LiquidThreads/classes (2 usages found)
View.php (2 usages found)
LqtView (2 usages found)
fixFauxRequestSession (1 usage found)
464 $request->setSessionData( $k, $v );
getInlineEditForm (1 usage found)
508 $wgRequest->setSessionData( $k, $v );
extensions/NetworkAuth (2 usages found)
NetworkAuth.class.php (2 usages found)
NetworkAuth (2 usages found)
onUserLoadAfterLoadFromSession (2 usages found)
64 $user->getRequest()->setSessionData( 'wsUserID', 0 );
65 $user->getRequest()->setSessionData( 'wsUserName', null );
extensions/OATHAuth/src/HTMLForm (2 usages found)
TOTPEnableForm.php (2 usages found)
TOTPEnableForm (2 usages found)
getDescriptors (1 usage found)
34 $this->getRequest()->setSessionData( 'oathauth_totp_key', $key );
onSubmit (1 usage found)
174 $this->getRequest()->setSessionData( 'oathauth_totp_key', null );
extensions/OAuthAuthentication/store (2 usages found)
PhpSessionStore.php (2 usages found)
PhpSessionStore (2 usages found)
delete (1 usage found)
23 $this->request->setSessionData( $key, null );
set (1 usage found)
19 $this->request->setSessionData( $key, $value );
extensions/SecureSessions (8 usages found)
SecureSessions.hooks.php (8 usages found)
SecureSessions (8 usages found)
onUserLoadFromSession (3 usages found)
287 $request->setSessionData( 'wsUserID', $user->getID() );
288 $request->setSessionData( 'wsUserName', $user->getName() );
289 $request->setSessionData( 'wsToken', $user->getToken() );
onUserLogout (2 usages found)
238 $request->setSessionData( 'wsUserAgent', null );
239 $request->setSessionData( 'wsIPAddress', null );
setup (2 usages found)
68 $request->setSessionData( 'wsObsolete', true );
69 $request->setSessionData( 'wsExpiry', time() + 5 );
updateSessionCache (1 usage found)
455 $this->getRequest()->setSessionData( 'id', $id );
extensions/SemanticPageSeries/includes (3 usages found)
SPSSpecialSeriesEdit.php (3 usages found)
SPSSpecialSeriesEdit (3 usages found)
evaluateForm (3 usages found)
238 $request->setSessionData( 'spsResult', $iteratorValuesCount );
239 $request->setSessionData( 'spsForm', $targetFormPageId );
240 $request->setSessionData( 'spsOrigin', $originPageId );
extensions/Thanks/includes (2 usages found)
ApiCoreThank.php (1 usage found)
ApiCoreThank (1 usage found)
sendThanks (1 usage found)
217 $user->getRequest()->setSessionData( "thanks-thanked-$type$id", true );
ApiFlowThank.php (1 usage found)
ApiFlowThank (1 usage found)
sendThanks (1 usage found)
170 $user->getRequest()->setSessionData( "flow-thanked-{$postId->getAlphadecimal()}", true );Session::set()
Method
set
Found usages (61 usages found)
Method call (61 usages found)
mediawiki (61 usages found)
core/includes (2 usages found)
FauxRequest.php (1 usage found)
FauxRequest (1 usage found)
__construct (1 usage found)
64 $mwsession->set( $key, $value );
WebRequest.php (1 usage found)
WebRequest (1 usage found)
setSessionData (1 usage found)
1124 $this->getSession()->set( $key, $data );
core/includes/api (1 usage found)
ApiAuthManagerHelper.php (1 usage found)
ApiAuthManagerHelper (1 usage found)
formatAuthenticationResponse (1 usage found)
218 $this->module->getRequest()->getSession()->set(
core/includes/auth (5 usages found)
AuthManager.php (5 usages found)
AuthManager (5 usages found)
autoCreateUser (3 usages found)
1634 $session->set( 'AuthManager::AutoCreateBlacklist', 'noname' );
1650 $session->set( 'AuthManager::AutoCreateBlacklist', 'authmanager-autocreate-noperm' );
1685 $session->set( 'AuthManager::AutoCreateBlacklist', $status );
setSessionDataForUser (2 usages found)
2409 $session->set( 'AuthManager:lastAuthId', $user->getId() );
2410 $session->set( 'AuthManager:lastAuthTimestamp', time() );
core/includes/preferences (1 usage found)
DefaultPreferencesFactory.php (1 usage found)
DefaultPreferencesFactory (1 usage found)
submitForm (1 usage found)
1732 $context->getRequest()->getSession()->set( 'specialPreferencesSaveSuccess', 1 );
core/includes/session (9 usages found)
PHPSessionHandler.php (3 usages found)
PHPSessionHandler (3 usages found)
write (3 usages found)
278 $session->set( $key, $value );
288 $session->set( $key, $value );
292 $session->set( $key, $value );
Session.php (6 usages found)
Session (6 usages found)
getSecretKeys (2 usages found)
401 $this->set( 'wsSessionSecret', $userSecret );
406 $this->set( 'wsSessionPbkdf2Iterations', $iterations );
getToken (1 usage found)
359 $this->set( 'wsTokenSecrets', $secrets );
offsetSet (1 usage found)
689 $this->set( $offset, $value );
resetToken (1 usage found)
379 $this->set( 'wsTokenSecrets', $secrets );
setSecret (1 usage found)
519 $this->set( $key, $encrypted );
core/includes/specials (2 usages found)
SpecialPreferences.php (1 usage found)
SpecialPreferences (1 usage found)
submitReset (1 usage found)
152 $this->getRequest()->getSession()->set( 'specialPreferencesSaveSuccess', 1 );
SpecialUserrights.php (1 usage found)
UserrightsPage (1 usage found)
execute (1 usage found)
205 $session->set( 'specialUserrightsSaveSuccess', 1 );
core/includes/user (4 usages found)
User.php (4 usages found)
User (4 usages found)
doLogout (1 usage found)
3907 $session->set( 'wsUserID', 0 ); // Other code expects this
loadFromSession (3 usages found)
1269 $session->set( 'wsUserID', $this->getId() );
1270 $session->set( 'wsUserName', $this->getName() );
1271 $session->set( 'wsToken', $this->getToken() );
core/tests/phpunit/includes/api (1 usage found)
ApiTestCase.php (1 usage found)
ApiTestCase (1 usage found)
doApiRequest (1 usage found)
76 $sessionObj->set( $key, $value );
core/tests/phpunit/includes/auth (2 usages found)
AuthManagerTest.php (2 usages found)
AuthManagerTest (2 usages found)
testAutoAccountCreation (2 usages found)
2503 $session->set( 'AuthManager::AutoCreateBlacklist', 'test' );
2518 $session->set( 'AuthManager::AutoCreateBlacklist', StatusValue::newFatal( 'test2' ) );
core/tests/phpunit/includes/session (21 usages found)
PHPSessionHandlerTest.php (18 usages found)
PHPSessionHandlerTest (18 usages found)
testSessionHandling (18 usages found)
210 $session->set( 'Unchanged', 'setup' );
211 $session->set( 'Unchanged, null', null );
212 $session->set( 'Changed in $_SESSION', 'setup' );
213 $session->set( 'Changed in Session', 'setup' );
214 $session->set( 'Changed in both', 'setup' );
215 $session->set( 'Deleted in Session', 'setup' );
216 $session->set( 'Deleted in $_SESSION', 'setup' );
217 $session->set( 'Deleted in both', 'setup' );
218 $session->set( 'Deleted in Session, changed in $_SESSION', 'setup' );
219 $session->set( 'Deleted in $_SESSION, changed in Session', 'setup' );
225 $session->set( 'Added in Session', 'Session' );
226 $session->set( 'Added in both', 'Session' );
227 $session->set( 'Changed in Session', 'Session' );
228 $session->set( 'Changed in both', 'Session' );
229 $session->set( 'Deleted in $_SESSION, changed in Session', 'Session' );
258 $session->set( 42, 'forty-two' );
259 $session->set( 'forty-two', 42 );
260 $session->set( 'wrong', 43 );
SessionTest.php (3 usages found)
SessionTest (3 usages found)
testSecrets (3 usages found)
71 $session->set( 'test', 'foobar' );
83 $session->set( 'test', $encrypted . 'x' );
99 $session->set( 'test', $encrypted );
core/tests/phpunit/unit/includes/session (3 usages found)
SessionUnitTest.php (3 usages found)
SessionUnitTest (3 usages found)
testDataAccess (3 usages found)
111 $session->set( 'foo', 55 );
116 $session->set( 1, 'one' );
121 $session->set( 1, 'one' );
extensions/Auth_remoteuser/src (2 usages found)
UserNameSessionProvider.php (2 usages found)
UserNameSessionProvider (2 usages found)
refreshSessionInfo (2 usages found)
707 $session->set( 'AuthManager:lastAuthId', $info->getUserInfo()->getId() );
708 $session->set( 'AuthManager:lastAuthTimestamp', time() );
extensions/BlueSpiceFoundation/src (2 usages found)
DeferredNotificationStack.php (2 usages found)
DeferredNotificationStack (2 usages found)
__construct (1 usage found)
25 $this->request->getSession()->set( 'notificationInfo', [] );
push (1 usage found)
37 $this->request->getSession()->set( 'notificationInfo', $notifications );
extensions/CentralAuth/includes (1 usage found)
CentralAuthUtils.php (1 usage found)
CentralAuthUtils (1 usage found)
setCentralSession (1 usage found)
283 $session->set( 'CentralAuth::centralSessionId', $id );
extensions/ConfirmEdit/includes/auth (1 usage found)
CaptchaPreAuthenticationProvider.php (1 usage found)
CaptchaPreAuthenticationProvider (1 usage found)
testForAuthentication (1 usage found)
86 $session->set( 'ConfirmEdit:loginCaptchaPerUserTriggered', true );
extensions/ConfirmEdit/includes/store (1 usage found)
CaptchaSessionStore.php (1 usage found)
CaptchaSessionStore (1 usage found)
store (1 usage found)
12 SessionManager::getGlobalSession()->set( 'captcha' . $index, $info );
extensions/ConfirmEdit/tests/phpunit (1 usage found)
CaptchaPreAuthenticationProviderTest.php (1 usage found)
CaptchaPreAuthenticationProviderTest (1 usage found)
flagSession (1 usage found)
283 ->set( 'ConfirmEdit:loginCaptchaPerUserTriggered', true );
extensions/LDAPProvider/src/Hook (1 usage found)
UserLoadAfterLoadFromSession.php (1 usage found)
UserLoadAfterLoadFromSession (1 usage found)
doProcess (1 usage found)
185 $session->set( $this->sessionDataKey, $nowTS );
extensions/UnCaptcha/includes (1 usage found)
RobotPreAuthenticationProvider.php (1 usage found)
RobotPreAuthenticationProvider (1 usage found)
getAuthenticationRequests (1 usage found)
40 $session->set( self::SESSION_KEY, $not );Comment Actions
Note that per T161647: RFC: Deprecate using php serialization inside MediaWiki we should not be using PHP serialization for this purpose. It's insecure and brittle against code changes. Only "dumb" objects should get serialized (associative arrays, essentially). Any code relying on "proper" objects coming back from serialization should be fixed.
Comment Actions
That's fair enough, but it's still a breaking change...
The code docs don't say this, MW doesn't seemingly seem to throw any deprecated warnings etc
We can trivially enough fix OATHAuth, so that's not an issue, but that doesn't prevent anyone else falling over on the same issue with MW in it's current state :)
Comment Actions
This is clearly a breaking change, no argument there. Fortunately, it is a configuration-dependent one and we've only affected ourselves, and in a limited fashion. I'm glad we were as conservative in rolling it out as we were, and I'm suddenly thankful for the the Kask performance investigation (now complete) that delayed further rollout.
So, what are the to-do items for MW?
- update WebRequest::setSessionData() documentation to indicate "proper" objects are not allowed
- is there a better doxygen/phpdoc way to indicate this than "mixed" and then saying something about "no objects" in the comments?
- given that objects do currently work for other backends, should the language be soft ("objects are not guaranteed to work")?
- corresponding update to Session::set() documentation, and probably also a note in the class comment
- corresponding update to functions with RESTBagOStuff, and probably also a note in the class comment
- add something to RELEASE-NOTES?
- does this justify an email to wikitech-l?
- what else?
@Reedy mentioned possibly throwing deprecated warnings, but I'm not sure what to do in-code. Session storage seems performance-critical enough that I'd rather not do introspection on the session data to be serialized/encoded. Is there something both effective and inoffensive that we could/should do? If so, would it live in RESTBagOStuff (because only wikis that use that backend are currently affected) or Session (because many more wikis may eventually be affected if we truly purge serialize/unserialize)?
Comment Actions
I think most of this sounds like a good way forward. I would suggest potentially mailing mediawiki-l too as well as wikitech-l to get a bit of a wider audience, for people who care about MediaWiki, but not so much the Wikimedia usage; especially if we're going to (eventually) be moving away from php serialisation completely (though that might be a major release of MW or two away) in the future
Should make sure any relevant docs on mediawiki.org are updated too (if there are any that need updating)
The deprecated warnings and related code to that might be somewhat OTT (as well as a potential performance hit as you suggest). If we have things sufficiently and correctly documented, published etc that this behaviour isn't supported (at least as currently it works), that goes a long way to mitigating and helping developers. When looking into this problem, I certainly couldn't tell from the MW code (which obviously the path is vague as to which underlying classes may be used during the config) that anything might've changed etc, and it wasn't until you mentioned it on IRC I had any idea that this was changing (I don't read every wikitech post etc). Was this actually announced anywhere that I probably should've seen it?
Something like that, potentially. If the class is implementing JsonSerializable, and as such is returning something that the json code can/will seriali[sz]e... It should work, and that seems like an acceptable solution/similar for the path forward. Comments are cheap, so adding that hint certainly wouldn't be a bad thing
Comment Actions
Was this actually announced anywhere that I probably should've seen it?
No, it was not. Apologies.
I've created T233537: Document and communicate potentially breaking session storage serialization change for making people aware of this, and added people who commented on this task as subscribers.
Comment Actions
Change 554910 had a related patch set uploaded (by Eevans; owner: Eevans):
[operations/mediawiki-config@master] Update session serialization (Kask) to PHP w/ HMAC
Comment Actions
Change 554910 merged by jenkins-bot:
[operations/mediawiki-config@master] Update session serialization (Kask) to PHP w/ HMAC
Comment Actions
Mentioned in SAL (#wikimedia-operations) [2019-12-06T00:47:25Z] <catrope@deploy1001> Synchronized wmf-config/CommonSettings.php: Use PHP serialization with HMAC for Kask session serialization (T222099) (duration: 01m 01s)
Comment Actions
Mentioned in SAL (#wikimedia-operations) [2019-12-06T01:02:44Z] <reedy@deploy1001> Synchronized private/PrivateSettings.php: wmgSessionStoreHMACKey T222099 (duration: 01m 07s)
Comment Actions
Mentioned in SAL (#wikimedia-operations) [2019-12-06T01:04:09Z] <catrope@deploy1001> Synchronized private/PrivateSettings.php: HMAC value for Kask config (T222099) (duration: 00m 59s)