Deprecate QuickTemplate::msgHtml & fix phan-taint-warning in includes/skins

QuickTemplate::msgHtml() (And the weird override that does the
same thing a different way - BaseTemplate::msgHtml()) are inherently
unsafe as they echo out a raw html message. This is strongly
discouraged in modern code. According to codeSearch tool, nothing
uses these methods, and there is a "@private" annotation on the
QuickTemplate::msgHtml() docblock. Thus hard deprecating it.

Change-Id: I4e9e157e922a36787adef4d0bf7608605c27f0c4

diff --git a/RELEASE-NOTES-1.32 b/RELEASE-NOTES-1.32
index fb2829b..7a3cb15 100644 (file)
--- a/RELEASE-NOTES-1.32
+++ b/RELEASE-NOTES-1.32
@@ -456,6 +456,9 @@ because of Phabricator reports.
   $wgTidyConfig instead.
 * All Tidy configurations other than Remex have been hard deprecated;
   future parsers will not emit compatible output for these configurations.
+* QuickTemplate::msgHtml() and BaseTemplate::msgHtml() have been deprecated
+  as they promote bad practises. I18n messages should always be properly
+  escaped.
 
 === Other changes in 1.32 ===
 * (T198811) The following tables have had their UNIQUE indexes turned into

diff --git a/includes/skins/BaseTemplate.php b/includes/skins/BaseTemplate.php
index 5c4d812..64145ad 100644 (file)
--- a/includes/skins/BaseTemplate.php
+++ b/includes/skins/BaseTemplate.php
@@ -43,7 +43,15 @@ abstract class BaseTemplate extends QuickTemplate {
                echo $this->getMsg( $str )->escaped();
        }
 
+       /**
+        * @param string $str
+        * @warning You should never use this method. I18n messages should be escaped
+        * @deprecated 1.32 Use ->msg() or ->msgWiki() instead.
+        * @suppress SecurityCheck-XSS
+        * @return-taint exec_html
+        */
        function msgHtml( $str ) {
+               wfDeprecated( __METHOD__, '1.32' );
                echo $this->getMsg( $str )->text();
        }
 

diff --git a/includes/skins/QuickTemplate.php b/includes/skins/QuickTemplate.php
index 969ac51..e4a2476 100644 (file)
--- a/includes/skins/QuickTemplate.php
+++ b/includes/skins/QuickTemplate.php
@@ -75,6 +75,7 @@ abstract class QuickTemplate {
         * @param string $name Key for the data
         * @param mixed|null $default Optional default (or null)
         * @return mixed The value of the data requested or the deafult
+        * @return-taint onlysafefor_htmlnoent
         */
        public function get( $name, $default = null ) {
                return $this->data[$name] ?? $default;
@@ -101,6 +102,7 @@ abstract class QuickTemplate {
        /**
         * @private
         * @param string $str
+        * @suppress SecurityCheck-DoubleEscaped $this->data can be either
         */
        function text( $str ) {
                echo htmlspecialchars( $this->data[$str] );
@@ -109,6 +111,7 @@ abstract class QuickTemplate {
        /**
         * @private
         * @param string $str
+        * @suppress SecurityCheck-XSS phan-taint-check cannot tell if $str is pre-escaped
         */
        function html( $str ) {
                echo $this->data[$str];
@@ -125,8 +128,13 @@ abstract class QuickTemplate {
        /**
         * @private
         * @param string $msgKey
+        * @warning You should never use this method. I18n messages should be escaped
+        * @deprecated 1.32 Use ->msg() or ->msgWiki() instead.
+        * @suppress SecurityCheck-XSS
+        * @return-taint exec_html
         */
        function msgHtml( $msgKey ) {
+               wfDeprecated( __METHOD__, '1.32' );
                echo wfMessage( $msgKey )->text();
        }
 

diff --git a/includes/skins/Skin.php b/includes/skins/Skin.php
index 2f5e0c8..e426f7f 100644 (file)
--- a/includes/skins/Skin.php
+++ b/includes/skins/Skin.php
@@ -1608,6 +1608,7 @@ abstract class Skin extends ContextSource {
        /**
         * Create a section edit link.
         *
+        * @suppress SecurityCheck-XSS $links has keys of different taint types
         * @param Title $nt The title being linked to (may not be the same as
         *   the current page, if the section is included from a template)
         * @param string $section The designation of the section being pointed to,