From a3c9621ad004aa42bf390770be3c691b23a3fbe7 Mon Sep 17 00:00:00 2001
From: "C. Scott Ananian" <cscott@cscott.net>
Date: Thu, 26 Mar 2020 17:57:22 -0400
Subject: [PATCH] Deprecate unusual uses of the ParserGetVariableValueSwitch
 hook

Ensure that the value returned is always also added to the cache, since
we (eventually) want to remove the hook's access to
$parser->mVarCache.  Also don't allow monkeying with the value passed
as $magicWordId, even though it's a reference.

Code search:
https://codesearch.wmflabs.org/deployed/?q=ParserGetVariableValueSwitch&i=nope&files=&repos=

Bug: T236813
Depends-On: Ia12faefada7e4cf04f1a6b12b3ed1703bf28e437
Depends-On: Ia9e2e00c6b7d0e62cbe80c3b124165b691b1ff3d
Depends-On: I1e48fa47b8723958d543a69aaf9b62b872200dc5
Depends-On: If9a0885a8664c22473ade712364c2dd0a5c04e0b
Change-Id: I3d6b281f8e4e0bf68eefbf9767047527b4573b79
---
 RELEASE-NOTES-1.35         |  4 ++++
 docs/hooks.txt             |  8 ++++----
 includes/parser/Parser.php | 14 +++++++++++++-
 3 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/RELEASE-NOTES-1.35 b/RELEASE-NOTES-1.35
index 6d29dcbb0b6..7192ded2cb8 100644
--- a/RELEASE-NOTES-1.35
+++ b/RELEASE-NOTES-1.35
@@ -700,6 +700,10 @@ because of Phabricator reports.
 * The Parser::enableOOUI() method has been deprecated.  Use
   $parser->getOutput()->enableOOUI() instead.
 * The ParserGetVariableValueVarCache hook has been deprecated.
+* When using the ParserGetVariableValueSwitch hook, the following unusual
+  uses have been deprecated: modifying the passed $magicWordId or failing to
+  cache the returned value in $variableCache.
+* Parser::$mVarCache has been deprecated for direct access.
 * A new UserNameUtils service was introduced. The following User methods
   were deprecated in favor of using the new service:
   - isIP
diff --git a/docs/hooks.txt b/docs/hooks.txt
index a68bc627bde..6629fbf19a2 100644
--- a/docs/hooks.txt
+++ b/docs/hooks.txt
@@ -2588,12 +2588,12 @@ $rev: Revision object of the template
 'ParserFirstCallInit': Called when the parser initialises for the first time.
 [&]$parser: Parser object being cleared
 
-'ParserGetVariableValueSwitch': Called when the parser need the value of a
+'ParserGetVariableValueSwitch': Called when the parser needs the value of a
 custom magic word
 [&]$parser: Parser object
-&$variableCache: array to store the value in case of multiples calls of the
-  same magic word
-&$magicWordId: index (string) of the magic word
+&$variableCache: array to cache the value; when you return
+  $variableCache[$magicWordId] should be the same as $ret
+[&]$magicWordId: index (string) of the magic word (hook should not mutate it!)
 &$ret: value of the magic word (the hook should set it)
 [&]$frame: PPFrame object to use for expanding any template variables
 
diff --git a/includes/parser/Parser.php b/includes/parser/Parser.php
index 94b8af00b36..2998aa602b6 100644
--- a/includes/parser/Parser.php
+++ b/includes/parser/Parser.php
@@ -150,6 +150,7 @@ class Parser {
 	public $mFunctionTagHooks = [];
 	public $mStripList = [];
 	public $mDefaultStripList = [];
+	/** @deprecated since 1.35 */
 	public $mVarCache = [];
 	public $mImageParams = [];
 	public $mImageParamsMagicArray = [];
@@ -3020,11 +3021,22 @@ class Parser {
 				break;
 			default:
 				$ret = null;
+				$originalIndex = $index;
 				Hooks::run(
 					'ParserGetVariableValueSwitch',
 					[ &$parser, &$this->mVarCache, &$index, &$ret, &$frame ]
 				);
-
+				if ( $index !== $originalIndex ) {
+					wfDeprecated(
+						'ParserGetVariableValueSwitch modifying $index', '1.35'
+					);
+				}
+				if ( !isset( $this->mVarCache[$originalIndex] ) ||
+					 $this->mVarCache[$originalIndex] !== $ret ) {
+					wfDeprecated(
+						'ParserGetVariableValueSwitch bypassing cache', '1.35'
+					);
+				}
 				return $ret;
 		}