PHP Warning: preg_match() expects parameter 2 to be string, array given
Closed, ResolvedPublicPRODUCTION ERROR
Actions

Description

Error

mwversion: 1.39.0-wmf.1
reqId: 810de6b3-b19d-4397-92a9-fe4fb0551b76
Find reqId in Logstash

normalized_message

[{reqId}] {exception_url}   PHP Warning: preg_match() expects parameter 2 to be string, array given

exception.trace

from /srv/mediawiki/php-1.39.0-wmf.1/includes/libs/ParamValidator/TypeDef/IntegerDef.php(26)
#0 [internal function]: MWExceptionHandler::handleError(integer, string, string, integer, array)
#1 /srv/mediawiki/php-1.39.0-wmf.1/includes/libs/ParamValidator/TypeDef/IntegerDef.php(26): preg_match(string, array)
#2 /srv/mediawiki/php-1.39.0-wmf.1/includes/libs/ParamValidator/ParamValidator.php(597): Wikimedia\ParamValidator\TypeDef\IntegerDef->validate(string, array, array, array)
#3 /srv/mediawiki/php-1.39.0-wmf.1/includes/libs/ParamValidator/ParamValidator.php(551): Wikimedia\ParamValidator\ParamValidator->validateValue(string, array, array, array)
#4 /srv/mediawiki/php-1.39.0-wmf.1/includes/Rest/Validator/Validator.php(108): Wikimedia\ParamValidator\ParamValidator->getValue(string, array, array)
#5 /srv/mediawiki/php-1.39.0-wmf.1/includes/Rest/Handler.php(184): MediaWiki\Rest\Validator\Validator->validateParams(array)
#6 /srv/mediawiki/php-1.39.0-wmf.1/includes/Rest/Router.php(405): MediaWiki\Rest\Handler->validate(MediaWiki\Rest\Validator\Validator)
#7 /srv/mediawiki/php-1.39.0-wmf.1/includes/Rest/Router.php(338): MediaWiki\Rest\Router->executeHandler(GrowthExperiments\Rest\Handler\MenteesHandler)
#8 /srv/mediawiki/php-1.39.0-wmf.1/includes/Rest/EntryPoint.php(167): MediaWiki\Rest\Router->execute(MediaWiki\Rest\RequestFromGlobals)
#9 /srv/mediawiki/php-1.39.0-wmf.1/includes/Rest/EntryPoint.php(132): MediaWiki\Rest\EntryPoint->execute()
#10 /srv/mediawiki/php-1.39.0-wmf.1/rest.php(31): MediaWiki\Rest\EntryPoint::main()
#11 /srv/mediawiki/w/rest.php(3): require(string)
#12 {main}

Impact

Moderate level of logspam during train deployment. Noticed during T300203: 🧪🚂 Trainsperiment Week: 1.39.0-wmf.1, 1.39.0-wmf.2, 1.39.0-wmf.3, 1.39.0-wmf.4 deployment blockers but not an immediate blocker.

Special:MentorDashboard's mentee overview module was not working, as it called the API endpoint with incorrect (and malformed) parameters, which didn't return the data required.

Notes

Seems to be related to bad client data. Noticed about the same time as T304349: PHP Notice: Undefined index: taskType - causes Argument 1 passed to GrowthExperiments\NewcomerTasks\NewcomerTasksChangeTagsManager::apply() must be of the type string, null given and could be related.

Details

Request URL: https://en.wikipedia.org/w/rest.php/growthexperiments/v0/mentees?order=*&sortby=*&limit%5BusersToShow%5D=*&offset=*&minedits=*&maxedits=*&onlystarred=*

Subject	Repo	Branch	Lines +/-
ParamValidator: Handle array url parameters on validation	mediawiki/core	master	+123 -31
ParamValidator: Disallow array values in integer param	mediawiki/core	master	+10 -1
MenteeOverviewPresets.getUsersToShow: Fix typo	mediawiki/extensions/GrowthExperiments	wmf/1.39.0-wmf.1	+1 -1
MenteeOverviewPresets.getUsersToShow: Fix typo	mediawiki/extensions/GrowthExperiments	wmf/1.39.0-wmf.3	+1 -1
MenteeOverviewPresets.getUsersToShow: Fix typo	mediawiki/extensions/GrowthExperiments	wmf/1.39.0-wmf.2	+1 -1
MenteeOverviewPresets.getUsersToShow: Fix typo	mediawiki/extensions/GrowthExperiments	master	+1 -1

