Florianschmidtwelzow has uploaded a new change for review. ( 
https://gerrit.wikimedia.org/r/348577 )

Change subject: Prevent setting an extension configuration twice
......................................................................

Prevent setting an extension configuration twice

The extension.json should hold the definition of configuration options
provided by the extension, not values that are suggested or required to
configuration options provided by other extensions, which would overwrite
the whole default value of the extension, which provides this configuration,
but this is highly dependant to the order how the extensions are loaded.

It is hard to track a problem down to this reason (that a config value is over-
written by another extension). Because extension registration isn't mean to
support these cases, it will now throw an error, if a config option will be
set, but the option key already exist (and was set by another extension).

Bug: T152929
Change-Id: Iaef32efab397e82ff70ddca8ac79c545c5b7d2bb
---
M includes/registration/ExtensionProcessor.php
M tests/phpunit/includes/registration/ExtensionProcessorTest.php
2 files changed, 59 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/77/348577/1

diff --git a/includes/registration/ExtensionProcessor.php 
b/includes/registration/ExtensionProcessor.php
index 1212f99..103e1d7 100644
--- a/includes/registration/ExtensionProcessor.php
+++ b/includes/registration/ExtensionProcessor.php
@@ -376,7 +376,7 @@
                        }
                        foreach ( $info['config'] as $key => $val ) {
                                if ( $key[0] !== '@' ) {
-                                       $this->globals["$prefix$key"] = $val;
+                                       $this->addConfigGlobal( "$prefix$key", 
$val );
                                }
                        }
                }
@@ -404,11 +404,26 @@
                                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 ( isset( $this->globals[$key] ) ) {
+                       throw new InvalidArgumentException(
+                               'Duplicate configuration key detected: The 
configuration option ' . $key .
+                               ' was already set by another extension and can 
not 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 71f760d..f091662 100644
--- a/tests/phpunit/includes/registration/ExtensionProcessorTest.php
+++ b/tests/phpunit/includes/registration/ExtensionProcessorTest.php
@@ -166,6 +166,48 @@
                $this->assertEquals( 'somevalue', 
$extracted['globals']['egBar'] );
        }
 
+       /**
+        * @covers ExtensionProcessor::addConfigGlobal()
+        * @expectedException InvalidArgumentException
+        */
+       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 InvalidArgumentException
+        */
+       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 [

-- 
To view, visit https://gerrit.wikimedia.org/r/348577
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iaef32efab397e82ff70ddca8ac79c545c5b7d2bb
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Florianschmidtwelzow <florian.schmidt.stargatewis...@gmail.com>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to