resourceloader: Refactor CSP $nonce passing
authorTimo Tijhof <krinklemail@gmail.com>
Thu, 17 May 2018 18:25:49 +0000 (20:25 +0200)
committerKunal Mehta <legoktm@member.fsf.org>
Fri, 18 May 2018 05:28:23 +0000 (22:28 -0700)
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
includes/resourceloader/ResourceLoader.php
includes/resourceloader/ResourceLoaderClientHtml.php
tests/phpunit/includes/resourceloader/ResourceLoaderClientHtmlTest.php

index 52dfc11..dd1a4db 100644 (file)
@@ -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 ) );
index bee3d0c..2274793 100644 (file)
@@ -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 ) {
index d0a9c42..9eae7e8 100644 (file)
@@ -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 = [];
index e763a19..1017632 100644 (file)
@@ -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 {
                        . '<script async="" src="/w/load.php?debug=false&amp;lang=nl&amp;modules=startup&amp;only=scripts&amp;skin=fallback&amp;target=example"></script>';
                // phpcs:enable
 
-               $this->assertEquals( $expected, $client->getHeadHtml( false ) );
+               $this->assertEquals( $expected, $client->getHeadHtml() );
        }
 
        /**
@@ -256,7 +258,7 @@ class ResourceLoaderClientHtmlTest extends PHPUnit\Framework\TestCase {
                        . '<script async="" src="/w/load.php?debug=false&amp;lang=nl&amp;modules=startup&amp;only=scripts&amp;skin=fallback"></script>';
                // phpcs:enable
 
-               $this->assertEquals( $expected, $client->getHeadHtml( false ) );
+               $this->assertEquals( $expected, $client->getHeadHtml() );
        }
 
        /**