Customize query in gerrit

Related Objects

Mentioned In: T326764: PHP Warning: substr() expects parameter 1 to be string, array given
Mentioned Here: T326764: PHP Warning: substr() expects parameter 1 to be string, array given
rEGRE84877bdb1fe5: MenteeOverviewPresets.getUsersToShow: Fix typo
rEGRE4f72198f9b14: MenteeOverviewPresets: Add helper methods for known properties
rEGREe9d89a3a319a: MenteeOverviewPresets.getUsersToShow: Fix typo
T301360: InvalidArgumentException on bad URL rather than helpful message
T300203: 🧪🚂 Trainsperiment Week: 1.39.0-wmf.1, 1.39.0-wmf.2, 1.39.0-wmf.3, 1.39.0-wmf.4 deployment blockers
T304349: PHP Notice: Undefined index: taskType - causes Argument 1 passed to GrowthExperiments\NewcomerTasks\NewcomerTasksChangeTagsManager::apply() must be of the type string, null given

Event Timeline

• dduvall created this task.Mar 21 2022, 7:10 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 21 2022, 7:10 PM

This is not related to T304349. Seems like a bug in MediaWiki-REST-API perhaps?

kostajh moved this task from Inbox to Triaged on the Growth-Team board.Mar 21 2022, 7:33 PM

brennen subscribed.Mar 21 2022, 7:58 PM

• dduvall triaged this task as Unbreak Now! priority.Mar 21 2022, 8:05 PM

• dduvall added a parent task: T300203: 🧪🚂 Trainsperiment Week: 1.39.0-wmf.1, 1.39.0-wmf.2, 1.39.0-wmf.3, 1.39.0-wmf.4 deployment blockers.

Agusbou2015 subscribed.Mar 21 2022, 8:20 PM

This query string seems malformed:

https://cs.wikipedia.org/w/rest.php/growthexperiments/v0/mentees?order=desc&sortby=last_active&limit%5BusersToShow%5D=10&offset=NaN&minedits=1&maxedits=500&onlystarred=false

limit isn't being accepted, and offset=NaN seems weird.

Urbanecm_WMF edited projects, added Growth-Team (Sprint 0 (Growth Team)), User-Urbanecm_WMF (Engineering); removed Growth-Team.Mar 21 2022, 8:36 PM

The frequency of these errors is now pretty low and occurring in wmf.1 (currently deployed to all wikis) as well as wmf.2. Removing this as a blocker and lowering priority since promoting wmf.2 should not result in an increase in occurrence. If there is a more serious underlying cause here, we can reassess and decide whether or not to rollback.

• dduvall removed a parent task: T300203: 🧪🚂 Trainsperiment Week: 1.39.0-wmf.1, 1.39.0-wmf.2, 1.39.0-wmf.3, 1.39.0-wmf.4 deployment blockers.Mar 21 2022, 8:44 PM

In T304353#7794496, @brennen wrote:

This query string seems malformed:

https://cs.wikipedia.org/w/rest.php/growthexperiments/v0/mentees?order=desc&sortby=last_active&limit%5BusersToShow%5D=10&offset=NaN&minedits=1&maxedits=500&onlystarred=false

limit isn't being accepted, and offset=NaN seems weird.

limit is array, but array is not allowed.

The parameter limit%5BusersToShow%5D=10 gets handled as array and maybe the problem (Similiar to T301360)

But the rest of the query is also invalid, because offset=NaN is invalid.

Limit could use the LimitTypeDef or the client side sends better URI

I just looked through some of the implementations of TypeDef::validate(). They all failhard if they are called with an array in the $value parameter. They all expect a string, or at least somethign that can be coerced into a string.

The $value parameter comes from TypeDef::getValue() which is documented to return null|mixed. The default implementation delegates to Callbacks::getValue(), which is documented to return string|string[]|mixed.

It seems to me that all implementations of TypeDef::validate should check the parameter has the expected type and throw ValidationException if it doesnt.

LimitDef accepts an integer or "max", so that wouldn't help either.

