From d7f99a3fbd1019587b2ec6ed5c77bb3bf88ecf5d 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 | 5 +++--
.../Error/CriticalConfigurationError.php | 5 +++--
lib/SimpleSAML/Error/Error.php | 9 +++++----
lib/SimpleSAML/Error/Exception.php | 17 +++++++++--------
.../Error/UnserializableException.php | 5 +++--
lib/SimpleSAML/Error/UserAborted.php | 6 ++++--
lib/SimpleSAML/Utils/XML.php | 10 ++++------
modules/saml/lib/Error.php | 9 +++++----
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, 53 insertions(+), 46 deletions(-)
diff --git a/lib/SimpleSAML/Error/AuthSource.php b/lib/SimpleSAML/Error/AuthSource.php
index 0824f3e2b..9ee3a50d7 100644
--- a/lib/SimpleSAML/Error/AuthSource.php
+++ b/lib/SimpleSAML/Error/AuthSource.php
@@ -5,6 +5,7 @@ declare(strict_types=1);
namespace SimpleSAML\Error;
use SimpleSAML\Assert\Assert;
+use Throwable;
/**
* Baseclass for auth source exceptions.
@@ -33,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(string $authsource, string $reason, \Exception $cause = null)
+ public function __construct(string $authsource, string $reason, Throwable $cause = null)
{
$this->authsource = $authsource;
$this->reason = $reason;
diff --git a/lib/SimpleSAML/Error/CriticalConfigurationError.php b/lib/SimpleSAML/Error/CriticalConfigurationError.php
index a6d25c51d..e42f6df7b 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 \SimpleSAML\Error\Exception
*/
- public static function fromException(\Exception $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 4e69e8e27..a54075dd8 100644
--- a/lib/SimpleSAML/Error/Error.php
+++ b/lib/SimpleSAML/Error/Error.php
@@ -10,6 +10,7 @@ use SimpleSAML\Logger;
use SimpleSAML\Session;
use SimpleSAML\Utils;
use SimpleSAML\XHTML\Template;
+use Throwable;
/**
* Class that wraps SimpleSAMLphp errors in exceptions.
@@ -76,11 +77,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, ?int $httpCode = null)
+ public function __construct($errorCode, Throwable $cause = null, ?int $httpCode = null)
{
Assert::true(is_string($errorCode) || is_array($errorCode));
diff --git a/lib/SimpleSAML/Error/Exception.php b/lib/SimpleSAML/Error/Exception.php
index da53562d1..4b853e89c 100644
--- a/lib/SimpleSAML/Error/Exception.php
+++ b/lib/SimpleSAML/Error/Exception.php
@@ -7,6 +7,7 @@ namespace SimpleSAML\Error;
use SimpleSAML\Assert\Assert;
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(string $message, int $code = 0, \Exception $cause = null)
+ public function __construct(string $message, int $code = 0, Throwable $cause = null)
{
parent::__construct($message, $code);
@@ -62,11 +63,11 @@ class Exception extends \Exception
/**
* Convert any exception into a \SimpleSAML\Error\Exception.
*
- * @param \Exception $e The exception.
+ * @param \Throwable $e The exception.
*
* @return \SimpleSAML\Error\Exception The new exception.
*/
- public static function fromException(\Exception $e): Exception
+ public static function fromException(Throwable $e): Exception
{
if ($e instanceof Exception) {
return $e;
@@ -78,10 +79,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): void
+ protected function initBacktrace(Throwable $exception): void
{
$this->backtrace = [];
@@ -121,9 +122,9 @@ class Exception extends \Exception
/**
* Retrieve the cause of this exception.
*
- * @return \SimpleSAML\Error\Exception|null The cause of this exception.
+ * @return \Throwable|null The cause of this exception.
*/
- public function getCause(): ?Exception
+ public function getCause(): ?Throwable
{
return $this->cause;
}
diff --git a/lib/SimpleSAML/Error/UnserializableException.php b/lib/SimpleSAML/Error/UnserializableException.php
index 6661d473c..14ea1d7b4 100644
--- a/lib/SimpleSAML/Error/UnserializableException.php
+++ b/lib/SimpleSAML/Error/UnserializableException.php
@@ -5,6 +5,7 @@ declare(strict_types=1);
namespace SimpleSAML\Error;
use PDOException;
+use Throwable;
/**
* Class for saving normal exceptions for serialization.
@@ -32,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 b976626b0..bf8de0085 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 64a3835a7..c5101edb3 100644
--- a/modules/saml/lib/Error.php
+++ b/modules/saml/lib/Error.php
@@ -6,6 +6,7 @@ namespace SimpleSAML\Module\saml;
use SAML2\Constants;
use SimpleSAML\Assert\Assert;
+use Throwable;
/**
* Class for representing a SAML 2 error.
@@ -45,13 +46,13 @@ 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(
string $status,
string $subStatus = null,
string $statusMessage = null,
- \Exception $cause = null
+ Throwable $cause = null
) {
$st = self::shortStatus($status);
if ($subStatus !== null) {
@@ -107,10 +108,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.
+ * @param \Throwable $exception The original exception.
* @return \SimpleSAML\Error\Exception The new exception.
*/
- public static function fromException(\Exception $exception): \SimpleSAML\Error\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 f9a3a40b1..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(string $responsible, string $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 f740aedc6..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(string $responsible, string $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 46ed47a3b..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(string $responsible, string $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 8e2a31dcd..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(string $responsible, string $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 cfdbfb1e0..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(string $responsible, string $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