Page MenuHomePhabricator

DeferredDescriptionUpdate.php --force: use of array_shift() converts array into a string leading to a TypeError
Closed, ResolvedPublicBUG REPORT

Description

Steps to replicate the issue (include links if applicable):

  • Run php maintenance/DeferredDescriptionUpdate.php --force

What happens?:

[61f4a2bcf066c55a13557817] [no req]   TypeError: count(): Argument #1 ($value) must be of type Countable|array, string given
Backtrace:
from w/extensions/WikiSEO/includes/DeferredDescriptionUpdate.php(109)
#0 w/extensions/WikiSEO/includes/DeferredDescriptionUpdate.php(109): count()
#1 w/extensions/WikiSEO/maintenance/GenerateDescription.php(94): MediaWiki\Extension\WikiSEO\DeferredDescriptionUpdate->doUpdate()
#2 w/maintenance/doMaintenance.php(114): GenerateDescription->execute()
#3 w/extensions/WikiSEO/maintenance/GenerateDescription.php(102): require_once(string)
#4 {main}

What should have happened instead?:
The maintenance script should have finished successfully.

Software version (skip for WMF-hosted wikis like Wikipedia):
1.38.2 with WikiSEO snapshot from GitHub.

Other information (browser name/version, screenshots, etc.):
N/A

Adding var_dump() prior and after

$propertyDescription = array_shift( $propertyDescription ) ?? [];

results in

array(1) {
  [6]=>
  string(137) "…"
}
string(137) "…"

This is followed by

case count( $propertyDescription ) > 1:

Event Timeline

IijimaYun renamed this task from DeferredDescriptionUpdate.php --force: array_shift() converts array into a string leading to a TypeError to DeferredDescriptionUpdate.php --force: use of array_shift() converts array into a string leading to a TypeError.Jul 20 2022, 3:39 PM
IijimaYun updated the task description. (Show Details)

I feel like array_shift() is out of place here: even if the assignment is replaced by a simple call, dropping the first element completely, it obviously makes the supposed later test

switch ( true ) {
                        case count( $propertyDescription ) > 1:
                                // There are multiple page props with the name 'description' present
                                // This shouldn't happen, but we'll try to clean it here
                                $db->delete(
                                        'page_props',
                                        [
                                                'pp_page' => $this->title->getArticleID(),
                                                'pp_propname' => 'description',
                                        ]
                                );
                        // Intentional fall-through, as deleting all 'description' props requires inserting a new row
                        case empty( $propertyDescription ):
                                $shouldInsert = true;
                                break;

                        default:
                                break;
                }

fail if there are two descriptions returned.

Change 815732 had a related patch set uploaded (by Iijima Yun; author: Iijima Yun):

[mediawiki/extensions/WikiSEO@master] T313419: DeferredDescriptionUpdate.php --force: fix type errors related to `array_shift()` call

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

Change 815732 had a related patch set uploaded (by Iijima Yun; author: Iijima Yun):

[mediawiki/extensions/WikiSEO@master] DeferredDescriptionUpdate.php --force: fix type errors related to `array_shift()` call

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

I think this was fixed a while back. At least when running php extensions/WikiSeo/maintenance/GenerateDescription.php --force 0 on MW 1.39 no such exception occurs.