In essence: we get an illegal value for that parameter, that needs to be fixed in the client (assuming it's not malicious). But MW should be fixed to fail gracefully. I'd consider this logspam. Doesn't need to block the train, I think.

Change 772508 had a related patch set uploaded (by Ammarpad; author: Ammarpad):

[mediawiki/core@master] Rest: Disallow array values in integer param

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

gerritbot added a project: Patch-For-Review.Mar 21 2022, 9:30 PM

Change 772537 had a related patch set uploaded (by Urbanecm; author: Urbanecm):

[mediawiki/extensions/GrowthExperiments@master] MenteeOverviewPresets.getUsersToShow: Fix typo

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

Change 772537 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] MenteeOverviewPresets.getUsersToShow: Fix typo

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

ReleaseTaggerBot added a project: MW-1.39-notes (1.39.0-wmf.4; 2022-03-23).Mar 22 2022, 6:00 AM

Change 772483 had a related patch set uploaded (by Urbanecm; author: Urbanecm):

[mediawiki/extensions/GrowthExperiments@wmf/1.39.0-wmf.1] MenteeOverviewPresets.getUsersToShow: Fix typo

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

Change 772484 had a related patch set uploaded (by Urbanecm; author: Urbanecm):

[mediawiki/extensions/GrowthExperiments@wmf/1.39.0-wmf.2] MenteeOverviewPresets.getUsersToShow: Fix typo

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

Change 772485 had a related patch set uploaded (by Urbanecm; author: Urbanecm):

[mediawiki/extensions/GrowthExperiments@wmf/1.39.0-wmf.3] MenteeOverviewPresets.getUsersToShow: Fix typo

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

The client-side error was introduced with 4f72198 and should be fixed with e9d89a3. In addition to yielding a PHP Warning, it also breaks the mentor dashboard (as the API errors out instead of providing expected data). I'm backporting the fix now, so hopefully the errors should cease.

Change 772484 merged by Urbanecm:

[mediawiki/extensions/GrowthExperiments@wmf/1.39.0-wmf.2] MenteeOverviewPresets.getUsersToShow: Fix typo

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

Change 772485 merged by Urbanecm:

[mediawiki/extensions/GrowthExperiments@wmf/1.39.0-wmf.3] MenteeOverviewPresets.getUsersToShow: Fix typo

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

Mentioned in SAL (#wikimedia-operations) [2022-03-22T07:57:03Z] <urbanecm@deploy1002> Synchronized php-1.39.0-wmf.2/extensions/GrowthExperiments/modules/ext.growthExperiments.MentorDashboard/MenteeOverview/MenteeOverviewPresets.js: 84877bd: MenteeOverviewPresets.getUsersToShow: Fix typo (T304353) (duration: 00m 49s)

This should now be resolved from Growth's POV, as the API client was fixed. A decision still needs to be made whether or not to proceed with the core patch as well.

Urbanecm_WMF moved this task from Backlog to My Part Done on the User-Urbanecm_WMF (Engineering) board.Mar 22 2022, 7:59 AM

ReleaseTaggerBot edited projects, added MW-1.39-notes (1.39.0-wmf.3; 2022-03-22); removed MW-1.39-notes (1.39.0-wmf.4; 2022-03-23).Mar 22 2022, 8:00 AM

Urbanecm_WMF updated the task description. (Show Details)Mar 22 2022, 8:03 AM

Change 772483 abandoned by Urbanecm:

[mediawiki/extensions/GrowthExperiments@wmf/1.39.0-wmf.1] MenteeOverviewPresets.getUsersToShow: Fix typo

Reason:

probably not necessary

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

Krinkle moved this task from Untriaged to Mar 2022 on the Wikimedia-production-error board.Mar 22 2022, 1:46 PM

• dduvall awarded a token.Mar 22 2022, 4:17 PM

daniel added a project: Platform Team Initiatives (MW REST API in PHP).Mar 23 2022, 8:45 AM

I don't think this is High prio anymore, as it's not "logspam that's currently happening", but a chance to get logspam thanks to clients sending malformed requests.

• nnikkhoui added a project: API Platform.Mar 23 2022, 2:28 PM

Change 773339 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/core@master] ParamValidator: Handle array url parameters on validation

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

Change 772508 merged by jenkins-bot:

[mediawiki/core@master] ParamValidator: Disallow array values in integer param

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

ReleaseTaggerBot edited projects, added MW-1.39-notes (1.39.0-wmf.5; 2022-03-28); removed MW-1.39-notes (1.39.0-wmf.3; 2022-03-22).Mar 25 2022, 6:00 PM