From e6ff1ff9153fa218015fbdaa5d5c55b0d57008de Mon Sep 17 00:00:00 2001
From: Timo Tijhof <krinklemail@gmail.com>
Date: Thu, 26 Sep 2019 18:34:17 +0100
Subject: [PATCH] resourceloader: Add $modules parameter to makeVersionQuery()

The 'version' and 'modules' parameters are a somewhat problematic
part of the ResourceLoaderContext object as we often pass around
the context but may be dealing with only a subset of the modules
in the outer request, or even entirely different ones (e.g.
for OutputPage's fake context 'modules' and 'version' are both
null).

This is already visible in ClientHtml where we create a derivative
context just to call setModules() and have makeVersionQuery()
work. This is now fixed.

This change is in preparation for use in ResourceLoaderImage
(to fix T233343) where we'll need to compute a version for only
1 module of a larger set, and ideally without needing to create
another context.

Bug: T233343
Change-Id: Icc1a4fd1f58c4e49e58eee43ca4ba2de6cfffc76
---
 includes/resourceloader/ResourceLoader.php      | 17 +++++++++++------
 .../resourceloader/ResourceLoaderClientHtml.php |  7 ++++---
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php
index d959ff6313..0d546fa50a 100644
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -689,24 +689,29 @@ class ResourceLoader implements LoggerAwareInterface {
 	 *
 	 * @since 1.28
 	 * @param ResourceLoaderContext $context
+	 * @param string[]|null $modules
 	 * @return string Hash
 	 */
-	public function makeVersionQuery( ResourceLoaderContext $context ) {
+	public function makeVersionQuery( ResourceLoaderContext $context, array $modules = null ) {
+		if ( $modules === null ) {
+			wfDeprecated( __METHOD__ . ' without $modules', '1.34' );
+			$modules = $context->getModules();
+		}
 		// As of MediaWiki 1.28, the server and client use the same algorithm for combining
 		// version hashes. There is no technical reason for this to be same, and for years the
 		// implementations differed. If getCombinedVersion in PHP (used for StartupModule and
 		// E-Tag headers) differs in the future from getCombinedVersion in JS (used for 'version'
 		// query parameter), then this method must continue to match the JS one.
-		$moduleNames = [];
-		foreach ( $context->getModules() as $name ) {
+		$filtered = [];
+		foreach ( $modules as $name ) {
 			if ( !$this->getModule( $name ) ) {
 				// If a versioned request contains a missing module, the version is a mismatch
 				// as the client considered a module (and version) we don't have.
 				return '';
 			}
-			$moduleNames[] = $name;
+			$filtered[] = $name;
 		}
-		return $this->getCombinedVersion( $context, $moduleNames );
+		return $this->getCombinedVersion( $context, $filtered );
 	}
 
 	/**
@@ -863,7 +868,7 @@ class ResourceLoader implements LoggerAwareInterface {
 		// - Version mismatch (T117587, T47877)
 		if ( is_null( $context->getVersion() )
 			|| $errors
-			|| $context->getVersion() !== $this->makeVersionQuery( $context )
+			|| $context->getVersion() !== $this->makeVersionQuery( $context, $context->getModules() )
 		) {
 			$maxage = $rlMaxage['unversioned']['client'];
 			$smaxage = $rlMaxage['unversioned']['server'];
diff --git a/includes/resourceloader/ResourceLoaderClientHtml.php b/includes/resourceloader/ResourceLoaderClientHtml.php
index bf03e49ad5..d98d86bdc7 100644
--- a/includes/resourceloader/ResourceLoaderClientHtml.php
+++ b/includes/resourceloader/ResourceLoaderClientHtml.php
@@ -436,7 +436,8 @@ JAVASCRIPT;
 
 				// Link/embed each set
 				foreach ( $moduleSets as list( $embed, $moduleSet ) ) {
-					$context->setModules( array_keys( $moduleSet ) );
+					$moduleSetNames = array_keys( $moduleSet );
+					$context->setModules( $moduleSetNames );
 					if ( $embed ) {
 						// Decide whether to use style or script element
 						if ( $only == ResourceLoaderModule::TYPE_STYLES ) {
@@ -456,10 +457,10 @@ JAVASCRIPT;
 						// This should NOT be done for the site group (T29564) because anons get that too
 						// and we shouldn't be putting timestamps in CDN-cached HTML
 						if ( $group === 'user' ) {
-							// Must setModules() before makeVersionQuery()
-							$context->setVersion( $rl->makeVersionQuery( $context ) );
+							$context->setVersion( $rl->makeVersionQuery( $context, $moduleSetNames ) );
 						}
 
+						// Must setModules() before createLoaderURL()
 						$url = $rl->createLoaderURL( $source, $context, $extraQuery );
 
 						// Decide whether to use 'style' or 'script' element
-- 
2.20.1