Use Sanitizer::stripAllTags( $x ) instead of html_entity_decode( strip_tags( $x ) )

We have a utility function for this, so let's use it.

What I don't understand though is why Sanitizer uses custom PHP implementations
for both tag stripping and entity decoding, instead of the built-in functions.
If there's a security reason for this or the built-ins are inadequate, that's
fine, but then that should be documented (and we should possibly ban usage
of the built-ins).

Change-Id: I2ba2ecd388cb3d9cd2360ecaa236f3d444f0eabf

diff --git a/includes/api/ApiErrorFormatter.php b/includes/api/ApiErrorFormatter.php
index 5484a78..7fb1352 100644 (file)
--- a/includes/api/ApiErrorFormatter.php
+++ b/includes/api/ApiErrorFormatter.php
@@ -254,7 +254,7 @@ class ApiErrorFormatter {
                $ret = preg_replace( '!</?(var|kbd|samp|code)>!', '"', $text );
 
                // Strip tags and decode.
-               $ret = html_entity_decode( strip_tags( $ret ), ENT_QUOTES | ENT_HTML5 );
+               $ret = Sanitizer::stripAllTags( $ret );
 
                return $ret;
        }

diff --git a/includes/exception/LocalizedException.php b/includes/exception/LocalizedException.php
index cbdb53e..d2cb5d1 100644 (file)
--- a/includes/exception/LocalizedException.php
+++ b/includes/exception/LocalizedException.php
@@ -56,7 +56,7 @@ class LocalizedException extends Exception implements ILocalizedException {
                // customizations, and make a basic attempt to turn markup into text.
                $msg = $this->getMessageObject()->inLanguage( 'en' )->useDatabase( false )->text();
                $msg = preg_replace( '!</?(var|kbd|samp|code)>!', '"', $msg );
-               $msg = html_entity_decode( strip_tags( $msg ), ENT_QUOTES | ENT_HTML5 );
+               $msg = Sanitizer::stripAllTags( $msg );
                parent::__construct( $msg, $code, $previous );
        }
 

diff --git a/includes/installer/CliInstaller.php b/includes/installer/CliInstaller.php
index 661c3ec..af55dbb 100644 (file)
--- a/includes/installer/CliInstaller.php
+++ b/includes/installer/CliInstaller.php
@@ -180,7 +180,7 @@ class CliInstaller extends Installer {
 
                $text = preg_replace( '/<a href="(.*?)".*?>(.*?)<\/a>/', '$2 &lt;$1&gt;', $text );
 
-               return html_entity_decode( strip_tags( $text ), ENT_QUOTES );
+               return Sanitizer::stripAllTags( $text );
        }
 
        /**

diff --git a/includes/specials/SpecialRecentchanges.php b/includes/specials/SpecialRecentchanges.php
index d856d4b..e7d5e66 100644 (file)
--- a/includes/specials/SpecialRecentchanges.php
+++ b/includes/specials/SpecialRecentchanges.php
@@ -202,10 +202,6 @@ class SpecialRecentChanges extends ChangesListSpecialPage {
         * @return Array Tag data
         */
        protected function buildChangeTagList() {
-               function stripAllHtml( $input ) {
-                       return trim( html_entity_decode( strip_tags( $input ) ) );
-               }
-
                $explicitlyDefinedTags = array_fill_keys( ChangeTags::listExplicitlyDefinedTags(), 0 );
                $softwareActivatedTags = array_fill_keys( ChangeTags::listSoftwareActivatedTags(), 0 );
                $tagStats = ChangeTags::tagUsageStatistics();
@@ -228,8 +224,10 @@ class SpecialRecentChanges extends ChangesListSpecialPage {
 
                                $result[] = [
                                        'name' => $tagName,
-                                       'label' => stripAllHtml( ChangeTags::tagDescription( $tagName, $this->getContext() ) ),
-                                       'description' => $desc ? stripAllHtml( $desc->parse() ) : '',
+                                       'label' => Sanitizer::stripAllTags(
+                                               ChangeTags::tagDescription( $tagName, $this->getContext() )
+                                       ),
+                                       'description' => $desc ? Sanitizer::stripAllTags( $desc->parse() ) : '',
                                        'cssClass' => Sanitizer::escapeClass( 'mw-tag-' . $tagName ),
                                        'hits' => $hits,
                                ];