From b7f4b55f2b63f56ecee6992d8226148682237fc9 Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Sat, 15 Sep 2018 21:51:43 +0100 Subject: [PATCH] resourceloader: Factor out encodeJsonForScript Follows-up dc3fc6cf81, which documented the reasoning for the specific json flags in StartupModule. In wanting to re-use them in a different module it became apparant that perhaps it was a bit too conservative in only allowing the flags to be used in a script HTTP response. Lax the contract a little bit (that is, do more escaping) to also allow safe re-use in HTML context. I considered making these separate methods (e.g. forScriptResponse, and forInlineScript) but decided not to because the vast majority of callers would have to be forInlineScript eventhough the code in question had no responsiblity or knowledge of it becoming an inline script, because ResourceLoader allows most modules to become embedded if they support a private or preview mode. Those modes are implemented by calling makeModuleResponse, and wrapping it an inline script. It seems appealing and simplifying to the API to require that script output is always safe for embedding rather than complicating the API for winning back a literal handful of bytes in the edge case that a user-generated string contains a '<' and was not embedded. I estimate that with gzip, it will literally save only a single byte, even if used multiple times. Let's focus optimisation efforts elsewhere :) Change-Id: I7742dabba6750deecf6fbf51cf9a77ee8cbfc727 --- includes/resourceloader/ResourceLoader.php | 35 +++++++++++++++++-- .../ResourceLoaderStartUpModule.php | 19 +++------- 2 files changed, 37 insertions(+), 17 deletions(-) diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php index dde72b2f36..95579a99fb 100644 --- a/includes/resourceloader/ResourceLoader.php +++ b/includes/resourceloader/ResourceLoader.php @@ -1162,8 +1162,8 @@ MESSAGE; } } else { if ( $states ) { - // Keep default escaping of slashes (e.g. "") for ResourceLoaderClientHtml. - $this->errors[] = 'Problematic modules: ' . json_encode( $states, JSON_PRETTY_PRINT ); + $this->errors[] = 'Problematic modules: ' + . self::encodeJsonForScript( $states ); } } @@ -1292,6 +1292,35 @@ MESSAGE; return $out; } + /** + * Wrapper around json_encode that avoids needless escapes, + * and pretty-prints in debug mode. + * + * @internal + * @since 1.32 + * @param bool|string|array $data + * @return string JSON + */ + public static function encodeJsonForScript( $data ) { + // Keep output as small as possible by disabling needless escape modes + // that PHP uses by default. + // However, while most module scripts are only served on HTTP responses + // for JavaScript, some modules can also be embedded in the HTML as inline + // scripts. This, and the fact that we sometimes need to export strings + // containing user-generated content and labels that may genuinely contain + // a sequences like "", we need to encode either '/' or '<'. + // By default PHP escapes '/'. Let's escape '<' instead which is less common + // and allows URLs to mostly remain readable. + $jsonFlags = JSON_UNESCAPED_SLASHES | + JSON_UNESCAPED_UNICODE | + JSON_HEX_TAG | + JSON_HEX_AMP; + if ( self::inDebugMode() ) { + $jsonFlags |= JSON_PRETTY_PRINT; + } + return json_encode( $data, $jsonFlags ); + } + /** * Returns a JS call to mw.loader.state, which sets the state of one * ore more modules to a given value. Has two calling conventions: @@ -1466,7 +1495,7 @@ MESSAGE; public static function makeInlineCodeWithModule( $modules, $script ) { // Adds an array to lazy-created RLQ return '(window.RLQ=window.RLQ||[]).push([' - . json_encode( $modules ) . ',' + . self::encodeJsonForScript( $modules ) . ',' . 'function(){' . trim( $script ) . '}' . ']);'; } diff --git a/includes/resourceloader/ResourceLoaderStartUpModule.php b/includes/resourceloader/ResourceLoaderStartUpModule.php index 8140c2c8c0..e4a753fc16 100644 --- a/includes/resourceloader/ResourceLoaderStartUpModule.php +++ b/includes/resourceloader/ResourceLoaderStartUpModule.php @@ -404,16 +404,9 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule { $mwLoaderCode .= file_get_contents( "$IP/resources/src/startup/profiler.js" ); } - // Keep output as small as possible by disabling needless escapes that PHP uses by default. - // This is not HTML output, only used in a JS response. - $jsonFlags = JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE; - if ( ResourceLoader::inDebugMode() ) { - $jsonFlags |= JSON_PRETTY_PRINT; - } - // Perform replacements for mediawiki.js $mwLoaderPairs = [ - '$VARS.baseModules' => json_encode( $this->getBaseModules(), $jsonFlags ), + '$VARS.baseModules' => ResourceLoader::encodeJsonForScript( $this->getBaseModules() ), ]; $profilerStubs = [ '$CODE.profileExecuteStart();' => 'mw.loader.profiler.onExecuteStart( module );', @@ -432,13 +425,11 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule { // Perform string replacements for startup.js $pairs = [ - '$VARS.wgLegacyJavaScriptGlobals' => json_encode( - $this->getConfig()->get( 'LegacyJavaScriptGlobals' ), - $jsonFlags + '$VARS.wgLegacyJavaScriptGlobals' => ResourceLoader::encodeJsonForScript( + $this->getConfig()->get( 'LegacyJavaScriptGlobals' ) ), - '$VARS.configuration' => json_encode( - $this->getConfigSettings( $context ), - $jsonFlags + '$VARS.configuration' => ResourceLoader::encodeJsonForScript( + $this->getConfigSettings( $context ) ), // Raw JavaScript code (not JSON) '$CODE.registrations();' => trim( $this->getModuleRegistrations( $context ) ), -- 2.20.1