From: Timo Tijhof <krinklemail@gmail.com>
Date: Thu, 16 Aug 2018 19:48:59 +0000 (+0100)
Subject: resourceloader: Audit use of JSON encoding and use json_encode directly
X-Git-Tag: 1.34.0-rc.0~4266^2
X-Git-Url: https://git.heureux-cyclage.org/?p=lhc%2Fweb%2Fwiklou.git;a=commitdiff_plain;h=dc3fc6cf81ecc2c84e6e45f3f4401c4b8976861c

resourceloader: Audit use of JSON encoding and use json_encode directly

* 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
---

diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php
index 98bae24934..fc0ca1d5a2 100644
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -135,7 +135,7 @@ class ResourceLoader implements LoggerAwareInterface {
 			$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;
 			}
@@ -1163,9 +1163,9 @@ MESSAGE;
 				$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 );
 			}
 		}
 
diff --git a/includes/resourceloader/ResourceLoaderModule.php b/includes/resourceloader/ResourceLoaderModule.php
index 3bf309d9ea..02d5802e97 100644
--- a/includes/resourceloader/ResourceLoaderModule.php
+++ b/includes/resourceloader/ResourceLoaderModule.php
@@ -416,7 +416,7 @@ abstract class ResourceLoaderModule implements LoggerAwareInterface {
 
 			if ( !is_null( $deps ) ) {
 				$this->fileDeps[$vary] = self::expandRelativePaths(
-					(array)FormatJson::decode( $deps, true )
+					(array)json_decode( $deps, true )
 				);
 			} else {
 				$this->fileDeps[$vary] = [];
@@ -476,7 +476,9 @@ abstract class ResourceLoaderModule implements LoggerAwareInterface {
 					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',
 					[
diff --git a/includes/resourceloader/ResourceLoaderStartUpModule.php b/includes/resourceloader/ResourceLoaderStartUpModule.php
index 99ffcd2a94..18cc4d5aad 100644
--- a/includes/resourceloader/ResourceLoaderStartUpModule.php
+++ b/includes/resourceloader/ResourceLoaderStartUpModule.php
@@ -392,16 +392,16 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
 			$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 = [
-			'$VARS.baseModules' => $mapToJson( $this->getBaseModules() ),
+			'$VARS.baseModules' => json_encode( $this->getBaseModules(), $jsonFlags ),
 		];
 		$profilerStubs = [
 			'$CODE.profileExecuteStart();' => 'mw.loader.profiler.onExecuteStart( module );',
@@ -418,18 +418,20 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
 		}
 		$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;