Chad has uploaded a new change for review.
https://gerrit.wikimedia.org/r/92472
Change subject: Revert "Add a way to redact certain function parameters from
exception stack traces"
......................................................................
Revert "Add a way to redact certain function parameters from exception stack
traces"
This reverts commit 459cb0b7bb385d559c2a8cbf4e209a6795993794. Blacklists for
something
like this are a bad idea, as we'll basically play whack-a-mole forever finding
parameters that need redacting. Opposition to this approach was raised before,
but
nobody felt the need to listen before we merged.
Should be replaced by an approach that masks all parameters from stacktrace
output,
possibly still giving types (think Java).
Needs backporting to REL1_22 so we don't release with such a half-thought-out
setting
included.
Bug: 30714
Change-Id: Ic833a9dec21366143bb57e3d652c4c1f09a88c50
---
M RELEASE-NOTES-1.22
M includes/DefaultSettings.php
M includes/Exception.php
3 files changed, 7 insertions(+), 104 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/72/92472/1
diff --git a/RELEASE-NOTES-1.22 b/RELEASE-NOTES-1.22
index 92699be..2de70a3 100644
--- a/RELEASE-NOTES-1.22
+++ b/RELEASE-NOTES-1.22
@@ -227,8 +227,6 @@
* Add deferrable update support for callback/closure.
* Add TitleMove hook before page renames.
* Revision deletion backend code is moved out of SpecialRevisiondelete
-* Add a variable (wgRedactedFunctionArguments) to redact the values sent as
certain function
- parameters from exception stack traces.
* Added {{REVISIONSIZE}} variable to get the current size of a revision.
* Add support for the LESS stylesheet language to ResourceLoader. LESS is a
stylesheet language that compiles into CSS. ResourceLoader file modules may
diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index d3c7b5f..ebae110 100644
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -4941,37 +4941,6 @@
$wgShowExceptionDetails = false;
/**
- * Array of functions which need parameters redacted from stack traces shown to
- * clients and logged. Keys are in the format '[class::]function', and the
- * values should be either an integer or an array of integers. These are the
- * indexes of the parameters which need to be kept secret.
- * @since 1.22
- */
-$wgRedactedFunctionArguments = array(
- 'AuthPlugin::setPassword' => 1,
- 'AuthPlugin::authenticate' => 1,
- 'AuthPlugin::addUser' => 1,
-
- 'DatabaseBase::__construct' => 2,
- 'DatabaseBase::open' => 2,
-
- 'SpecialChangeEmail::attemptChange' => 1,
- 'SpecialChangePassword::attemptReset' => 0,
-
- 'User::setPassword' => 0,
- 'User::setInternalPassword' => 0,
- 'User::checkPassword' => 0,
- 'User::setNewpassword' => 0,
- 'User::comparePasswords' => array( 0, 1 ),
- 'User::checkTemporaryPassword' => 0,
- 'User::setToken' => 0,
- 'User::crypt' => 0,
- 'User::oldCrypt' => 0,
- 'User::getPasswordValidity' => 0,
- 'User::isValidPassword' => 0,
-);
-
-/**
* If true, show a backtrace for database errors
*/
$wgShowDBErrorBacktrace = false;
diff --git a/includes/Exception.php b/includes/Exception.php
index a91f865..122ed51 100644
--- a/includes/Exception.php
+++ b/includes/Exception.php
@@ -126,7 +126,7 @@
if ( $wgShowExceptionDetails ) {
return '<p>' . nl2br( htmlspecialchars(
$this->getMessage() ) ) .
- '</p><p>Backtrace:</p><p>' . nl2br(
htmlspecialchars( MWExceptionHandler::formatRedactedTrace( $this ) ) ) .
+ '</p><p>Backtrace:</p><p>' . nl2br(
htmlspecialchars( $this->getTraceAsString() ) ) .
"</p>\n";
} else {
return "<div class=\"errorbox\">" .
@@ -151,7 +151,7 @@
if ( $wgShowExceptionDetails ) {
return $this->getMessage() .
- "\nBacktrace:\n" .
MWExceptionHandler::formatRedactedTrace( $this ) . "\n";
+ "\nBacktrace:\n" . $this->getTraceAsString() .
"\n";
} else {
return "Set \$wgShowExceptionDetails = true; " .
"in LocalSettings.php to show detailed
debugging information.\n";
@@ -601,7 +601,7 @@
$message = "MediaWiki internal error.\n\n";
if ( $wgShowExceptionDetails ) {
- $message .= 'Original exception: ' .
self::formatRedactedTrace( $e ) . "\n\n" .
+ $message .= 'Original exception: ' .
$e->__toString() . "\n\n" .
'Exception caught inside
exception handler: ' . $e2->__toString();
} else {
$message .= "Exception caught inside
exception handler.\n\n" .
@@ -618,10 +618,11 @@
}
}
} else {
- $message = "Unexpected non-MediaWiki exception
encountered, of type \"" . get_class( $e ) . "\"";
+ $message = "Unexpected non-MediaWiki exception
encountered, of type \"" . get_class( $e ) . "\"\n" .
+ $e->__toString() . "\n";
if ( $wgShowExceptionDetails ) {
- $message .= "\nexception '" . get_class( $e ) .
"' in " . $e->getFile() . ":" . $e->getLine() . "\nStack trace:\n" .
self::formatRedactedTrace( $e ) . "\n";
+ $message .= "\n" . $e->getTraceAsString() .
"\n";
}
if ( $cmdLine ) {
@@ -676,71 +677,6 @@
// Exit value should be nonzero for the benefit of shell jobs
exit( 1 );
}
-
- /**
- * Get the stack trace from the exception as a string, redacting
certain function arguments in the process
- * @param Exception $e The exception
- * @return string The stack trace as a string
- */
- public static function formatRedactedTrace( Exception $e ) {
- global $wgRedactedFunctionArguments;
- $finalExceptionText = '';
-
- // Unique value to indicate redacted parameters
- $redacted = new stdClass();
-
- foreach ( $e->getTrace() as $i => $call ) {
- $checkFor = array();
- if ( isset( $call['class'] ) ) {
- $checkFor[] = $call['class'] . '::' .
$call['function'];
- foreach ( class_parents( $call['class'] ) as
$parent ) {
- $checkFor[] = $parent . '::' .
$call['function'];
- }
- } else {
- $checkFor[] = $call['function'];
- }
-
- foreach ( $checkFor as $check ) {
- if ( isset(
$wgRedactedFunctionArguments[$check] ) ) {
- foreach (
(array)$wgRedactedFunctionArguments[$check] as $argNo ) {
- $call['args'][$argNo] =
$redacted;
- }
- }
- }
-
- if ( isset( $call['file'] ) && isset( $call['line'] ) )
{
- $finalExceptionText .= "#{$i}
{$call['file']}({$call['line']}): ";
- } else {
- // 'file' and 'line' are unset for calls via
call_user_func (bug 55634)
- // This matches behaviour of
Exception::getTraceAsString to instead
- // display "[internal function]".
- $finalExceptionText .= "#{$i} [internal
function]: ";
- }
-
- if ( isset( $call['class'] ) ) {
- $finalExceptionText .= $call['class'] .
$call['type'] . $call['function'];
- } else {
- $finalExceptionText .= $call['function'];
- }
- $args = array();
- if ( isset( $call['args'] ) ) {
- foreach ( $call['args'] as $arg ) {
- if ( $arg === $redacted ) {
- $args[] = 'REDACTED';
- } elseif ( is_object( $arg ) ) {
- $args[] = 'Object(' .
get_class( $arg ) . ')';
- } elseif( is_array( $arg ) ) {
- $args[] = 'Array';
- } else {
- $args[] = var_export( $arg,
true );
- }
- }
- }
- $finalExceptionText .= '(' . implode( ', ', $args ) .
")\n";
- }
- return $finalExceptionText . '#' . ( $i + 1 ) . ' {main}';
- }
-
/**
* Get the ID for this error.
@@ -802,7 +738,7 @@
$log = self::getLogMessage( $e );
if ( $log ) {
if ( $wgLogExceptionBacktrace ) {
- wfDebugLog( 'exception', $log . "\n" .
self::formatRedactedTrace( $e ) . "\n" );
+ wfDebugLog( 'exception', $log . "\n" .
$this->getTraceAsString() . "\n" );
} else {
wfDebugLog( 'exception', $log );
}
--
To view, visit https://gerrit.wikimedia.org/r/92472
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic833a9dec21366143bb57e3d652c4c1f09a88c50
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Chad <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
