From 0980f94e131f9ffad723d237876956e14a773e3d Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Thu, 17 May 2018 20:25:49 +0200 Subject: [PATCH] resourceloader: Refactor CSP $nonce passing Follows-up 70941efd35562dcb700 which broke various public signatures of the ClientHtml class that I'd prefer to handle differently. This commit mainly restores support for all previously public signatures, and either removes the need for a parameter, or moves it to the end of the original signature (as optional param). * ClientHtml::getHeadHtml: Remove the positional/required parameter that was added. Restoring the method to being a stateless computer that requires no parameters. Pass the option via construct instead. * ClientHtml::makeLoad: - Make $nonce optional. - Restore $extraQuery as optional. * ResourceLoader::makeInlineScript: Document $nonce as optional (matching the implementation). Change-Id: Iaf33f2a060048e6606fba8d875b6d2953b21ef45 --- includes/OutputPage.php | 3 ++- includes/resourceloader/ResourceLoader.php | 2 +- .../ResourceLoaderClientHtml.php | 18 +++++++++++------- .../ResourceLoaderClientHtmlTest.php | 10 ++++++---- 4 files changed, 20 insertions(+), 13 deletions(-) diff --git a/includes/OutputPage.php b/includes/OutputPage.php index 52dfc1164e..dd1a4db0b7 100644 --- a/includes/OutputPage.php +++ b/includes/OutputPage.php @@ -2861,6 +2861,7 @@ class OutputPage extends ContextSource { $rlClient = new ResourceLoaderClientHtml( $context, [ 'target' => $this->getTarget(), + 'nonce' => $this->getCSPNonce(), ] ); $rlClient->setConfig( $this->getJSVars() ); $rlClient->setModules( $this->getModules( /*filter*/ true ) ); @@ -2907,7 +2908,7 @@ class OutputPage extends ContextSource { } $pieces[] = Html::element( 'title', null, $this->getHTMLTitle() ); - $pieces[] = $this->getRlClient()->getHeadHtml( $this->getCSPNonce() ); + $pieces[] = $this->getRlClient()->getHeadHtml(); $pieces[] = $this->buildExemptModules(); $pieces = array_merge( $pieces, array_values( $this->getHeadLinksArray() ) ); $pieces = array_merge( $pieces, array_values( $this->mHeadItems ) ); diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php index bee3d0c9d4..227479336b 100644 --- a/includes/resourceloader/ResourceLoader.php +++ b/includes/resourceloader/ResourceLoader.php @@ -1503,7 +1503,7 @@ MESSAGE; * startup module if the client has adequate support for MediaWiki JavaScript code. * * @param string $script JavaScript code - * @param string $nonce Content-security-policy nonce, from OutputPage::getCSPNonce() + * @param string $nonce [optional] Content-Security-Policy nonce (from OutputPage::getCSPNonce) * @return WrappedString HTML */ public static function makeInlineScript( $script, $nonce = null ) { diff --git a/includes/resourceloader/ResourceLoaderClientHtml.php b/includes/resourceloader/ResourceLoaderClientHtml.php index d0a9c4248a..9eae7e8976 100644 --- a/includes/resourceloader/ResourceLoaderClientHtml.php +++ b/includes/resourceloader/ResourceLoaderClientHtml.php @@ -58,11 +58,15 @@ class ResourceLoaderClientHtml { * @param ResourceLoaderContext $context * @param array $options [optional] Array of options * - 'target': Custom parameter passed to StartupModule. + * - 'nonce': From OutputPage::getCSPNonce(). */ public function __construct( ResourceLoaderContext $context, array $options = [] ) { $this->context = $context; $this->resourceLoader = $context->getResourceLoader(); - $this->options = $options; + $this->options = $options + [ + 'target' => null, + 'nonce' => null, + ]; } /** @@ -248,10 +252,10 @@ class ResourceLoaderClientHtml { * - Inline scripts can't be asynchronous. * - For styles, earlier is better. * - * @param string $nonce From OutputPage::getCSPNonce() * @return string|WrappedStringList HTML */ - public function getHeadHtml( $nonce ) { + public function getHeadHtml() { + $nonce = $this->options['nonce']; $data = $this->getData(); $chunks = []; @@ -327,7 +331,7 @@ class ResourceLoaderClientHtml { // Async scripts. Once the startup is loaded, inline RLQ scripts will run. // Pass-through a custom 'target' from OutputPage (T143066). - $startupQuery = isset( $this->options['target'] ) + $startupQuery = $this->options['target'] !== null ? [ 'target' => (string)$this->options['target'] ] : []; $chunks[] = $this->getLoad( @@ -379,12 +383,12 @@ class ResourceLoaderClientHtml { * @param ResourceLoaderContext $mainContext * @param array $modules One or more module names * @param string $only ResourceLoaderModule TYPE_ class constant - * @param array $extraQuery Array with extra query parameters for the request - * @param string $nonce See OutputPage::getCSPNonce() [Since 1.32] + * @param array $extraQuery [optional] Array with extra query parameters for the request + * @param string $nonce [optional] Content-Security-Policy nonce (from OutputPage::getCSPNonce) * @return string|WrappedStringList HTML */ public static function makeLoad( ResourceLoaderContext $mainContext, array $modules, $only, - array $extraQuery, $nonce + array $extraQuery = [], $nonce = null ) { $rl = $mainContext->getResourceLoader(); $chunks = []; diff --git a/tests/phpunit/includes/resourceloader/ResourceLoaderClientHtmlTest.php b/tests/phpunit/includes/resourceloader/ResourceLoaderClientHtmlTest.php index e763a19466..10176324ad 100644 --- a/tests/phpunit/includes/resourceloader/ResourceLoaderClientHtmlTest.php +++ b/tests/phpunit/includes/resourceloader/ResourceLoaderClientHtmlTest.php @@ -186,7 +186,9 @@ class ResourceLoaderClientHtmlTest extends PHPUnit\Framework\TestCase { $context = self::makeContext(); $context->getResourceLoader()->register( self::makeSampleModules() ); - $client = new ResourceLoaderClientHtml( $context ); + $client = new ResourceLoaderClientHtml( $context, [ + 'nonce' => false, + ] ); $client->setConfig( [ 'key' => 'value' ] ); $client->setModules( [ 'test', @@ -218,7 +220,7 @@ class ResourceLoaderClientHtmlTest extends PHPUnit\Framework\TestCase { // phpcs:enable $expected = self::expandVariables( $expected ); - $this->assertEquals( $expected, $client->getHeadHtml( false ) ); + $this->assertEquals( $expected, $client->getHeadHtml() ); } /** @@ -237,7 +239,7 @@ class ResourceLoaderClientHtmlTest extends PHPUnit\Framework\TestCase { . ''; // phpcs:enable - $this->assertEquals( $expected, $client->getHeadHtml( false ) ); + $this->assertEquals( $expected, $client->getHeadHtml() ); } /** @@ -256,7 +258,7 @@ class ResourceLoaderClientHtmlTest extends PHPUnit\Framework\TestCase { . ''; // phpcs:enable - $this->assertEquals( $expected, $client->getHeadHtml( false ) ); + $this->assertEquals( $expected, $client->getHeadHtml() ); } /** -- 2.20.1