Page MenuHomePhabricator

Blocked MobileFrontend merges: Calling method \ApiMobileView::parseSectionsData in \ApiMobileView::getData that is always unsafe
Closed, ResolvedPublic

Description

A new phan-taint-plugin related error is blocking merges in MobileFrontend.

<?xml version="1.0" encoding="ISO-8859-15"?>
15:37:04 <checkstyle version="6.5">
15:37:04   <file name="./includes/api/ApiMobileView.php">
15:37:04     <error line="660" severity="warning" message="Calling method \ApiMobileView::parseSectionsData in \ApiMobileView::getData that is always unsafe  (Caused by: ./includes/api/ApiMobileView.php +550; ./includes/api/ApiMobileView.php +529; ./includes/api/ApiMobileView.php +528)" source="SecurityCheck-XSS"/>
15:37:04   </file>
15:37:04 </checkstyle>
15:37:05 Build step 'Execute shell' marked build as failure
15:37:05 [CHECKSTYLE] Collecting checkstyle analysis files...
15:37:05 [CHECKSTYLE] Searching for all files in /srv/jenkins-workspace/workspace/mwext-php70-phan-seccheck-docker that match the pattern src/seccheck-issues
15:37:05 [CHECKSTYLE] Parsing 1 file in /srv/jenkins-workspace/workspace/mwext-php70-phan-seccheck-docker
15:37:05 [CHECKSTYLE] Successfully parsed file /srv/jenkins-workspace/workspace/mwext-php70-phan-seccheck-docker/src/seccheck-issues with 1 unique warning and 0 duplicates.
15:37:05 [CHECKSTYLE] Computing warning deltas based on reference build #13268
15:37:05 [CHECKSTYLE] Ignore new warnings since this is the first valid build
15:37:05 [CHECKSTYLE] Plug-in Result: Success - no threshold has been exceeded
15:37:06 Finished: FAILURE

Event Timeline

Jdlrobson created this task.Sep 4 2018, 5:48 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 4 2018, 5:48 PM
Jdlrobson triaged this task as High priority.Sep 4 2018, 5:48 PM
Bawolff added a subscriber: Bawolff.Sep 4 2018, 7:57 PM

Its a bug in the plugin. I think i know whats causing it and its next up to work on.

In the mean time you can @suppress the error

Change 457994 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Temporarily suppress SecurityCheck-XSS error

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

Change 457994 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Temporarily suppress SecurityCheck-XSS error

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

Immediate issue resolved. The bug remains.
Thanks for the pointer @Bawolff !

Change 458334 had a related patch set uploaded (by Brian Wolff; owner: Brian Wolff):
[mediawiki/tools/phan/SecurityCheckPlugin@master] Fix some confusion over which group of taints to mask out in various places

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

Immediate issue resolved. The bug remains.
Thanks for the pointer @Bawolff !

Thank you for your patience as we work out the kinks.

Change 458334 merged by jenkins-bot:
[mediawiki/tools/phan/SecurityCheckPlugin@master] Fix some confusion over which group of taints to mask out in various places

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

Bawolff closed this task as Resolved.Sep 13 2018, 3:32 AM
Bawolff claimed this task.