Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge "REST: Hide exception message when wgShowExceptionDetails=false"

Browse source
This commit is contained in:
jenkins-bot 2022-06-09 21:51:45 +00:00 committed by Gerrit Code Review
commit 41fe47cde9
8 changed files with 38 additions and 60 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
docs/config-schema.yaml
View file

@ -5688,10 +5688,10 @@ config-schema:
ShowExceptionDetails:
default: false
description: |-
If set to true, uncaught exceptions will print the exception message and a
complete stack trace to output. This should only be used for debugging, as it
may reveal private information in function parameters due to PHP's backtrace
formatting. If set to false, only the exception's class will be shown.
Show exception message and stack trace when printing details about uncaught exceptions
in web response output.
This may reveal private information in error messages or function parameters.
If set to false, only the exception type or class name will be exposed.
LogExceptionBacktrace:
default: true
description: 'If true, send the exception backtrace to the error log'

9
includes/MainConfigSchema.php
View file

@ -9016,10 +9016,11 @@ class MainConfigSchema {
];
/**
* If set to true, uncaught exceptions will print the exception message and a
* complete stack trace to output. This should only be used for debugging, as it
* may reveal private information in function parameters due to PHP's backtrace
* formatting. If set to false, only the exception's class will be shown.
* Show exception message and stack trace when printing details about uncaught exceptions
* in web response output.
*
* This may reveal private information in error messages or function parameters.
* If set to false, only the exception type or class name will be exposed.
*/
public const ShowExceptionDetails = [
'default' => false,

2
includes/Rest/EntryPoint.php
View file

@ -112,7 +112,7 @@ class EntryPoint {
$conf = $services->getMainConfig();
$responseFactory = new ResponseFactory( self::getTextFormatters( $services ) );
$responseFactory->setSendExceptionBacktrace( MWExceptionRenderer::shouldShowExceptionDetails() );
$responseFactory->setShowExceptionDetails( MWExceptionRenderer::shouldShowExceptionDetails() );
$cors = new CorsUtils(
new ServiceOptions(

21
includes/Rest/ResponseFactory.php
View file

@ -23,7 +23,7 @@ class ResponseFactory {
private $textFormatters;
/** @var bool Whether to send exception backtraces to the client */
private $sendExceptionBacktrace = false;
private $showExceptionDetails = false;
/**
* @param ITextFormatter[] $textFormatters
@ -33,12 +33,14 @@ class ResponseFactory {
}
/**
* Controls whether error responses should include a backtrace
* Control whether web responses may include a exception messager and backtrace
*
* @see $wgShowExceptionDetails
* @since 1.39
* @param bool $sendExceptionBacktrace
* @param bool $showExceptionDetails
*/
public function setSendExceptionBacktrace( bool $sendExceptionBacktrace ): void {
$this->sendExceptionBacktrace = $sendExceptionBacktrace;
public function setShowExceptionDetails( bool $showExceptionDetails ): void {
$this->showExceptionDetails = $showExceptionDetails;
}
/**
@ -241,18 +243,21 @@ class ResponseFactory {
)
);
}
} else {
} elseif ( $this->showExceptionDetails ) {
$response = $this->createHttpError( 500, [
'message' => 'Error: exception of type ' . get_class( $exception ) . ': '
. $exception->getMessage(),
'exception' => MWExceptionHandler::getStructuredExceptionData(
$exception,
MWExceptionHandler::CAUGHT_BY_OTHER,
$this->sendExceptionBacktrace
MWExceptionHandler::CAUGHT_BY_OTHER
)
] );
// XXX: should we try to do something useful with ILocalizedException?
// XXX: should we try to do something useful with common MediaWiki errors like ReadOnlyError?
} else {
$response = $this->createHttpError( 500, [
'message' => 'Error: exception of type ' . get_class( $exception ),
] );
}
return $response;
}

21
includes/exception/MWExceptionHandler.php
View file

@ -605,22 +605,13 @@ TXT;
*
* @param Throwable $e
* @param string $catcher CAUGHT_BY_* class constant indicating what caught the error
* @param bool|null $includeBacktrace Whether to include a backtrace. If null,
* the value of $includeBacktrace that was provided to installHandler() will be
* used.
*
* @return array
* @since 1.26
*/
public static function getStructuredExceptionData(
Throwable $e,
$catcher = self::CAUGHT_BY_OTHER,
?bool $includeBacktrace = null
$catcher = self::CAUGHT_BY_OTHER
) {
if ( $includeBacktrace === null ) {
$includeBacktrace = self::$logExceptionBacktrace;
}
$data = [
'id' => WebRequest::getRequestId(),
'type' => get_class( $e ),
@ -639,7 +630,7 @@ TXT;
$data['suppressed'] = true;
}
if ( $includeBacktrace ) {
if ( self::$logExceptionBacktrace ) {
$data['backtrace'] = self::getRedactedTrace( $e );
}
@ -703,20 +694,16 @@ TXT;
* @param bool $pretty Add non-significant whitespace to improve readability (default: false).
* @param int $escaping Bitfield consisting of FormatJson::.*_OK class constants.
* @param string $catcher CAUGHT_BY_* class constant indicating what caught the error
* @param bool|null $includeBacktrace Whether to include a backtrace. If null,
* the value of $includeBacktrace that was provided to installHandler() will be
* used.
* @return string|false JSON string if successful; false upon failure
*/
public static function jsonSerializeException(
Throwable $e,
$pretty = false,
$escaping = 0,
$catcher = self::CAUGHT_BY_OTHER,
?bool $includeBacktrace = null
$catcher = self::CAUGHT_BY_OTHER
) {
return FormatJson::encode(
self::getStructuredExceptionData( $e, $catcher, $includeBacktrace ),
self::getStructuredExceptionData( $e, $catcher ),
$pretty,
$escaping
);

2
tests/phpunit/unit/includes/Rest/ResponseFactoryTest.php
View file

@ -162,7 +162,7 @@ class ResponseFactoryTest extends MediaWikiUnitTestCase {
$body->rewind();
$data = json_decode( $body->getContents(), true );
$this->assertSame( 500, $data['httpCode'] );
$this->assertSame( 'Error: exception of type Exception: hello', $data['message'] );
$this->assertSame( 'Error: exception of type Exception', $data['message'] );
}
public function testCreateFromExceptionWrapped() {

4
tests/phpunit/unit/includes/Rest/RouterTest.php
View file

@ -164,10 +164,6 @@ class RouterTest extends \MediaWikiUnitTestCase {
$body->rewind();
$data = json_decode( $body->getContents(), true );
$this->assertStringContainsString( 'RuntimeException', $data['message'] );
$this->assertStringContainsString( 'Fatal mock error', $data['message'] );
$this->assertSame( RuntimeException::class, $data['exception']['type'] );
$this->assertSame( 12345, $data['exception']['code'] );
$this->assertSame( 'Fatal mock error', $data['exception']['message'] );
$this->assertNotEmpty( $this->reportedErrors );
$this->assertInstanceOf( RuntimeException::class, $this->reportedErrors[0] );
}

31
tests/phpunit/unit/includes/exception/MWExceptionHandlerTest.php
View file

@ -1,6 +1,7 @@
<?php
use Wikimedia\NormalizedException\NormalizedException;
use Wikimedia\TestingAccessWrapper;
/**
* @author Antoine Musso
@ -21,6 +22,8 @@ class MWExceptionHandlerTest extends \MediaWikiUnitTestCase {
}
protected function tearDown(): void {
TestingAccessWrapper::newFromClass( MWExceptionHandler::class )
->logExceptionBacktrace = true;
ini_set( 'zend.exception_ignore_args', $this->oldSettingValue );
parent::tearDown();
}
@ -151,13 +154,7 @@ TEXT;
*/
public function testJsonserializeexceptionKeys( $expectedKeyType, $exClass, $key ) {
$json = json_decode(
MWExceptionHandler::jsonSerializeException(
new $exClass(),
true,
0,
MWExceptionHandler::CAUGHT_BY_OTHER,
true
)
MWExceptionHandler::jsonSerializeException( new $exClass() )
);
$this->assertObjectHasAttribute( $key, $json );
$this->assertSame( $expectedKeyType, gettype( $json->$key ), "Type of the '$key' key" );
@ -185,14 +182,10 @@ TEXT;
* @covers MWExceptionHandler::jsonSerializeException
*/
public function testJsonserializeexceptionBacktracingEnabled() {
TestingAccessWrapper::newFromClass( MWExceptionHandler::class )
->logExceptionBacktrace = true;
$json = json_decode(
MWExceptionHandler::jsonSerializeException(
new Exception(),
true,
0,
MWExceptionHandler::CAUGHT_BY_OTHER,
true
)
MWExceptionHandler::jsonSerializeException( new Exception() )
);
$this->assertObjectHasAttribute( 'backtrace', $json );
}
@ -204,14 +197,10 @@ TEXT;
* @covers MWExceptionHandler::jsonSerializeException
*/
public function testJsonserializeexceptionBacktracingDisabled() {
TestingAccessWrapper::newFromClass( MWExceptionHandler::class )
->logExceptionBacktrace = false;
$json = json_decode(
MWExceptionHandler::jsonSerializeException(
new Exception(),
true,
0,
MWExceptionHandler::CAUGHT_BY_OTHER,
false
)
MWExceptionHandler::jsonSerializeException( new Exception() )
);
$this->assertObjectNotHasAttribute( 'backtrace', $json );
}