Page MenuHomePhabricator
Create Task
Maniphest T265807

CI failures related to SocialProfile affecting other repositories for patches being submitted on the REL1_34 and REL1_35 branch, but not master
Open, Needs TriagePublic
Actions

Description

This was found while working on a security issue in Cosmos that was being backported to the REL1_34 and REL1_35 branch (but did not occur on the patch for master).

This issue was found in the Cosmos skin when backporting as Cosmos (optionally) integrates with SocialProfile, since the patch to T265539 added SocialProfile as a soft dependency of Cosmos in the CI jobs.

error for the patch (unrelated to SP) that was backported to REL1_34:

16:28:20 There was 1 error:
16:28:20 
16:28:20 1) SpecialPageFatalTest::testSpecialPageDoesNotFatal with data set "GiftManagerLogo" (GiftManagerLogo Object (...))
16:28:20 === Logs generated by test case
16:28:20 [objectcache] [debug] MainWANObjectCache using store {class} {"class":"EmptyBagOStuff"}
16:28:20 [localisation] [debug] LocalisationCache: using store LCStoreNull []
16:28:20 [objectcache] [debug] MainWANObjectCache using store {class} {"class":"EmptyBagOStuff"}
16:28:20 [wfDebug] [debug] IP: 127.0.0.1 {"private":false}
16:28:20 [localisation] [debug] LocalisationCache::isExpired(en): cache missing, need to make one []
16:28:20 [wfDebug] [debug] User::getBlockedStatus: checking... {"private":false}
16:28:20 [wfDebug] [debug] IP: 127.0.0.1 {"private":false}
16:28:20 ===
16:28:20 Undefined index: creator_user_id
16:28:20 
16:28:20 /workspace/src/extensions/SocialProfile/UserGifts/includes/specials/SpecialGiftManagerLogo.php:53
16:28:20 /workspace/src/extensions/SocialProfile/UserGifts/includes/specials/SpecialGiftManagerLogo.php:631
16:28:20 /workspace/src/extensions/SocialProfile/UserGifts/includes/specials/SpecialGiftManagerLogo.php:163
16:28:20 /workspace/src/extensions/SocialProfile/UserGifts/includes/specials/SpecialGiftManagerLogo.php:45
16:28:20 /workspace/src/tests/phpunit/includes/specials/SpecialPageExecutor.php:108
16:28:20 /workspace/src/tests/phpunit/includes/specials/SpecialPageExecutor.php:36
16:28:20 /workspace/src/tests/phpunit/structure/SpecialPageFatalTest.php:40
16:28:20 /workspace/src/tests/phpunit/MediaWikiIntegrationTestCase.php:455
16:28:20 /workspace/src/maintenance/doMaintenance.php:99

For REL1_35:

16:28:10 There was 1 error:
16:28:10 
16:28:10 1) SpecialPageFatalTest::testSpecialPageDoesNotFatal with data set "UploadAvatar" (SpecialUploadAvatar Object (...))
16:28:10 Declaration of UploadAvatar::performUpload($comment, $pageText, $watch, $user, $tags = Array) should be compatible with UploadBase::performUpload($comment, $pageText, $watch, $user, $tags = Array, ?string $watchlistExpiry = NULL)
16:28:10 
16:28:10 /workspace/src/extensions/SocialProfile/UserProfile/includes/avatar/UploadAvatar.php:257
16:28:10 /workspace/src/includes/AutoLoader.php:109
16:28:10 /workspace/src/includes/AutoLoader.php:109
16:28:10 /workspace/src/extensions/SocialProfile/UserProfile/includes/specials/SpecialUploadAvatar.php:34
16:28:10 /workspace/src/includes/specials/SpecialUpload.php:201
16:28:10 /workspace/src/extensions/SocialProfile/UserProfile/includes/specials/SpecialUploadAvatar.php:58
16:28:10 /workspace/src/tests/phpunit/includes/specials/SpecialPageExecutor.php:107
16:28:10 /workspace/src/tests/phpunit/includes/specials/SpecialPageExecutor.php:37
16:28:10 /workspace/src/tests/phpunit/structure/SpecialPageFatalTest.php:43
16:28:10 /workspace/src/tests/phpunit/MediaWikiIntegrationTestCase.php:446
16:28:10 /workspace/src/maintenance/doMaintenance.php:107
16:28:10 === Logs generated by test case
16:28:10 [objectcache] [debug] MainWANObjectCache using store {class} {"class":"EmptyBagOStuff"}
16:28:10 [localisation] [debug] LocalisationCache using store LCStoreNull []
16:28:10 [objectcache] [debug] MainWANObjectCache using store {class} {"class":"EmptyBagOStuff"}
16:28:10 [localisation] [debug] LocalisationCache::isExpired(en): cache missing, need to make one []
16:28:10 [MessageCache] [debug] MessageCache using store {class} {"class":"HashBagOStuff"}
16:28:10 [UserOptionsManager] [debug] Loading options from database {"user_id":1}
16:28:10 [wfDebug] [debug] ParserFactory: using default preprocessor {"private":false}
16:28:10 [localisation] [debug] LocalisationCache::isExpired(qqx): cache missing, need to make one []
16:28:10 [wfDebug] [debug] User::getBlockedStatus: checking blocked status for UTSysop {"private":false}
16:28:10 [wfDebug] [debug] UploadBase::createFromRequest: class name: UploadFromFile {"private":false}
16:28:10 ===

