Page MenuHomePhabricator
Create Task
Maniphest T264363

Instances of User are not serializable!
Closed, ResolvedPublicPRODUCTION ERROR
Actions

Description

Error

MediaWiki version: 1.36.0-wmf.11

message
Instances of User are not serializable!

Impact

Serializing user objects can drag along large amounts of data. Deserialized user objects may contain stale data. This is the case especially for the following member fields of User:

  • mRequest A WebRequest that would be invalid and misleading when deserialized during a later request.
  • mBlock, mBlockedFromCreateAccount, $mGlobalBlock:AbstractBlock instances that may be inaccurate at a later time.

Notes

One referer: https://fr.wikisource.org/w/index.php?title=Page:Revue_des_Deux_Mondes_-_1833_-_tome_4.djvu/362&action=edit

It seems that there are at least two things that cause User to be serialized:

  • ProofreadPage's PageContent gets serialized by PageEditStash. It contains a PageLevel, which contains a User object. This could be replaced with a UserIdentityValue, at least for serialization. PageContent actually already has special code for handing the User object when serializing to JSON for storage. This just isn't used when it's serialized via PHP's native serialization for caching. We could consider using proper content serialization in PageEditStash. Filed as T264389.
  • FeaturedFeedChannel gets serialized by FeatureFeeds for caching. It contains a User object, which is used to construct ParserOptions in the getFeedItem() method. The solution here would be to pass a User option (or ParserOptiosn) to getFeedItem() as a parameter. Filed as T264391.

Details

Request ID
90dc0671-65c3-4705-943f-5d420ec7e791
Request URL
https://fr.wikisource.org/w/api.php
Stack Trace
exception.trace
#0 [internal function]: User->__sleep()
#1 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MemcachedPeclBagOStuff.php(423): serialize(stdClass)
#2 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MediumSpecificBagOStuff.php(987): MemcachedPeclBagOStuff->serialize(stdClass)
#3 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MediumSpecificBagOStuff.php(788): MediumSpecificBagOStuff->getSerialized(stdClass, string)
#4 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MediumSpecificBagOStuff.php(172): MediumSpecificBagOStuff->makeValueOrSegmentList(string, stdClass, integer, integer)
#5 /srv/mediawiki/php-1.36.0-wmf.11/includes/Storage/PageEditStash.php(479): MediumSpecificBagOStuff->set(string, stdClass, integer, integer)
#6 /srv/mediawiki/php-1.36.0-wmf.11/includes/Storage/PageEditStash.php(160): MediaWiki\Storage\PageEditStash->storeStashValue(string, ProofreadPage\Page\PageContent, ParserOutput, string, User)
#7 /srv/mediawiki/php-1.36.0-wmf.11/includes/api/ApiStashEdit.php(151): MediaWiki\Storage\PageEditStash->parseAndCache(WikiPage, ProofreadPage\Page\PageContent, User, string)
#8 /srv/mediawiki/php-1.36.0-wmf.11/includes/api/ApiMain.php(1565): ApiStashEdit->execute()
#9 /srv/mediawiki/php-1.36.0-wmf.11/includes/api/ApiMain.php(545): ApiMain->executeAction()
#10 /srv/mediawiki/php-1.36.0-wmf.11/includes/api/ApiMain.php(516): ApiMain->executeActionWithErrorHandling()
#11 /srv/mediawiki/php-1.36.0-wmf.11/api.php(90): ApiMain->execute()
#12 /srv/mediawiki/php-1.36.0-wmf.11/api.php(45): wfApiMain()
#13 /srv/mediawiki/w/api.php(3): require(string)
#14 {main}
ProjectBranchLines +/-Subject
mediawiki/coremaster+0 -11Remove NonSerializableTrait from User object
mediawiki/corewmf/1.36.0-wmf.11+0 -11Remove NonSerializableTrait from User object
Customize query in gerrit

Related Objects
Search...

StatusSubtypeAssignedTask
 ResolvedReleasedduvallT263177 1.36.0-wmf.11 deployment blockers
 ResolvedPRODUCTION ERRORdanielT264363 Instances of User are not serializable!

Event Timeline

mmodell created this task.Oct 1 2020, 8:49 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 1 2020, 8:49 PM
mmodell added a project: MediaWiki-libs-ObjectCache.Oct 1 2020, 8:50 PM
DannyS712 added a subscriber: DannyS712.Oct 1 2020, 8:52 PM
• Pchelolo added a subscriber: • Pchelolo.Oct 1 2020, 8:52 PM

Caused by https://gerrit.wikimedia.org/r/c/mediawiki/core/+/625612

