Maniphest T205782

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

Assigned To

None

Authored By

	MarkAHershberger
	Sep 29 2018, 6:52 PM

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)

Related Objects

Mentioned Here: T205785: Extension registration needs better documentation
T148582: Lockdown blocks extensions relying on API interaction

Event Timeline

MarkAHershberger created this task.Sep 29 2018, 6:52 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 29 2018, 6:52 PM

See this commit for initial discussion.

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

MarkAHershberger updated the task description. (Show Details)Sep 29 2018, 7:06 PM

MGChecker updated the task description. (Show Details)Sep 29 2018, 8:50 PM

MarkAHershberger updated the task description. (Show Details)Sep 29 2018, 8:54 PM

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.

Initialization in of an empty array in extension.json overwrites integer keys in LocalSettings.phpClosed, InvalidPublicActions

Description

Related Objects

Event Timeline

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