registration: Validate no duplicate keys exist in extension.json

The JSON specification permits duplicate keys, but doing so is almost
always nearly a mistake.

We can use the JSON lint library (added as
dev-requirement to composer), which is now also used to decode the
JSON (as it's the same step as validating the JSON).

Bug: T153507
Change-Id: Ia713a1906169333c1aa2aebdc0ed060d26428d72

diff --git a/composer.json b/composer.json
index f8de4c3..3cd5ea3 100644 (file)
--- a/composer.json
+++ b/composer.json
@@ -63,6 +63,7 @@
                "mediawiki/mediawiki-codesniffer": "20.0.0",
                "monolog/monolog": "~1.22.1",
                "nikic/php-parser": "3.1.3",
+               "seld/jsonlint": "1.7.1",
                "nmred/kafka-php": "0.1.5",
                "phpunit/phpunit": "4.8.36 || ^6.5",
                "psy/psysh": "0.9.6",

diff --git a/includes/registration/ExtensionJsonValidator.php b/includes/registration/ExtensionJsonValidator.php
index 564ea6b..7d59a02 100644 (file)
--- a/includes/registration/ExtensionJsonValidator.php
+++ b/includes/registration/ExtensionJsonValidator.php
@@ -21,6 +21,8 @@
 
 use Composer\Spdx\SpdxLicenses;
 use JsonSchema\Validator;
+use Seld\JsonLint\JsonParser;
+use Seld\JsonLint\ParsingException;
 
 /**
  * @since 1.29
@@ -54,6 +56,10 @@ class ExtensionJsonValidator {
                                'The spdx-licenses library cannot be found, please install it through composer.'
                        );
                        return false;
+               } elseif ( !class_exists( JsonParser::class ) ) {
+                       call_user_func( $this->missingDepCallback,
+                               'The JSON lint library cannot be found, please install it through composer.'
+                       );
                }
 
                return true;
@@ -65,8 +71,14 @@ class ExtensionJsonValidator {
         * @throws ExtensionJsonValidationError on any failure
         */
        public function validate( $path ) {
-               $data = json_decode( file_get_contents( $path ) );
-               if ( !is_object( $data ) ) {
+               $contents = file_get_contents( $path );
+               $jsonParser = new JsonParser();
+               try {
+                       $data = $jsonParser->parse( $contents, JsonParser::DETECT_KEY_CONFLICTS );
+               } catch ( ParsingException $e ) {
+                       if ( $e instanceof \Seld\JsonLint\DuplicateKeyException ) {
+                               throw new ExtensionJsonValidationError( $e->getMessage() );
+                       }
                        throw new ExtensionJsonValidationError( "$path is not valid JSON" );
                }
 

diff --git a/tests/phpunit/data/registration/duplicate_keys.json b/tests/phpunit/data/registration/duplicate_keys.json
new file mode 100644 (file)
index 0000000..40f2f7e
--- /dev/null
+++ b/tests/phpunit/data/registration/duplicate_keys.json
@@ -0,0 +1,4 @@
+{
+       "name": "FooBar",
+       "name": "Test"
+}

diff --git a/tests/phpunit/includes/registration/ExtensionJsonValidatorTest.php b/tests/phpunit/includes/registration/ExtensionJsonValidatorTest.php
index 355f4ef..46c697f 100644 (file)
--- a/tests/phpunit/includes/registration/ExtensionJsonValidatorTest.php
+++ b/tests/phpunit/includes/registration/ExtensionJsonValidatorTest.php
@@ -52,6 +52,10 @@ class ExtensionJsonValidatorTest extends MediaWikiTestCase {
                                'notjson.txt',
                                'notjson.txt is not valid JSON'
                        ],
+                       [
+                               'duplicate_keys.json',
+                               'Duplicate key: name'
+                       ],
                        [
                                'no_manifest_version.json',
                                'no_manifest_version.json does not have manifest_version set.'