Related Objects

Event Timeline

SamanthaNguyen created this task.Oct 17 2020, 9:33 PM
SamanthaNguyen added a comment.Oct 17 2020, 9:36 PM

Looks like this column was dropped in the SQL patch at https://github.com/wikimedia/mediawiki-extensions-SocialProfile/blob/master/UserGifts/sql/patches/actor/drop-gift_creator_user_id.sql but there's still code trying to access this SQL column

SamanthaNguyen added a comment.Oct 17 2020, 9:40 PM

So this database column was dropped in master branch, but not in the REL1_34 branch, however last time I worked on SocialProfile, the master branch was the branch that users were expected to use and is expected to be stable with the latest stable version of MediaWiki.

This CI error is causing errors for other repositories, but not sure how to go further here, since the development of SP (and other social tools) has typically never really done backports to previous branches.

Relevant code block: https://github.com/wikimedia/mediawiki-extensions-SocialProfile/blob/REL1_34/UserGifts/includes/specials/SpecialGiftManagerLogo.php#L53

	function canUserManage() {
		$user = $this->getUser();

		$gift = Gifts::getGift( $this->gift_id );
		if (
			$user->getId() == $gift['creator_user_id'] ||
			$user->isAllowed( 'giftadmin' )
		) {
			return true;
		}

		return false;
	}
SamanthaNguyen updated the task description. (Show Details)Oct 17 2020, 9:41 PM
SamanthaNguyen added a comment.Oct 17 2020, 9:45 PM

UploadAvatar extends UploadFromFile, which extends UploadBase, which defines the performUpload() function. Since 1.35, it introduces the $watchlistExpiry parameter, so the method signatures don't match now; this causes a PHP exception to throw.

SamanthaNguyen updated the task description. (Show Details)Oct 17 2020, 9:50 PM
ashley added subscribers: Jdforrester-WMF, hashar.Oct 21 2020, 8:58 PM
In T265807#6557794, @SamanthaNguyen wrote:

last time I worked on SocialProfile, the master branch was the branch that users were expected to use and is expected to be stable with the latest stable version of MediaWiki.

This CI error is causing errors for other repositories, but not sure how to go further here, since the development of SP (and other social tools) has typically never really done backports to previous branches.

Indeed, and that hasn't changed. To me the release branches are as if they wouldn't be there at all. If someone accidentally ends up using them, they accept the risk of using an untested, unsupported configuration (non-master versions of social tools extensions). I just don't want anyone to get the false impression that those branches would be somehow "officially" supported by the social tools dev team (which is basically me), as they aren't.

Is there a way to force CI to use master version of SocialProfile instead of a REL1_xx version? Seems like that would fix this particular issue. Maybe @hashar or @Jdforrester-WMF would know?

P.S. Nice to see you around, @SamanthaNguyen. :-)

SamanthaNguyen added a comment.Oct 24 2020, 1:34 AM

@ashley Nice to see you too Jack Phoenix! :)

Yeah, I think getting CI to use the master branch would probably be the best route. I have no idea how difficult it would be to configure CI to do that though, so I'll also wait and see.

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and 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