resourceloader: Factor out encodeJsonForScript
authorTimo Tijhof <krinklemail@gmail.com>
Sat, 15 Sep 2018 20:51:43 +0000 (21:51 +0100)
committerTimo Tijhof <krinklemail@gmail.com>
Mon, 17 Sep 2018 23:53:42 +0000 (00:53 +0100)
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
includes/resourceloader/ResourceLoaderStartUpModule.php

index dde72b2..95579a9 100644 (file)
@@ -1162,8 +1162,8 @@ MESSAGE;
                        }
                } else {
                        if ( $states ) {
-                               // Keep default escaping of slashes (e.g. "</script>") 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 "</script>", 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 ) . '}'
                        . ']);';
        }
index 8140c2c..e4a753f 100644 (file)
@@ -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 ) ),