From 5d528f39d84e490e30fe9c81b95c6eef981eb3c7 Mon Sep 17 00:00:00 2001
From: daniel <daniel.kinzler@wikimedia.de>
Date: Wed, 7 Jun 2017 12:49:24 +0200
Subject: [PATCH] Parser: Ensure function and tag hook callbacks are callable
 upon registration

This is to safeguard against issues like T167238, where the callback was invalid,
but was only noticed when the magic word was used in an article.

In general, type checks should be applied when things get
registered for later use. Fail early, stay sane.

Change-Id: Ifb7005ee214829c98cec534261c0db7d13f50f35
---
 includes/parser/Parser.php | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/includes/parser/Parser.php b/includes/parser/Parser.php
index 34f62324e0..dc7871c39f 100644
--- a/includes/parser/Parser.php
+++ b/includes/parser/Parser.php
@@ -3377,11 +3377,6 @@ class Parser {
 
 		list( $callback, $flags ) = $this->mFunctionHooks[$function];
 
-		# Workaround for PHP bug 35229 and similar
-		if ( !is_callable( $callback ) ) {
-			throw new MWException( "Tag hook for $function is not callable\n" );
-		}
-
 		// Avoid PHP 7.1 warning from passing $this by reference
 		$parser = $this;
 
@@ -3882,17 +3877,10 @@ class Parser {
 			}
 
 			if ( isset( $this->mTagHooks[$name] ) ) {
-				# Workaround for PHP bug 35229 and similar
-				if ( !is_callable( $this->mTagHooks[$name] ) ) {
-					throw new MWException( "Tag hook for $name is not callable\n" );
-				}
 				$output = call_user_func_array( $this->mTagHooks[$name],
 					[ $content, $attributes, $this, $frame ] );
 			} elseif ( isset( $this->mFunctionTagHooks[$name] ) ) {
 				list( $callback, ) = $this->mFunctionTagHooks[$name];
-				if ( !is_callable( $callback ) ) {
-					throw new MWException( "Tag hook for $name is not callable\n" );
-				}
 
 				// Avoid PHP 7.1 warning from passing $this by reference
 				$parser = $this;
@@ -4761,7 +4749,7 @@ class Parser {
 	 * @throws MWException
 	 * @return callable|null The old value of the mTagHooks array associated with the hook
 	 */
-	public function setHook( $tag, $callback ) {
+	public function setHook( $tag, callable $callback ) {
 		$tag = strtolower( $tag );
 		if ( preg_match( '/[<>\r\n]/', $tag, $m ) ) {
 			throw new MWException( "Invalid character {$m[0]} in setHook('$tag', ...) call" );
@@ -4792,7 +4780,7 @@ class Parser {
 	 * @throws MWException
 	 * @return callable|null The old value of the mTagHooks array associated with the hook
 	 */
-	public function setTransparentTagHook( $tag, $callback ) {
+	public function setTransparentTagHook( $tag, callable $callback ) {
 		$tag = strtolower( $tag );
 		if ( preg_match( '/[<>\r\n]/', $tag, $m ) ) {
 			throw new MWException( "Invalid character {$m[0]} in setTransparentHook('$tag', ...) call" );
@@ -4855,7 +4843,7 @@ class Parser {
 	 * @throws MWException
 	 * @return string|callable The old callback function for this name, if any
 	 */
-	public function setFunctionHook( $id, $callback, $flags = 0 ) {
+	public function setFunctionHook( $id, callable $callback, $flags = 0 ) {
 		global $wgContLang;
 
 		$oldVal = isset( $this->mFunctionHooks[$id] ) ? $this->mFunctionHooks[$id][0] : null;
@@ -4907,7 +4895,7 @@ class Parser {
 	 * @throws MWException
 	 * @return null
 	 */
-	public function setFunctionTagHook( $tag, $callback, $flags ) {
+	public function setFunctionTagHook( $tag, callable $callback, $flags ) {
 		$tag = strtolower( $tag );
 		if ( preg_match( '/[<>\r\n]/', $tag, $m ) ) {
 			throw new MWException( "Invalid character {$m[0]} in setFunctionTagHook('$tag', ...) call" );
-- 
2.20.1