[MediaWiki-commits] [Gerrit] mediawiki/core[master]: Use Sanitizer::stripAllTags( $x ) instead of html_entity_dec...

2017-07-07 Thread jenkins-bot (Code Review) 
jenkins-bot has submitted this change and it was merged. ( 
https://gerrit.wikimedia.org/r/363987 )

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


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
---
M includes/api/ApiErrorFormatter.php
M includes/exception/LocalizedException.php
M includes/installer/CliInstaller.php
M includes/specials/SpecialRecentchanges.php
4 files changed, 7 insertions(+), 9 deletions(-)

Approvals:
  Krinkle: Looks good to me, approved
  jenkins-bot: Verified
  Jforrester: Looks good to me, but someone else must approve



diff --git a/includes/api/ApiErrorFormatter.php 
b/includes/api/ApiErrorFormatter.php
index 5484a78..7fb1352 100644
--- a/includes/api/ApiErrorFormatter.php
+++ b/includes/api/ApiErrorFormatter.php
@@ -254,7 +254,7 @@
$ret = preg_replace( '!!', '"', $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
--- a/includes/exception/LocalizedException.php
+++ b/includes/exception/LocalizedException.php
@@ -56,7 +56,7 @@
// customizations, and make a basic attempt to turn markup into 
text.
$msg = $this->getMessageObject()->inLanguage( 'en' 
)->useDatabase( false )->text();
$msg = preg_replace( '!!', '"', $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
--- a/includes/installer/CliInstaller.php
+++ b/includes/installer/CliInstaller.php
@@ -180,7 +180,7 @@
 
$text = preg_replace( '/(.*?)<\/a>/', '$2 
$1', $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
--- a/includes/specials/SpecialRecentchanges.php
+++ b/includes/specials/SpecialRecentchanges.php
@@ -202,10 +202,6 @@
 * @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 @@
 
$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,
];

-- 
To view, visit https://gerrit.wikimedia.org/r/363987
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I2ba2ecd388cb3d9cd2360ecaa236f3d444f0eabf
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Catrope 
Gerrit-Reviewer: Jforrester 
Gerrit-Reviewer: Krinkle 
Gerrit-Reviewer: MaxSem 
Gerrit-Reviewer:

[MediaWiki-commits] [Gerrit] mediawiki/core[master]: Use Sanitizer::stripAllTags( $x ) instead of html_entity_dec...

2017-07-07 Thread Catrope (Code Review) 
Catrope has uploaded a new change for review. ( 
https://gerrit.wikimedia.org/r/363987 )

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

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
---
M includes/api/ApiErrorFormatter.php
M includes/exception/LocalizedException.php
M includes/installer/CliInstaller.php
M includes/specials/SpecialRecentchanges.php
4 files changed, 5 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/87/363987/1

diff --git a/includes/api/ApiErrorFormatter.php 
b/includes/api/ApiErrorFormatter.php
index 5484a78..7fb1352 100644
--- a/includes/api/ApiErrorFormatter.php
+++ b/includes/api/ApiErrorFormatter.php
@@ -254,7 +254,7 @@
$ret = preg_replace( '!!', '"', $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
--- a/includes/exception/LocalizedException.php
+++ b/includes/exception/LocalizedException.php
@@ -56,7 +56,7 @@
// customizations, and make a basic attempt to turn markup into 
text.
$msg = $this->getMessageObject()->inLanguage( 'en' 
)->useDatabase( false )->text();
$msg = preg_replace( '!!', '"', $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
--- a/includes/installer/CliInstaller.php
+++ b/includes/installer/CliInstaller.php
@@ -180,7 +180,7 @@
 
$text = preg_replace( '/(.*?)<\/a>/', '$2 
$1', $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..11b2954 100644
--- a/includes/specials/SpecialRecentchanges.php
+++ b/includes/specials/SpecialRecentchanges.php
@@ -202,10 +202,6 @@
 * @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,8 @@
 
$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,
];

-- 
To view, visit https://gerrit.wikimedia.org/r/363987
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I2ba2ecd388cb3d9cd2360ecaa236f3d444f0eabf
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Catrope 

___
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits