From: Florianschmidtwelzow <florian.schmidt.stargatewissen@gmail.com>
Date: Mon, 28 Aug 2017 15:03:47 +0000 (+0000)
Subject: registration: Only allow one extension to set a specific config setting
X-Git-Tag: 1.31.0-rc.0~1686^2
X-Git-Url: https://git.heureux-cyclage.org/?p=lhc%2Fweb%2Fwiklou.git;a=commitdiff_plain;h=29a5fc72e3ab7d3da28bc4cc91d4bb051b9a690a;hp=236456777b920047b8b8502628143b218c486d8b

registration: Only allow one extension to set a specific config setting

ExtensionProcessor would previously just blindly overwrite duplicate
config settings, which ends up depending upon load order.

It's relatively hard to debug since it is silently overwritten. This now
throws exceptions in case of duplicate config settings.

This will also have some side-effects of catching people putting things
like "ResourceModules" in their "config" section when it should be a
top-level item.

Bug: T152929
Depends-On: I4c5eaf87657f5dc07787480a2f1a56a1db8c714f
Change-Id: Ieeb26011e42c741041d2c3252238ca0823b99eb4
---

diff --git a/includes/registration/ExtensionProcessor.php b/includes/registration/ExtensionProcessor.php
index ce262bd23e..14d822295c 100644
--- a/includes/registration/ExtensionProcessor.php
+++ b/includes/registration/ExtensionProcessor.php
@@ -450,7 +450,7 @@ class ExtensionProcessor implements Processor {
 			}
 			foreach ( $info['config'] as $key => $val ) {
 				if ( $key[0] !== '@' ) {
-					$this->globals["$prefix$key"] = $val;
+					$this->addConfigGlobal( "$prefix$key", $val );
 				}
 			}
 		}
@@ -478,11 +478,26 @@ class ExtensionProcessor implements Processor {
 				if ( isset( $data['path'] ) && $data['path'] ) {
 					$value = "$dir/$value";
 				}
-				$this->globals["$prefix$key"] = $value;
+				$this->addConfigGlobal( "$prefix$key", $value );
 			}
 		}
 	}
 
+	/**
+	 * Helper function to set a value to a specific global, if it isn't set already.
+	 *
+	 * @param string $key The config key with the prefix and anything
+	 * @param mixed $value The value of the config
+	 */
+	private function addConfigGlobal( $key, $value ) {
+		if ( array_key_exists( $key, $this->globals ) ) {
+			throw new RuntimeException(
+				"The configuration setting '$key' was already set by another extension,"
+				. " and cannot be set again." );
+		}
+		$this->globals[$key] = $value;
+	}
+
 	protected function extractServiceWiringFiles( $dir, array $info ) {
 		if ( isset( $info['ServiceWiringFiles'] ) ) {
 			foreach ( $info['ServiceWiringFiles'] as $path ) {
diff --git a/tests/phpunit/includes/registration/ExtensionProcessorTest.php b/tests/phpunit/includes/registration/ExtensionProcessorTest.php
index 7b56def1c3..5ef30e872a 100644
--- a/tests/phpunit/includes/registration/ExtensionProcessorTest.php
+++ b/tests/phpunit/includes/registration/ExtensionProcessorTest.php
@@ -220,6 +220,48 @@ class ExtensionProcessorTest extends MediaWikiTestCase {
 		$this->assertEquals( 'somevalue', $extracted['globals']['egBar'] );
 	}
 
+	/**
+	 * @covers ExtensionProcessor::addConfigGlobal()
+	 * @expectedException RuntimeException
+	 */
+	public function testDuplicateConfigKey1() {
+		$processor = new ExtensionProcessor;
+		$info = [
+			'config' => [
+				'Bar' => '',
+			]
+		] + self::$default;
+		$info2 = [
+			'config' => [
+				'Bar' => 'g',
+			],
+			'name' => 'FooBar2',
+		];
+		$processor->extractInfo( $this->dir, $info, 1 );
+		$processor->extractInfo( $this->dir, $info2, 1 );
+	}
+
+	/**
+	 * @covers ExtensionProcessor::addConfigGlobal()
+	 * @expectedException RuntimeException
+	 */
+	public function testDuplicateConfigKey2() {
+		$processor = new ExtensionProcessor;
+		$info = [
+			'config' => [
+				'Bar' => [ 'value' => 'somevalue' ],
+			]
+		] + self::$default;
+		$info2 = [
+			'config' => [
+				'Bar' => [ 'value' => 'somevalue' ],
+			],
+			'name' => 'FooBar2',
+		];
+		$processor->extractInfo( $this->dir, $info, 2 );
+		$processor->extractInfo( $this->dir, $info2, 2 );
+	}
+
 	public static function provideExtractExtensionMessagesFiles() {
 		$dir = __DIR__ . '/FooBar/';
 		return [