From 8f347e026486deb173a3ccfba5d6c98236cb4226 Mon Sep 17 00:00:00 2001
From: Tim van Dijen <tim.dijen@minbzk.nl>
Date: Mon, 10 Aug 2020 10:47:30 +0200
Subject: [PATCH] Improve error handling (#1365)
Improve error handling
---
lib/SimpleSAML/Error/AuthSource.php | 6 ++++--
.../Error/CriticalConfigurationError.php | 5 +++--
lib/SimpleSAML/Error/Error.php | 9 ++++----
lib/SimpleSAML/Error/Exception.php | 17 ++++++++-------
.../Error/UnserializableException.php | 7 +++++--
lib/SimpleSAML/Error/UserAborted.php | 6 ++++--
lib/SimpleSAML/Utils/XML.php | 10 ++++-----
modules/saml/lib/Error.php | 21 ++++++++++---------
modules/saml/lib/Error/NoAuthnContext.php | 5 +++--
modules/saml/lib/Error/NoAvailableIDP.php | 5 +++--
modules/saml/lib/Error/NoPassive.php | 5 +++--
modules/saml/lib/Error/NoSupportedIDP.php | 5 +++--
modules/saml/lib/Error/ProxyCountExceeded.php | 5 +++--
www/_include.php | 8 ++-----
14 files changed, 62 insertions(+), 52 deletions(-)
diff --git a/lib/SimpleSAML/Error/AuthSource.php b/lib/SimpleSAML/Error/AuthSource.php
index f6caaf1ad..0321e424c 100644
--- a/lib/SimpleSAML/Error/AuthSource.php
+++ b/lib/SimpleSAML/Error/AuthSource.php
@@ -4,6 +4,8 @@ declare(strict_types=1);
namespace SimpleSAML\Error;
+use Throwable;
+
/**
* Baseclass for auth source exceptions.
*
@@ -32,9 +34,9 @@ class AuthSource extends Error
*
* @param string $authsource Authsource module name from where this error was thrown.
* @param string $reason Description of the error.
- * @param \Exception|null $cause
+ * @param \Throwable|null $cause
*/
- public function __construct($authsource, $reason, $cause = null)
+ public function __construct(string $authsource, string $reason, Throwable $cause = null)
{
assert(is_string($authsource));
assert(is_string($reason));
diff --git a/lib/SimpleSAML/Error/CriticalConfigurationError.php b/lib/SimpleSAML/Error/CriticalConfigurationError.php
index 8ddfd4f9f..da78e2ed7 100644
--- a/lib/SimpleSAML/Error/CriticalConfigurationError.php
+++ b/lib/SimpleSAML/Error/CriticalConfigurationError.php
@@ -7,6 +7,7 @@ namespace SimpleSAML\Error;
use SimpleSAML\Configuration;
use SimpleSAML\Logger;
use SimpleSAML\Utils;
+use Throwable;
/**
* This exception represents a configuration error that we cannot recover from.
@@ -66,11 +67,11 @@ class CriticalConfigurationError extends ConfigurationError
/**
- * @param \Exception $exception
+ * @param \Throwable $exception
*
* @return CriticalConfigurationError
*/
- public static function fromException(\Exception $exception)
+ public static function fromException(Throwable $exception): Exception
{
$reason = null;
$file = null;
diff --git a/lib/SimpleSAML/Error/Error.php b/lib/SimpleSAML/Error/Error.php
index 563d37381..387cf8c5a 100644
--- a/lib/SimpleSAML/Error/Error.php
+++ b/lib/SimpleSAML/Error/Error.php
@@ -9,6 +9,7 @@ use SimpleSAML\Logger;
use SimpleSAML\Session;
use SimpleSAML\Utils;
use SimpleSAML\XHTML\Template;
+use Throwable;
/**
* Class that wraps SimpleSAMLphp errors in exceptions.
@@ -74,11 +75,11 @@ class Error extends Exception
* The error can either be given as a string, or as an array. If it is an array, the first element in the array
* (with index 0), is the error code, while the other elements are replacements for the error text.
*
- * @param mixed $errorCode One of the error codes defined in the errors dictionary.
- * @param \Exception $cause The exception which caused this fatal error (if any). Optional.
- * @param int|null $httpCode The HTTP response code to use. Optional.
+ * @param mixed $errorCode One of the error codes defined in the errors dictionary.
+ * @param \Throwable $cause The exception which caused this fatal error (if any). Optional.
+ * @param int|null $httpCode The HTTP response code to use. Optional.
*/
- public function __construct($errorCode, \Exception $cause = null, $httpCode = null)
+ public function __construct($errorCode, Throwable $cause = null, ?int $httpCode = null)
{
assert(is_string($errorCode) || is_array($errorCode));
diff --git a/lib/SimpleSAML/Error/Exception.php b/lib/SimpleSAML/Error/Exception.php
index df7ed1a52..5177fd64d 100644
--- a/lib/SimpleSAML/Error/Exception.php
+++ b/lib/SimpleSAML/Error/Exception.php
@@ -6,6 +6,7 @@ namespace SimpleSAML\Error;
use SimpleSAML\Configuration;
use SimpleSAML\Logger;
+use Throwable;
/**
* Base class for SimpleSAMLphp Exceptions
@@ -45,9 +46,9 @@ class Exception extends \Exception
*
* @param string $message Exception message
* @param int $code Error code
- * @param \Exception|null $cause The cause of this exception.
+ * @param \Throwable|null $cause The cause of this exception.
*/
- public function __construct($message, $code = 0, \Exception $cause = null)
+ public function __construct(string $message, int $code = 0, Throwable $cause = null)
{
assert(is_string($message));
assert(is_int($code));
@@ -65,11 +66,11 @@ class Exception extends \Exception
/**
* Convert any exception into a \SimpleSAML\Error\Exception.
*
- * @param \Exception $e The exception.
+ * @param \Throwable $e The exception.
*
* @return Exception The new exception.
*/
- public static function fromException(\Exception $e)
+ public static function fromException(Throwable $e): Exception
{
if ($e instanceof Exception) {
return $e;
@@ -81,10 +82,10 @@ class Exception extends \Exception
/**
* Load the backtrace from the given exception.
*
- * @param \Exception $exception The exception we should fetch the backtrace from.
+ * @param \Throwable $exception The exception we should fetch the backtrace from.
* @return void
*/
- protected function initBacktrace(\Exception $exception)
+ protected function initBacktrace(Throwable $exception): void
{
$this->backtrace = [];
@@ -124,9 +125,9 @@ class Exception extends \Exception
/**
* Retrieve the cause of this exception.
*
- * @return Exception|null The cause of this exception.
+ * @return \Throwable|null The cause of this exception.
*/
- public function getCause()
+ public function getCause(): ?Throwable
{
return $this->cause;
}
diff --git a/lib/SimpleSAML/Error/UnserializableException.php b/lib/SimpleSAML/Error/UnserializableException.php
index dcee7b0a5..287475590 100644
--- a/lib/SimpleSAML/Error/UnserializableException.php
+++ b/lib/SimpleSAML/Error/UnserializableException.php
@@ -4,6 +4,9 @@ declare(strict_types=1);
namespace SimpleSAML\Error;
+use PDOException;
+use Throwable;
+
/**
* Class for saving normal exceptions for serialization.
*
@@ -30,9 +33,9 @@ class UnserializableException extends Exception
/**
* Create a serializable exception representing an unserializable exception.
*
- * @param \Exception $original The original exception.
+ * @param \Throwable $original The original exception.
*/
- public function __construct(\Exception $original)
+ public function __construct(Throwable $original)
{
$this->class = get_class($original);
diff --git a/lib/SimpleSAML/Error/UserAborted.php b/lib/SimpleSAML/Error/UserAborted.php
index 8d06695c6..e4926f636 100644
--- a/lib/SimpleSAML/Error/UserAborted.php
+++ b/lib/SimpleSAML/Error/UserAborted.php
@@ -4,6 +4,8 @@ declare(strict_types=1);
namespace SimpleSAML\Error;
+use Throwable;
+
/**
* Exception indicating user aborting the authentication process.
*
@@ -15,9 +17,9 @@ class UserAborted extends Error
/**
* Create the error
*
- * @param \Exception|null $cause The exception that caused this error.
+ * @param \Throwable|null $cause The exception that caused this error.
*/
- public function __construct(\Exception $cause = null)
+ public function __construct(Throwable $cause = null)
{
parent::__construct('USERABORTED', $cause);
}
diff --git a/lib/SimpleSAML/Utils/XML.php b/lib/SimpleSAML/Utils/XML.php
index 8c178fee1..63e819f39 100644
--- a/lib/SimpleSAML/Utils/XML.php
+++ b/lib/SimpleSAML/Utils/XML.php
@@ -59,12 +59,10 @@ class XML
$enabled = Configuration::getInstance()->getBoolean('debug.validatexml', false);
if (
- !(in_array('validatexml', $debug, true) // implicitly enabled
- || (array_key_exists('validatexml', $debug)
- && $debug['validatexml'] === true)
- // explicitly enabled
- // TODO: deprecate this option and remove it in 2.0
- || $enabled) // old 'debug.validatexml' configuration option
+ !(
+ in_array('validatexml', $debug, true)
+ || (array_key_exists('validatexml', $debug) && ($debug['validatexml'] === true))
+ )
) {
// XML validation is disabled
return;
diff --git a/modules/saml/lib/Error.php b/modules/saml/lib/Error.php
index c91c8e97d..11a264547 100644
--- a/modules/saml/lib/Error.php
+++ b/modules/saml/lib/Error.php
@@ -5,6 +5,7 @@ declare(strict_types=1);
namespace SimpleSAML\Module\saml;
use SAML2\Constants;
+use Throwable;
/**
* Class for representing a SAML 2 error.
@@ -44,14 +45,14 @@ class Error extends \SimpleSAML\Error\Exception
* Can be NULL, in which case there is no second-level status code.
* @param string|null $statusMessage The status message.
* Can be NULL, in which case there is no status message.
- * @param \Exception|null $cause The cause of this exception. Can be NULL.
+ * @param \Throwable|null $cause The cause of this exception. Can be NULL.
*/
- public function __construct($status, $subStatus = null, $statusMessage = null, \Exception $cause = null)
- {
- assert(is_string($status));
- assert($subStatus === null || is_string($subStatus));
- assert($statusMessage === null || is_string($statusMessage));
-
+ public function __construct(
+ string $status,
+ string $subStatus = null,
+ string $statusMessage = null,
+ Throwable $cause = null
+ ) {
$st = self::shortStatus($status);
if ($subStatus !== null) {
$st .= '/' . self::shortStatus($subStatus);
@@ -106,10 +107,10 @@ class Error extends \SimpleSAML\Error\Exception
* This function attempts to create a SAML2 error with the appropriate
* status codes from an arbitrary exception.
*
- * @param \Exception $exception The original exception.
- * @return \SimpleSAML\Module\saml\Error The new exception.
+ * @param \Throwable $exception The original exception.
+ * @return \SimpleSAML\Error\Exception The new exception.
*/
- public static function fromException(\Exception $exception)
+ public static function fromException(Throwable $exception): \SimpleSAML\Error\Exception
{
if ($exception instanceof \SimpleSAML\Module\saml\Error) {
// Return the original exception unchanged
diff --git a/modules/saml/lib/Error/NoAuthnContext.php b/modules/saml/lib/Error/NoAuthnContext.php
index 508da8e0c..100951540 100644
--- a/modules/saml/lib/Error/NoAuthnContext.php
+++ b/modules/saml/lib/Error/NoAuthnContext.php
@@ -5,6 +5,7 @@ declare(strict_types=1);
namespace SimpleSAML\Module\saml\Error;
use SAML2\Constants;
+use Throwable;
/**
* A SAML error indicating that none of the requested Authentication Contexts can be used.
@@ -21,9 +22,9 @@ class NoAuthnContext extends \SimpleSAML\Module\saml\Error
* - \SAML2\Constants::STATUS_RESPONDER: in case the error is caused by this SAML responder.
* - \SAML2\Constants::STATUS_REQUESTER: in case the error is caused by the SAML requester.
* @param string|null $message A short message explaining why this error happened.
- * @param \Exception|null $cause An exception that caused this error.
+ * @param \Throwable|null $cause An exception that caused this error.
*/
- public function __construct($responsible, $message = null, \Exception $cause = null)
+ public function __construct(string $responsible, string $message = null, Throwable $cause = null)
{
parent::__construct($responsible, Constants::STATUS_NO_AUTHN_CONTEXT, $message, $cause);
}
diff --git a/modules/saml/lib/Error/NoAvailableIDP.php b/modules/saml/lib/Error/NoAvailableIDP.php
index f1c66ab4b..5b6900092 100644
--- a/modules/saml/lib/Error/NoAvailableIDP.php
+++ b/modules/saml/lib/Error/NoAvailableIDP.php
@@ -5,6 +5,7 @@ declare(strict_types=1);
namespace SimpleSAML\Module\saml\Error;
use SAML2\Constants;
+use Throwable;
/**
* A SAML error indicating that none of the requested IdPs can be used.
@@ -21,9 +22,9 @@ class NoAvailableIDP extends \SimpleSAML\Module\saml\Error
* - \SAML2\Constants::STATUS_RESPONDER: in case the error is caused by this SAML responder.
* - \SAML2\Constants::STATUS_REQUESTER: in case the error is caused by the SAML requester.
* @param string|null $message A short message explaining why this error happened.
- * @param \Exception|null $cause An exception that caused this error.
+ * @param \Throwable|null $cause An exception that caused this error.
*/
- public function __construct($responsible, $message = null, \Exception $cause = null)
+ public function __construct(string $responsible, string $message = null, Throwable $cause = null)
{
parent::__construct($responsible, Constants::STATUS_NO_AVAILABLE_IDP, $message, $cause);
}
diff --git a/modules/saml/lib/Error/NoPassive.php b/modules/saml/lib/Error/NoPassive.php
index 3fc03e049..f6e8f7251 100644
--- a/modules/saml/lib/Error/NoPassive.php
+++ b/modules/saml/lib/Error/NoPassive.php
@@ -5,6 +5,7 @@ declare(strict_types=1);
namespace SimpleSAML\Module\saml\Error;
use SAML2\Constants;
+use Throwable;
/**
* A SAML error indicating that passive authentication cannot be used.
@@ -21,9 +22,9 @@ class NoPassive extends \SimpleSAML\Module\saml\Error
* - \SAML2\Constants::STATUS_RESPONDER: in case the error is caused by this SAML responder.
* - \SAML2\Constants::STATUS_REQUESTER: in case the error is caused by the SAML requester.
* @param string|null $message A short message explaining why this error happened.
- * @param \Exception|null $cause An exception that caused this error.
+ * @param \Throwable|null $cause An exception that caused this error.
*/
- public function __construct($responsible, $message = null, \Exception $cause = null)
+ public function __construct(string $responsible, string $message = null, Throwable $cause = null)
{
parent::__construct($responsible, Constants::STATUS_NO_PASSIVE, $message, $cause);
}
diff --git a/modules/saml/lib/Error/NoSupportedIDP.php b/modules/saml/lib/Error/NoSupportedIDP.php
index 7806fb379..13fa5c391 100644
--- a/modules/saml/lib/Error/NoSupportedIDP.php
+++ b/modules/saml/lib/Error/NoSupportedIDP.php
@@ -5,6 +5,7 @@ declare(strict_types=1);
namespace SimpleSAML\Module\saml\Error;
use SAML2\Constants;
+use Throwable;
/**
* A SAML error indicating that none of the IdPs requested are supported.
@@ -21,9 +22,9 @@ class NoSupportedIDP extends \SimpleSAML\Module\saml\Error
* - \SAML2\Constants::STATUS_RESPONDER: in case the error is caused by this SAML responder.
* - \SAML2\Constants::STATUS_REQUESTER: in case the error is caused by the SAML requester.
* @param string|null $message A short message explaining why this error happened.
- * @param \Exception|null $cause An exception that caused this error.
+ * @param \Throwable|null $cause An exception that caused this error.
*/
- public function __construct($responsible, $message = null, \Exception $cause = null)
+ public function __construct(string $responsible, string $message = null, Throwable $cause = null)
{
parent::__construct($responsible, Constants::STATUS_NO_SUPPORTED_IDP, $message, $cause);
}
diff --git a/modules/saml/lib/Error/ProxyCountExceeded.php b/modules/saml/lib/Error/ProxyCountExceeded.php
index 044aded1f..e078c803b 100644
--- a/modules/saml/lib/Error/ProxyCountExceeded.php
+++ b/modules/saml/lib/Error/ProxyCountExceeded.php
@@ -5,6 +5,7 @@ declare(strict_types=1);
namespace SimpleSAML\Module\saml\Error;
use SAML2\Constants;
+use Throwable;
/**
* A SAML error indicating that the maximum amount of proxies traversed has been reached.
@@ -21,9 +22,9 @@ class ProxyCountExceeded extends \SimpleSAML\Module\saml\Error
* - \SAML2\Constants::STATUS_RESPONDER: in case the error is caused by this SAML responder.
* - \SAML2\Constants::STATUS_REQUESTER: in case the error is caused by the SAML requester.
* @param string|null $message A short message explaining why this error happened.
- * @param \Exception|null $cause An exception that caused this error.
+ * @param \Throwable|null $cause An exception that caused this error.
*/
- public function __construct($responsible, $message = null, \Exception $cause = null)
+ public function __construct(string $responsible, string $message = null, Throwable $cause = null)
{
parent::__construct($responsible, Constants::STATUS_PROXY_COUNT_EXCEEDED, $message, $cause);
}
diff --git a/www/_include.php b/www/_include.php
index afe9b6b39..fa3035c15 100644
--- a/www/_include.php
+++ b/www/_include.php
@@ -17,12 +17,8 @@ function SimpleSAML_exception_handler($exception)
$e = new \SimpleSAML\Error\Error('UNHANDLEDEXCEPTION', $exception);
$e->show();
} elseif (class_exists('Error') && $exception instanceof \Error) {
- $code = $exception->getCode();
- $errno = ($code > 0) ? $code : E_ERROR;
- $errstr = $exception->getMessage();
- $errfile = $exception->getFile();
- $errline = $exception->getLine();
- SimpleSAML_error_handler($errno, $errstr, $errfile, $errline);
+ $e = new \SimpleSAML\Error\Error('UNHANDLEDEXCEPTION', $exception);
+ $e->show();
}
}
--
GitLab