Page MenuHomePhabricator

Initialization in of an empty array in extension.json overwrites integer keys in LocalSettings.php
Closed, InvalidPublic

Description

extensions/Example/extension.json:

{
	"name": "Example",
	"config": {
		"DemonstrateBug": {
			"value": []
		}
	},
	"manifest_version": 2
}

LocalSettings.php:

wfLoadExtension( 'Example' );
$wgDemonstrateBug['foo'] = 'bar';
$wgDemonstrateBug[NS_PROJECT] = 'baz';
$wgHooks["getUserPermissionsErrors"][] = function() {
	global $wgDemonstrateBug;
	var_dump($wgDemonstrateBug);
	exit;
};

The second key (NS_PROJECT == 4) is expected to remain:

.../LocalSettings.php:46:
array (size=2)
  'foo' => string 'bar' (length=3)
  4 => string 'baz' (length=3)

Instead, it is overwritten with 0 in the actual output:

.../LocalSettings.php:46:
array (size=2)
  'foo' => string 'bar' (length=3)
  0 => string 'baz' (length=3)

Event Timeline

Also, worth pointing out that T148582#4624990 is where I discovered this.

Tgr added a subscriber: Tgr.

The default merge strategy does not preserve integer keys. This is not a bug (although the documentation is obscure). The variable should be defined as

"DemonstrateBug": {
    "value": [],
    "merge_strategy": "array_plus"
}

Thanks, @Tgr. That said, I found this bug in schema v1 (which is the only version available in MW 1.27).

Testing against 1.31:

extension.json:

{
	"name": "Example",
	"config": {
		"DemonstrateBug": {
			"value": [],
			"_merge_strategy": "array_plus"

		}
	},
	"manifest_version": 1
}

results in

.../LocalSettings.php:46:
array (size=3)
  'foo' => string 'bar' (length=3)
  4 => string 'baz' (length=3)
  'value' => 
    array (size=0)
      empty

This could work, but the extra value key is a bug that needs fixing.

In v1 it would be

"DemonstrateBug": {
    "_merge_strategy": "array_plus"
}

or if you do have default values

"DemonstrateBug": {
    "_merge_strategy": "array_plus",
    "0": "foo",
    "1": "bar"
}

ie. the the whole config item is the initial value (with the special _merge_strategy key removed).

The documentation is a mess, filed T205785: Extension registration needs better documentation about that.