ResourceLoader: Limit injection of valid skins to names only

Follows-up I0191a812f044a5c04c:

* $installedSkins was named after MediaWiki's SkinFactory and its
  return value was not a list of skins, or actually a registry
  of skin names and PHP classes (subclasses of "Skin"). This
  information is of no use to ResourceLoader as it cannot construct
  skin objects (skins would require an RequestContext such used on
  index.php).

  Change its format to an actual list, and rename to reflect its
  local purpose.

* Simplify logic to: If not set, or, we do validation and the
  value is not valid.

* Update docs to explain what "perform validation" means.

Change-Id: Ic8544d99055a7b7c7e55496006b5971e4e4fa14f

3 files changed, 18 insertions, 18 deletions

diff --git a/includes/ResourceLoader/Context.php b/includes/ResourceLoader/Context.php
index a4dfe8522c3f..a523cafe9b87 100644
--- a/includes/ResourceLoader/Context.php
+++ b/includes/ResourceLoader/Context.php
@@ -102,12 +102,11 @@ class Context implements MessageLocalizer {
 	/**
 	 * @param ResourceLoader $resourceLoader
 	 * @param WebRequest $request
-	 * @param array|null $installedSkins If a list of skins are supplied, perform
-	 *   validation with them. But if we don't have a list of skins to validate on
-	 *   and a skin is not supplied, also fallback to the default skin.
+	 * @param string[]|null $validSkins List of valid skin names. If not passed,
+	 *   any skin name is considered valid. Invalid skins are replaced by the default.
 	 */
 	public function __construct(
-		ResourceLoader $resourceLoader, WebRequest $request, $installedSkins = null
+		ResourceLoader $resourceLoader, WebRequest $request, $validSkins = null
 	) {
 		$this->resourceLoader = $resourceLoader;
 		$this->request = $request;
@@ -135,14 +134,12 @@ class Context implements MessageLocalizer {
 
 		$this->skin = $request->getRawVal( 'skin' );
 
-		if ( is_array( $installedSkins ) ) {
-			if ( !$this->skin || !isset( $installedSkins[$this->skin] ) ) {
-				// The 'skin' parameter is required. (Not yet enforced.)
-				// For requests without a known skin specified,
-				// use MediaWiki's 'fallback' skin for skin-specific decisions.
-				$this->skin = self::DEFAULT_SKIN;
-			}
-		} elseif ( !$this->skin ) {
+		if (
+			!$this->skin
+			|| ( is_array( $validSkins ) && !in_array( $this->skin, $validSkins ) )
+		) {
+			// For requests without a known skin specified,
+			// use MediaWiki's 'fallback' skin for any skin-specific decisions.
 			$this->skin = self::DEFAULT_SKIN;
 		}
 	}
diff --git a/includes/ResourceLoader/ResourceLoaderEntryPoint.php b/includes/ResourceLoader/ResourceLoaderEntryPoint.php
index 8fbe2c566aec..e05aa1576ee8 100644
--- a/includes/ResourceLoader/ResourceLoaderEntryPoint.php
+++ b/includes/ResourceLoader/ResourceLoaderEntryPoint.php
@@ -47,7 +47,7 @@ class ResourceLoaderEntryPoint extends MediaWikiEntryPoint {
 		$context = new Context(
 			$resourceLoader,
 			$this->getRequest(),
-			$services->getSkinFactory()->getInstalledSkins()
+			array_keys( $services->getSkinFactory()->getInstalledSkins() )
 		);
 
 		// Respond to ResourceLoader request
diff --git a/tests/phpunit/includes/ResourceLoader/ContextTest.php b/tests/phpunit/includes/ResourceLoader/ContextTest.php
index cd04dbc7f76e..c570f7addfa4 100644
--- a/tests/phpunit/includes/ResourceLoader/ContextTest.php
+++ b/tests/phpunit/includes/ResourceLoader/ContextTest.php
@@ -205,17 +205,20 @@ class ContextTest extends TestCase {
 	public static function skinsProvider(): Generator {
 		// expected skin, supplied skin, installed skins
 		yield 'keep validated' => [
-			'example', [ 'skin' => 'example' ],
-			[ 'example' => 'ExampleSkin', 'foo' => 'FooSkin', 'bar' => 'BarSkin' ]
+			'example',
+			[ 'skin' => 'example' ],
+			[ 'example', 'foo', 'bar' ]
 		];
 
 		yield 'fallback invalid' => [
-			'fallback', [ 'skin' => 'not-example' ],
-			[ 'example' => 'ExampleSkin', 'foo' => 'FooSkin', 'bar' => 'BarSkin' ]
+			'fallback',
+			[ 'skin' => 'not-example' ],
+			[ 'example', 'foo', 'bar' ]
 		];
 
 		yield 'keep anything without validation' => [
-			'not-example', [ 'skin' => 'not-example' ],
+			'not-example',
+			[ 'skin' => 'not-example' ],
 			null
 		];
 	}