DannyS712 added a comment.Oct 1 2020, 8:53 PM

There doesn't appear to be a user object being serialized though?

// Store what is actually needed and split the output into another key (T204742)
$stashInfo = (object)[
	'pstContent' => $pstContent,
	'output'     => $parserOutput,
	'timestamp'  => $timestamp,
	'edits'      => $user->getEditCount()
];
$ok = $this->cache->set( $key, $stashInfo, $ttl, BagOStuff::WRITE_ALLOW_SEGMENTS );
gerritbot added a comment.Oct 1 2020, 8:54 PM

Change 631536 had a related patch set uploaded (by Urbanecm; owner: Urbanecm):
[mediawiki/core@master] Remove NonSerializableTrait from User object

https://gerrit.wikimedia.org/r/631536

gerritbot added a project: Patch-For-Review.Oct 1 2020, 8:54 PM
mmodell added a comment.Oct 1 2020, 8:56 PM

Apparently caused by rMWdc436c3cff5f: Introduce and apply NonSerializableTrait

mmodell added a comment.Oct 1 2020, 8:57 PM
In T264363#6510442, @DannyS712 wrote:

There doesn't appear to be a user object being serialized though?

// Store what is actually needed and split the output into another key (T204742)
$stashInfo = (object)[
	'pstContent' => $pstContent,
	'output'     => $parserOutput,
	'timestamp'  => $timestamp,
	'edits'      => $user->getEditCount()
];
$ok = $this->cache->set( $key, $stashInfo, $ttl, BagOStuff::WRITE_ALLOW_SEGMENTS );

maybe $pstContent or $parserOutput have a user object in there by accident?

mmodell added a comment.Oct 1 2020, 9:00 PM

https://gerrit.wikimedia.org/g/mediawiki/core/+/e7ff3cbb6b4db6f4db55746422d64d4205720f79/includes/Storage/PageEditStash.php#160

PageEditStash.php
			$code = $this->storeStashValue(
				$key,
				$editInfo->pstContent,
				$editInfo->output,
				$editInfo->timestamp,
				$user
			);
DannyS712 added a comment.Oct 1 2020, 9:02 PM
In T264363#6510489, @mmodell wrote:

https://gerrit.wikimedia.org/g/mediawiki/core/+/e7ff3cbb6b4db6f4db55746422d64d4205720f79/includes/Storage/PageEditStash.php#160

PageEditStash.php
			$code = $this->storeStashValue(
				$key,
				$editInfo->pstContent,
				$editInfo->output,
				$editInfo->timestamp,
				$user
			);

Yes but within storeStashValue the user is only used to retrieve the edit count

mmodell added a comment.Oct 1 2020, 9:03 PM

yeah I don't know what's going on. it must be the parser output that has a user in it somehow?

Krinkle added a parent task: T263177: 1.36.0-wmf.11 deployment blockers.Oct 1 2020, 9:08 PM
Krinkle moved this task from Untriaged to Sep 2020 on the Wikimedia-production-error board.
Krinkle edited projects, added MediaWiki-User-management, Platform Engineering; removed MediaWiki-libs-ObjectCache.
gerritbot added a comment.Oct 1 2020, 9:09 PM

Change 631477 had a related patch set uploaded (by 20after4; owner: Urbanecm):
[mediawiki/core@wmf/1.36.0-wmf.11] Remove NonSerializableTrait from User object

https://gerrit.wikimedia.org/r/631477

Urbanecm added a subscriber: Urbanecm.Oct 1 2020, 9:11 PM

This isn't used only in storeStashvalue through, a different instance is $availableFeeds = array_keys( FeaturedFeeds::getFeeds( false, $this->getUser() ) ); which can easily make user to get serialized. I confirm it gets serialized in FeaturedFeeds::getFeeds at least. Not sure how it happened in stash edit logic through :/.

Krinkle added a subscriber: Krinkle.Oct 1 2020, 9:16 PM

Call chain:

  • PageEditStash->parseAndCache takes User $user
  • PageEditStash->prepareContentForEdit
  • WikiPage->prepareContentForEdit -> getDerivedDataUpdater() -> getPreparedEdit()
  • DerivedPageDataUpdater->getPreparedEdit()
    • $preparedEdit = new PreparedEdit();
    • $preparedEdit->popts = $this->getCanonicalParserOptions();

It looks like ParserOptions might be storing a User object.

Stashbot added a comment.Oct 1 2020, 9:29 PM

