resourceloader: Audit use of JSON encoding and use json_encode directly
authorTimo Tijhof <krinklemail@gmail.com>
Thu, 16 Aug 2018 19:48:59 +0000 (20:48 +0100)
committerAaron Schulz <aschulz@wikimedia.org>
Wed, 29 Aug 2018 00:46:11 +0000 (00:46 +0000)
* Remove use of MediaWiki's FormatJson class. Ref T32956.

* FormatJson::decode() was a direct alias of json_decode.

* FormatJson::encode() is a wrapper for json_encode that
  sets `JSON_UNESCAPED_SLASHES | JSON_HEX_TAG | JSON_HEX_AMP`
  by default, plus a boolean parameter to add JSON_PRETTY_PRINT.

  Instead, use json_encode() directly. By default json_encode() escapes
  slashes and non-ASCII unicode.

* Audit our uses of JSON encoding, document their needs, and set flags
  as needed.

* Remove $mapToJson abstraction from ResourceLoaderStartUpModule.

* Always pretty-print the list of module names in the error message,
  regardless of debug mode.

* Remove hacky indentation-adder from ResourceLoaderStartUpModule.
  This was relying on there not being line breaks in the json output
  outside debug mode, and adding a level of indentation to roughly
  match the code in the JSON is substituted. This didn't match and
  just generally doesn't seem worth it.

Change-Id: I3e09ddeb4d53c8980195c1855303c0ee25bd4c20

includes/resourceloader/ResourceLoader.php
includes/resourceloader/ResourceLoaderModule.php
includes/resourceloader/ResourceLoaderStartUpModule.php

index 98bae24..fc0ca1d 100644 (file)
@@ -135,7 +135,7 @@ class ResourceLoader implements LoggerAwareInterface {
                        $module = $this->getModule( $row->md_module );
                        if ( $module ) {
                                $module->setFileDependencies( $context, ResourceLoaderModule::expandRelativePaths(
                        $module = $this->getModule( $row->md_module );
                        if ( $module ) {
                                $module->setFileDependencies( $context, ResourceLoaderModule::expandRelativePaths(
-                                       FormatJson::decode( $row->md_deps, true )
+                                       json_decode( $row->md_deps, true )
                                ) );
                                $modulesWithDeps[] = $row->md_module;
                        }
                                ) );
                                $modulesWithDeps[] = $row->md_module;
                        }
@@ -1163,9 +1163,9 @@ MESSAGE;
                                $out = $this->ensureNewline( $out ) . $stateScript;
                        }
                } else {
                                $out = $this->ensureNewline( $out ) . $stateScript;
                        }
                } else {
-                       if ( count( $states ) ) {
-                               $this->errors[] = 'Problematic modules: ' .
-                                       FormatJson::encode( $states, self::inDebugMode() );
+                       if ( $states ) {
+                               // Keep default escaping of slashes (e.g. "</script>") for ResourceLoaderClientHtml.
+                               $this->errors[] = 'Problematic modules: ' . json_encode( $states, JSON_PRETTY_PRINT );
                        }
                }
 
                        }
                }
 
index 3bf309d..02d5802 100644 (file)
@@ -416,7 +416,7 @@ abstract class ResourceLoaderModule implements LoggerAwareInterface {
 
                        if ( !is_null( $deps ) ) {
                                $this->fileDeps[$vary] = self::expandRelativePaths(
 
                        if ( !is_null( $deps ) ) {
                                $this->fileDeps[$vary] = self::expandRelativePaths(
-                                       (array)FormatJson::decode( $deps, true )
+                                       (array)json_decode( $deps, true )
                                );
                        } else {
                                $this->fileDeps[$vary] = [];
                                );
                        } else {
                                $this->fileDeps[$vary] = [];
@@ -476,7 +476,9 @@ abstract class ResourceLoaderModule implements LoggerAwareInterface {
                                        return; // T124649; avoid write slams
                                }
 
                                        return; // T124649; avoid write slams
                                }
 
-                               $deps = FormatJson::encode( $localPaths );
+                               // No needless escaping as this isn't HTML output.
+                               // Only stored in the database and parsed in PHP.
+                               $deps = json_encode( $localPaths, JSON_UNESCAPED_SLASHES );
                                $dbw = wfGetDB( DB_MASTER );
                                $dbw->upsert( 'module_deps',
                                        [
                                $dbw = wfGetDB( DB_MASTER );
                                $dbw->upsert( 'module_deps',
                                        [
index 99ffcd2..18cc4d5 100644 (file)
@@ -392,16 +392,16 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
                        $mwLoaderCode .= file_get_contents( "$IP/resources/src/startup/profiler.js" );
                }
 
                        $mwLoaderCode .= file_get_contents( "$IP/resources/src/startup/profiler.js" );
                }
 
-               $mapToJson = function ( $value ) {
-                       $value = FormatJson::encode( $value, ResourceLoader::inDebugMode(), FormatJson::ALL_OK );
-                       // Fix indentation
-                       $value = str_replace( "\n", "\n\t", $value );
-                       return $value;
-               };
+               // 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 = [
 
                // Perform replacements for mediawiki.js
                $mwLoaderPairs = [
-                       '$VARS.baseModules' => $mapToJson( $this->getBaseModules() ),
+                       '$VARS.baseModules' => json_encode( $this->getBaseModules(), $jsonFlags ),
                ];
                $profilerStubs = [
                        '$CODE.profileExecuteStart();' => 'mw.loader.profiler.onExecuteStart( module );',
                ];
                $profilerStubs = [
                        '$CODE.profileExecuteStart();' => 'mw.loader.profiler.onExecuteStart( module );',
@@ -418,18 +418,20 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
                }
                $mwLoaderCode = strtr( $mwLoaderCode, $mwLoaderPairs );
 
                }
                $mwLoaderCode = strtr( $mwLoaderCode, $mwLoaderPairs );
 
-               // Perform replacements for startup.js
-               $pairs = array_map( $mapToJson, [
-                       '$VARS.wgLegacyJavaScriptGlobals' => $this->getConfig()->get( 'LegacyJavaScriptGlobals' ),
-                       '$VARS.configuration' => $this->getConfigSettings( $context ),
-               ] );
-               // Raw JavaScript code (not for JSON)
-               $pairs['$CODE.registrations();'] = str_replace(
-                       "\n",
-                       "\n\t",
-                       trim( $this->getModuleRegistrations( $context ) )
-               );
-               $pairs['$CODE.defineLoader();'] = $mwLoaderCode;
+               // Perform string replacements for startup.js
+               $pairs = [
+                       '$VARS.wgLegacyJavaScriptGlobals' => json_encode(
+                               $this->getConfig()->get( 'LegacyJavaScriptGlobals' ),
+                               $jsonFlags
+                       ),
+                       '$VARS.configuration' => json_encode(
+                               $this->getConfigSettings( $context ),
+                               $jsonFlags
+                       ),
+                       // Raw JavaScript code (not JSON)
+                       '$CODE.registrations();' => trim( $this->getModuleRegistrations( $context ) ),
+                       '$CODE.defineLoader();' => $mwLoaderCode,
+               ];
                $startupCode = strtr( $startupCode, $pairs );
 
                return $startupCode;
                $startupCode = strtr( $startupCode, $pairs );
 
                return $startupCode;