Page MenuHomePhabricator

Invalid message parameter CommentStoreComment from EventPresentationModel.php
Closed, ResolvedPublic

Description

Error

Request URL: GET /w/api.php?…
Request ID: XOAvHQpAAEsAACjWhGcAAABX

message
PHP Warning: htmlspecialchars() expects parameter 1 to be string, object given
trace
#0 /srv/mediawiki/php-1.34.0-wmf.5/includes/Message.php(1344): MWExceptionHandler::handleError(integer, string, string, integer, array, array)
#1 /srv/mediawiki/php-1.34.0-wmf.5/includes/Message.php(1202): Message->formatPlaintext(CommentStoreComment, string)
#2 /srv/mediawiki/php-1.34.0-wmf.5/includes/Message.php(1154): Message->extractParam(array, string)
#3 /srv/mediawiki/php-1.34.0-wmf.5/includes/Message.php(886): Message->replaceParameters(string, string, string)
#4 /srv/mediawiki/php-1.34.0-wmf.5/includes/Message.php(992): Message->toString(string)
#5 /srv/mediawiki/php-1.34.0-wmf.5/extensions/Echo/includes/formatters/EventPresentationModel.php(533): Message->escaped()
#6 /srv/mediawiki/php-1.34.0-wmf.5/extensions/Echo/includes/formatters/EchoModelFormatter.php(13): EchoEventPresentationModel->jsonSerialize()
#7 /srv/mediawiki/php-1.34.0-wmf.5/extensions/Echo/includes/formatters/EchoEventFormatter.php(72): EchoModelFormatter->formatModel(EchoEditUserTalkPresentationModel)
#8 /srv/mediawiki/php-1.34.0-wmf.5/extensions/Echo/includes/DataOutputFormatter.php(189): EchoEventFormatter->format(EchoEvent)
#9 /srv/mediawiki/php-1.34.0-wmf.5/extensions/Echo/includes/DataOutputFormatter.php(149): EchoDataOutputFormatter::formatNotification(EchoEvent, User, string, LanguageEn)
#10 /srv/mediawiki/php-1.34.0-wmf.5/extensions/Echo/includes/DataOutputFormatter.php(167): EchoDataOutputFormatter::formatOutput(EchoNotification, string, User, LanguageEn)
#11 [internal function]: Closure$EchoDataOutputFormatter::formatOutput#3(EchoNotification)
#12 /srv/mediawiki/php-1.34.0-wmf.5/extensions/Echo/includes/DataOutputFormatter.php(170): array_map(Closure$EchoDataOutputFormatter::formatOutput#3;2309, array)
#13 /srv/mediawiki/php-1.34.0-wmf.5/extensions/Echo/includes/api/ApiEchoNotifications.php(289): EchoDataOutputFormatter::formatOutput(EchoNotification, string, User, LanguageEn)
#14 /srv/mediawiki/php-1.34.0-wmf.5/extensions/Echo/includes/api/ApiEchoNotifications.php(109): ApiEchoNotifications->getPropList(User, array, array, integer, NULL, string, NULL, boolean, boolean)
#15 /srv/mediawiki/php-1.34.0-wmf.5/extensions/Echo/includes/api/ApiEchoNotifications.php(40): ApiEchoNotifications->getLocalNotifications(array)
#16 /srv/mediawiki/php-1.34.0-wmf.5/includes/api/ApiQuery.php(249): ApiEchoNotifications->execute()
#17 /srv/mediawiki/php-1.34.0-wmf.5/includes/api/ApiMain.php(1593): ApiQuery->execute()
#18 /srv/mediawiki/php-1.34.0-wmf.5/includes/api/ApiMain.php(531): ApiMain->executeAction()
#19 /srv/mediawiki/php-1.34.0-wmf.5/includes/api/ApiMain.php(502): ApiMain->executeActionWithErrorHandling()
#20 /srv/mediawiki/php-1.34.0-wmf.5/api.php(87): ApiMain->execute()
#21 /srv/mediawiki/w/api.php(3): include(string)

Impact

Unknown.

I suspect users are seeing pages with the incorrect content shown. Instead of seeing an edit summary, they are probably seeing the word "object" or "CommentStoreComment" instead due to PHP being forced to cast the object to a string.

Notes

This is a high-confidence regression that started with wmf.5 this week.

Event Timeline

Krinkle created this task.May 18 2019, 4:33 PM
Restricted Application added a project: Growth-Team. · View Herald TranscriptMay 18 2019, 4:33 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Krinkle triaged this task as High priority.May 18 2019, 4:33 PM

Change 511742 had a related patch set uploaded (by Sbisson; owner: Sbisson):
[mediawiki/extensions/Echo@master] Don't add CommentStoreComment as plaintext params

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

SBisson claimed this task.May 21 2019, 3:51 PM
SBisson moved this task from Incoming to Code Review on the Growth-Team (Current Sprint) board.
SBisson added a subscriber: SBisson.

@Krinkle I think the patch above will fix the log spamming and prevent new Echo events from being created with a CommentStoreComment. Faulty events would stay in place but eventually expire and be deleted.

Let me know what you think on the patch. Other Growth team engineers are not available until the end of the week so review and merge might be delayed but if we are both comfortable with the patch (or a future version of it) I can SWAT it tomorrow.

I should have searched in Phabricator before filling another task. Thank you @SBisson to have spotted it!

@SBisson Thanks, LGTM. Per CR on the patch that caused it, there is similar problem further down in the same patch, which will need fixing as well!

Change 511742 merged by jenkins-bot:
[mediawiki/extensions/Echo@master] Don't add CommentStoreComment as plaintext params

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

Change 512070 had a related patch set uploaded (by Krinkle; owner: Sbisson):
[mediawiki/extensions/Echo@wmf/1.34.0-wmf.6] Don't add CommentStoreComment as plaintext params

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

Per CR on the patch that caused it, there is similar problem further down in the same patch, which will need fixing as well!

@D3r1ck01 are you interested in making a follow-up patch to address this comment?

Change 512070 merged by jenkins-bot:
[mediawiki/extensions/Echo@wmf/1.34.0-wmf.6] Don't add CommentStoreComment as plaintext params

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

The fix for this is now merged into wmf.6 and deployed everywhere. The error should NOT appear in the log anymore.

Krinkle closed this task as Resolved.May 26 2019, 1:06 PM
Krinkle removed a project: Patch-For-Review.

Confirmed in Logstash. From 20-50 reports per hour, to 0, as of May 23.

mmodell changed the subtype of this task from "Task" to "Production Error".Wed, Aug 28, 11:07 PM