Mentioned in SAL (#wikimedia-operations) [2020-10-01T21:29:53Z] <twentyafterfour@deploy1001> rebuilt and synchronized wikiversions files: rollback group0 as well T264363

daniel updated the task description. (Show Details)Oct 1 2020, 9:30 PM
daniel added a subscriber: daniel.Oct 1 2020, 9:36 PM
In T264363#6510495, @mmodell wrote:

yeah I don't know what's going on. it must be the parser output that has a user in it somehow?

Not natively, but some extension might be stuffing something into setExtensionData() or setProperty(). Maybe we should start logging the types of things that go in there.

gerritbot added a comment.Oct 1 2020, 9:40 PM

Change 631477 merged by jenkins-bot:
[mediawiki/core@wmf/1.36.0-wmf.11] Remove NonSerializableTrait from User object

https://gerrit.wikimedia.org/r/631477

gerritbot added a comment.Oct 1 2020, 9:43 PM

Change 631536 merged by jenkins-bot:
[mediawiki/core@master] Remove NonSerializableTrait from User object

https://gerrit.wikimedia.org/r/631536

ReleaseTaggerBot added a project: MW-1.36-notes (1.36.0-wmf.12; 2020-10-05; NEVER DEPLOYED).Oct 1 2020, 10:00 PM
daniel added a comment.Oct 1 2020, 10:06 PM
In T264363#6510522, @Krinkle wrote:

It looks like ParserOptions might be storing a User object.

Likely, but I don't see how ParserOptions would end up in the object that is going into the cache.

Maintenance_bot removed a project: Patch-For-Review.Oct 1 2020, 10:10 PM
Daimona added a subscriber: Daimona.Oct 1 2020, 10:38 PM
Krinkle added a comment.EditedOct 1 2020, 10:51 PM
In T264363#6510680, @daniel wrote:
In T264363#6510522, @Krinkle wrote:

It looks like ParserOptions might be storing a User object.

Likely, but I don't see how ParserOptions would end up in the object that is going into the cache.

ParserOptions is stored as part of the PreparedEdit object, right?

EDIT: Ah, I see. We only store part of PreparedEdit, not the whole object. Hmn.. I'm not sure in that case.

Daimona added a comment.Oct 1 2020, 11:02 PM

#6 /srv/mediawiki/php-1.36.0-wmf.11/includes/Storage/PageEditStash.php(160): MediaWiki\Storage\PageEditStash->storeStashValue(string, ProofreadPage\Page\PageContent, ParserOutput, string, User)

ProofreadPage\Page\PageContent
/**
	 * @param WikitextContent $header
	 * @param WikitextContent $body
	 * @param WikitextContent $footer
	 * @param PageLevel $level <--------------
	 */
	public function __construct(
		WikitextContent $header, WikitextContent $body, WikitextContent $footer, PageLevel $level
	) {
ProofreadPage\Page\PageLevel
/**
	 * @var User|null last user of the page
	 */
	protected $user = null;
	/**
	 * @param int $level
	 * @param User|null $user
	 */
	public function __construct( $level = self::NOT_PROOFREAD, User $user = null ) {
		$this->level = $level;
		$this->user = $user; // <--------------------------
	}

Isn't it?

Daimona added a comment.Oct 1 2020, 11:31 PM

FTR, we also have different traces on logstash:

Variant 1
#0 [internal function]: User->__sleep()
#1 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MemcachedPeclBagOStuff.php(254): Memcached->add(string, array, integer)
#2 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MediumSpecificBagOStuff.php(235): MemcachedPeclBagOStuff->doAdd(string, array, integer, integer)
#3 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MediumSpecificBagOStuff.php(311): MediumSpecificBagOStuff->add(string, array, integer, integer)
#4 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MediumSpecificBagOStuff.php(266): MediumSpecificBagOStuff->mergeViaCas(string, Closure, integer, integer, integer)
#5 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/wancache/WANObjectCache.php(762): MediumSpecificBagOStuff->merge(string, Closure, integer, integer)
#6 /srv/mediawiki/php-1.36.0-wmf.11/extensions/FeaturedFeeds/includes/FeaturedFeeds.php(58): WANObjectCache->set(string, array, integer)
#7 /srv/mediawiki/php-1.36.0-wmf.11/extensions/FeaturedFeeds/includes/FeaturedFeeds.php(119): FeaturedFeeds::getFeeds(string, User)
#8 /srv/mediawiki/php-1.36.0-wmf.11/includes/HookContainer/HookContainer.php(333): FeaturedFeeds::beforePageDisplay(OutputPage, SkinMinerva)
#9 /srv/mediawiki/php-1.36.0-wmf.11/includes/HookContainer/HookContainer.php(140): MediaWiki\HookContainer\HookContainer->callLegacyHook(string, array, array, array)
#10 /srv/mediawiki/php-1.36.0-wmf.11/includes/HookContainer/HookRunner.php(987): MediaWiki\HookContainer\HookContainer->run(string, array, array)
#11 /srv/mediawiki/php-1.36.0-wmf.11/includes/OutputPage.php(2610): MediaWiki\HookContainer\HookRunner->onBeforePageDisplay(OutputPage, SkinMinerva) ///// <-- any skin class can end up here, seen also for Vector, MB,Timeless, and Modern
#12 /srv/mediawiki/php-1.36.0-wmf.11/includes/MediaWiki.php(947): OutputPage->output(boolean)
#13 /srv/mediawiki/php-1.36.0-wmf.11/includes/MediaWiki.php(960): MediaWiki->{closure}()
#14 /srv/mediawiki/php-1.36.0-wmf.11/includes/MediaWiki.php(543): MediaWiki->main()
#15 /srv/mediawiki/php-1.36.0-wmf.11/index.php(53): MediaWiki->run()
#16 /srv/mediawiki/php-1.36.0-wmf.11/index.php(46): wfIndexMain()
#17 /srv/mediawiki/w/index.php(3): require(string)
#18 {main}
Variant 2
#0 [internal function]: User->__sleep()
#1 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MemcachedPeclBagOStuff.php(254): Memcached->add(string, array, integer)
#2 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MediumSpecificBagOStuff.php(235): MemcachedPeclBagOStuff->doAdd(string, array, integer, integer)
#3 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MediumSpecificBagOStuff.php(311): MediumSpecificBagOStuff->add(string, array, integer, integer)
#4 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MediumSpecificBagOStuff.php(266): MediumSpecificBagOStuff->mergeViaCas(string, Closure, integer, integer, integer)
#5 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/wancache/WANObjectCache.php(762): MediumSpecificBagOStuff->merge(string, Closure, integer, integer)
#6 /srv/mediawiki/php-1.36.0-wmf.11/extensions/FeaturedFeeds/includes/FeaturedFeeds.php(58): WANObjectCache->set(string, array, integer)
#7 /srv/mediawiki/php-1.36.0-wmf.11/extensions/FeaturedFeeds/includes/SpecialFeedItem.php(19): FeaturedFeeds::getFeeds(string, User)
#8 /srv/mediawiki/php-1.36.0-wmf.11/includes/specialpage/SpecialPage.php(600): SpecialFeedItem->execute(string)
#9 /srv/mediawiki/php-1.36.0-wmf.11/includes/specialpage/SpecialPageFactory.php(709): SpecialPage->run(string)
#10 /srv/mediawiki/php-1.36.0-wmf.11/includes/MediaWiki.php(307): MediaWiki\SpecialPage\SpecialPageFactory->executePath(Title, RequestContext)
#11 /srv/mediawiki/php-1.36.0-wmf.11/includes/MediaWiki.php(940): MediaWiki->performRequest()
#12 /srv/mediawiki/php-1.36.0-wmf.11/includes/MediaWiki.php(543): MediaWiki->main()
#13 /srv/mediawiki/php-1.36.0-wmf.11/index.php(53): MediaWiki->run()
#14 /srv/mediawiki/php-1.36.0-wmf.11/index.php(46): wfIndexMain()
#15 /srv/mediawiki/w/index.php(3): require(string)
#16 {main}
Variant 3
#0 [internal function]: User->__sleep()
#1 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MemcachedPeclBagOStuff.php(254): Memcached->add(string, array, integer)
#2 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MediumSpecificBagOStuff.php(235): MemcachedPeclBagOStuff->doAdd(string, array, integer, integer)
#3 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MediumSpecificBagOStuff.php(311): MediumSpecificBagOStuff->add(string, array, integer, integer)
#4 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MediumSpecificBagOStuff.php(266): MediumSpecificBagOStuff->mergeViaCas(string, Closure, integer, integer, integer)
#5 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/wancache/WANObjectCache.php(762): MediumSpecificBagOStuff->merge(string, Closure, integer, integer)
#6 /srv/mediawiki/php-1.36.0-wmf.11/extensions/FeaturedFeeds/includes/FeaturedFeeds.php(58): WANObjectCache->set(string, array, integer)
#7 /srv/mediawiki/php-1.36.0-wmf.11/extensions/FeaturedFeeds/includes/ApiFeaturedFeeds.php(49): FeaturedFeeds::getFeeds(string, User)
#8 /srv/mediawiki/php-1.36.0-wmf.11/includes/api/ApiBase.php(1705): ApiFeaturedFeeds->getAllowedParams(integer)
#9 /srv/mediawiki/php-1.36.0-wmf.11/includes/api/ApiBase.php(730): ApiBase->getFinalParams()
#10 /srv/mediawiki/php-1.36.0-wmf.11/includes/api/ApiMain.php(1156): ApiBase->extractRequestParams()
#11 /srv/mediawiki/php-1.36.0-wmf.11/includes/api/ApiMain.php(1544): ApiMain->setupModule()
#12 /srv/mediawiki/php-1.36.0-wmf.11/includes/api/ApiMain.php(545): ApiMain->executeAction()
#13 /srv/mediawiki/php-1.36.0-wmf.11/includes/api/ApiMain.php(516): ApiMain->executeActionWithErrorHandling()
#14 /srv/mediawiki/php-1.36.0-wmf.11/api.php(90): ApiMain->execute()
#15 /srv/mediawiki/php-1.36.0-wmf.11/api.php(45): wfApiMain()
#16 /srv/mediawiki/w/api.php(3): require(string)
#17 {main}
Variant 4
#0 [internal function]: User->__sleep()
#1 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MemcachedPeclBagOStuff.php(254): Memcached->add(string, array, integer)
#2 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MediumSpecificBagOStuff.php(235): MemcachedPeclBagOStuff->doAdd(string, array, integer, integer)
#3 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MediumSpecificBagOStuff.php(311): MediumSpecificBagOStuff->add(string, array, integer, integer)
#4 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/MediumSpecificBagOStuff.php(266): MediumSpecificBagOStuff->mergeViaCas(string, Closure, integer, integer, integer)
#5 /srv/mediawiki/php-1.36.0-wmf.11/includes/libs/objectcache/wancache/WANObjectCache.php(762): MediumSpecificBagOStuff->merge(string, Closure, integer, integer)
#6 /srv/mediawiki/php-1.36.0-wmf.11/extensions/FeaturedFeeds/includes/FeaturedFeeds.php(58): WANObjectCache->set(string, array, integer)
#7 /srv/mediawiki/php-1.36.0-wmf.11/extensions/FeaturedFeeds/includes/ApiFeaturedFeeds.php(30): FeaturedFeeds::getFeeds(string, User)
#8 /srv/mediawiki/php-1.36.0-wmf.11/includes/api/ApiMain.php(1565): ApiFeaturedFeeds->execute()
#9 /srv/mediawiki/php-1.36.0-wmf.11/includes/api/ApiMain.php(545): ApiMain->executeAction()
#10 /srv/mediawiki/php-1.36.0-wmf.11/includes/api/ApiMain.php(516): ApiMain->executeActionWithErrorHandling()
#11 /srv/mediawiki/php-1.36.0-wmf.11/api.php(90): ApiMain->execute()
#12 /srv/mediawiki/php-1.36.0-wmf.11/api.php(45): wfApiMain()
#13 /srv/mediawiki/w/api.php(3): require(string)
#14 {main}

All of these seem to share the same root cause (FeaturedFeeds::getFeeds), and if we include the one in the task description there's no more of them.

daniel added a comment.Oct 2 2020, 7:28 AM

Thanks for the analysis, @Daimona! I'll update the description.

daniel updated the task description. (Show Details)Oct 2 2020, 7:45 AM
daniel mentioned this in T264389: ProofreadPageContent must not contain User object, since it cannot be serialized safely..Oct 2 2020, 7:51 AM
daniel mentioned this in T264391: FeaturedFeedChannel must not contain a User object, since it cannot be serialized safely..
daniel updated the task description. (Show Details)Oct 2 2020, 7:55 AM
daniel closed this task as Resolved.Oct 2 2020, 7:58 AM
daniel claimed this task.
daniel mentioned this in T264393: Mark user object as non-serializable.

Closing this, since the production error has been fixed. Follow-up work filed under T264393: Mark user object as non-serializable.

daniel mentioned this in T264394: Ensure content of ParserOutput is safe to serialize.Oct 2 2020, 8:14 AM
Base mentioned this in T264370: User authentication security issue (Oct 1, 2020).Oct 2 2020, 2:59 PM
hashar added a subscriber: hashar.Feb 3 2021, 5:26 PM

Similar issue happens again with FeaturedFeeds: T273242

tstarling mentioned this in T273242: MemcachedPeclBagOStuff: Serialization of 'Closure' is not allowed.Feb 9 2021, 12:36 AM
Content licensed under Creative Commons Attribution-ShareAlike 3.0 (CC-BY-SA) unless otherwise noted; code licensed under GNU General Public License (GPL) or other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL