Page MenuHomePhabricator

PHP Fatal Error: Argument 1 passed to SpecialCollection::postZip() must be an instance of array, bool given
Closed, ResolvedPublic

Description

Error

Request ID: XIErJgpAMFQAAC@p1YMAAACN

message
PHP Fatal Error: Argument 1 passed to SpecialCollection::postZip() must be an instance of array, bool given
trace
#0 /srv/mediawiki/php-1.33.0-wmf.20/extensions/Collection/Collection.body.php(235): NO_FUNCTION_GIVEN()
#1 /srv/mediawiki/php-1.33.0-wmf.20/includes/specialpage/SpecialPage.php(569): SpecialCollection->execute(NULL)
#2 /srv/mediawiki/php-1.33.0-wmf.20/includes/specialpage/SpecialPageFactory.php(558): SpecialPage->run(NULL)
#3 /srv/mediawiki/php-1.33.0-wmf.20/includes/MediaWiki.php(288): MediaWiki\Special\SpecialPageFactory->executePath(Title, RequestContext)
#4 /srv/mediawiki/php-1.33.0-wmf.20/includes/MediaWiki.php(867): MediaWiki->performRequest()
#5 /srv/mediawiki/php-1.33.0-wmf.20/includes/MediaWiki.php(517): MediaWiki->main()
#6 /srv/mediawiki/php-1.33.0-wmf.20/index.php(42): MediaWiki->run()
#7 /srv/mediawiki/w/index.php(3): include(string)
#8 {main}

Impact

Apparently raised from: https://pt.wikibooks.org/w/index.php?title=Especial:Livro&bookcmd=order_collection&colltitle=Wikilivros%3ALivros%2FEstado%2Bmoderno&partner=pediapress

Notes

Event Timeline

hashar created this task.Mar 7 2019, 2:47 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 7 2019, 2:47 PM

Change 497134 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/Collection@master] Add missing false check before doing a bad API request

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

Oh dear. I would like to point out that it was not really the type hint I added who is creating an issue here. What the type hint did was making an existing issue visible. The actual issue is this code:

$title = Title::newFromText( $request->getVal( 'colltitle', '' ) );
if ( !$title ) {
    return;
}
$collection = $this->loadCollection( $title );
// …
$this->postZip( $collection, $partner );

Along with this code:

private function loadCollection( $title, $append = false ) {
    if ( !$title->exists() ) {
        // …
        return false;
    }

What this means:

  • When the user provides a string that is not a valid title, it is silently ignored.
  • When the user provides a valid page name that doesn't exist, postZip() is called with $collection set to false, which triggered an inherently broken API request before.

This misbehavior is now blocked by the type hint.

I can see multiple ways to fix this.

  • Silently ignore non-existing pages as well by changing the first condition to if ( !$title || !$title->exists() ).
  • Add a check for if ( $collection ) to the postZip() call.

I feel the later is an acceptable fix for now, because 3 calls to loadCollection() exist, and two have such a check already.

Change 497134 had a related patch set uploaded (by Umherirrender; owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/Collection@master] Add missing false check before doing a bad API request

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

Umherirrender closed this task as Resolved.Mar 20 2019, 6:41 PM
Umherirrender assigned this task to thiemowmde.
Umherirrender triaged this task as Normal priority.

Change 497134 merged by jenkins-bot:
[mediawiki/extensions/Collection@master] Add missing false check before doing a bad API request

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

Change 498229 had a related patch set uploaded (by Krinkle; owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/Collection@wmf/1.33.0-wmf.22] Add missing false check before doing a bad API request

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

Change 498229 merged by jenkins-bot:
[mediawiki/extensions/Collection@wmf/1.33.0-wmf.22] Add missing false check before doing a bad API request

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

Mentioned in SAL (#wikimedia-operations) [2019-03-22T18:41:15Z] <krinkle@deploy1001> Synchronized php-1.33.0-wmf.22/extensions/Collection/: I2c4f5d005fc25c52 / T217835 (duration: 00m 52s)

Task description

After the fix, this no longer fatals (yay). Instead, it now tries to render a user interface message (yay), but the message key doesn't exist (oops?).

Livro não foi encontrado
⧼coll-notfound_msg⧽

Pre-existing unrelated issue, so won't re-open.

Change 498633 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/Collection@master] Fix broken "coll-notfound_msg" error message

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

Change 498633 merged by jenkins-bot:
[mediawiki/extensions/Collection@master] Fix broken "coll-notfound_msg" error message

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

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