From 43dcdf0ea4f39072a19404ce5aa5ec1eb585f30d Mon Sep 17 00:00:00 2001
From: Tim van Dijen <tim.dijen@minbzk.nl>
Date: Thu, 15 Aug 2019 16:52:32 +0200
Subject: [PATCH] Fully typehint lib/XML/*.php

---
 lib/SimpleSAML/Utils/XML.php     |  2 +-
 lib/SimpleSAML/XML/Errors.php    | 13 ++++------
 lib/SimpleSAML/XML/Parser.php    | 23 +++++++++---------
 lib/SimpleSAML/XML/Signer.php    | 41 +++++++++-----------------------
 lib/SimpleSAML/XML/Validator.php | 11 ++++-----
 5 files changed, 33 insertions(+), 57 deletions(-)

diff --git a/lib/SimpleSAML/Utils/XML.php b/lib/SimpleSAML/Utils/XML.php
index 46c3ef91f..c7590f7bf 100644
--- a/lib/SimpleSAML/Utils/XML.php
+++ b/lib/SimpleSAML/Utils/XML.php
@@ -416,7 +416,7 @@ class XML
      */
     public static function isValid($xml, string $schema)
     {
-        if (!(is_string($xml) || $xml instanceof DOMDocument))) {
+        if (!is_string($xml) && ! ($xml instanceof DOMDocument)) {
             throw new \InvalidArgumentException('Invalid input parameters.');
         }
 
diff --git a/lib/SimpleSAML/XML/Errors.php b/lib/SimpleSAML/XML/Errors.php
index 2f7114f53..5f000349c 100644
--- a/lib/SimpleSAML/XML/Errors.php
+++ b/lib/SimpleSAML/XML/Errors.php
@@ -35,7 +35,7 @@ class Errors
      *
      * @return void
      */
-    private static function addErrors()
+    private static function addErrors(): void
     {
         $currentErrors = libxml_get_errors();
         libxml_clear_errors();
@@ -53,7 +53,7 @@ class Errors
      *
      * @return void
      */
-    public static function begin()
+    public static function begin(): void
     {
 
         // Check whether the error access functions are present
@@ -82,7 +82,7 @@ class Errors
      *
      * @return array  An array with the LibXMLErrors which has occurred since begin() was called.
      */
-    public static function end()
+    public static function end(): array
     {
         // Check whether the error access functions are present
         if (!function_exists('libxml_use_internal_errors')) {
@@ -113,9 +113,8 @@ class Errors
      * @param \LibXMLError $error  The LibXMLError which should be formatted.
      * @return string  A string representing the given LibXMLError.
      */
-    public static function formatError($error)
+    public static function formatError(LibXMLError $error): string
     {
-        Assert::isInstanceOf($error, LibXMLError::class);
         return 'level=' . $error->level
             . ',code=' . $error->code
             . ',line=' . $error->line
@@ -134,10 +133,8 @@ class Errors
      * @return string  A string representing the errors. An empty string will be returned if there were no
      *          errors in the array.
      */
-    public static function formatErrors($errors)
+    public static function formatErrors(array $errors): string
     {
-        Assert::isArray($errors);
-
         $ret = '';
         foreach ($errors as $error) {
             $ret .= self::formatError($error) . "\n";
diff --git a/lib/SimpleSAML/XML/Parser.php b/lib/SimpleSAML/XML/Parser.php
index dead12554..9da5dd381 100644
--- a/lib/SimpleSAML/XML/Parser.php
+++ b/lib/SimpleSAML/XML/Parser.php
@@ -11,6 +11,8 @@ declare(strict_types=1);
 
 namespace SimpleSAML\XML;
 
+use SimpleXMLElement;
+
 class Parser
 {
     /** @var \SimpleXMLElement */
@@ -19,28 +21,27 @@ class Parser
     /**
      * @param string $xml
      */
-    public function __construct($xml)
+    public function __construct(string $xml)
     {
         $this->simplexml = new \SimpleXMLElement($xml);
         $this->simplexml->registerXPathNamespace('saml2', 'urn:oasis:names:tc:SAML:2.0:assertion');
         $this->simplexml->registerXPathNamespace('saml2meta', 'urn:oasis:names:tc:SAML:2.0:metadata');
         $this->simplexml->registerXPathNamespace('ds', 'http://www.w3.org/2000/09/xmldsig#');
     }
-    
+
 
     /**
      * @param \SimpleXMLElement $element
      * @return \SimpleSAML\XML\Parser
-     * @psalm-return \SimpleSAML\XML\Parser
      */
-    public static function fromSimpleXMLElement(\SimpleXMLElement $element)
+    public static function fromSimpleXMLElement(SimpleXMLElement $element) : Parser
     {
         // Traverse all existing namespaces in element
         $namespaces = $element->getNamespaces();
         foreach ($namespaces as $prefix => $ns) {
             $element[(($prefix === '') ? 'xmlns' : 'xmlns:' . $prefix)] = $ns;
         }
-        
+
         /* Create a new parser with the xml document where the namespace definitions
          * are added.
          */
@@ -50,7 +51,7 @@ class Parser
         }
         return new Parser($xml);
     }
-    
+
 
     /**
      * @param string $xpath
@@ -58,7 +59,7 @@ class Parser
      * @throws \Exception
      * @return string
      */
-    public function getValueDefault($xpath, $defvalue)
+    public function getValueDefault(string $xpath, string $defvalue) : string
     {
         try {
             /** @var string */
@@ -67,7 +68,7 @@ class Parser
             return $defvalue;
         }
     }
-    
+
 
     /**
      * @param string $xpath
@@ -75,7 +76,7 @@ class Parser
      * @throws \Exception
      * @return string|null
      */
-    public function getValue($xpath, $required = false)
+    public function getValue(string $xpath, bool $required = false) : ?string
     {
         $result = $this->simplexml->xpath($xpath);
         if (!is_array($result) || empty($result)) {
@@ -89,7 +90,7 @@ class Parser
         }
         return (string) $result[0];
     }
-    
+
 
     /**
      * @param array $xpath
@@ -97,7 +98,7 @@ class Parser
      * @throws \Exception
      * @return string|null
      */
-    public function getValueAlternatives(array $xpath, $required = false)
+    public function getValueAlternatives(array $xpath, bool $required = false) : ?string
     {
         foreach ($xpath as $x) {
             $seek = $this->getValue($x);
diff --git a/lib/SimpleSAML/XML/Signer.php b/lib/SimpleSAML/XML/Signer.php
index 7c84570fb..f4c9716bc 100644
--- a/lib/SimpleSAML/XML/Signer.php
+++ b/lib/SimpleSAML/XML/Signer.php
@@ -61,10 +61,8 @@ class Signer
      *
      * @param array $options  Associative array with options for the constructor. Defaults to an empty array.
      */
-    public function __construct($options = [])
+    public function __construct(array $options = [])
     {
-        Assert::isArray($options);
-
         if (array_key_exists('privatekey', $options)) {
             $pass = null;
             if (array_key_exists('privatekey_pass', $options)) {
@@ -101,9 +99,8 @@ class Signer
      * @param array $privatekey  The private key.
      * @return void
      */
-    public function loadPrivateKeyArray($privatekey)
+    public function loadPrivateKeyArray(array $privatekey): void
     {
-        Assert::isArray($privatekey);
         Assert::keyExists($privatekey, 'PEM');
 
         $this->privateKey = new XMLSecurityKey(XMLSecurityKey::RSA_SHA256, ['type' => 'private']);
@@ -128,12 +125,8 @@ class Signer
      * @throws \Exception
      * @return void
      */
-    public function loadPrivateKey($file, $pass = null, $full_path = false)
+    public function loadPrivateKey(string $file, ?string $pass, bool $full_path = false): void
     {
-        Assert::string($file);
-        Assert::nullOrString($pass);
-        Assert::boolean($full_path);
-
         if (!$full_path) {
             $keyFile = Utils\Config::getCertPath($file);
         } else {
@@ -166,10 +159,8 @@ class Signer
      * @throws \Exception
      * @return void
      */
-    public function loadPublicKeyArray($publickey)
+    public function loadPublicKeyArray(array $publickey): void
     {
-        Assert::isArray($publickey);
-
         if (!array_key_exists('PEM', $publickey)) {
             // We have a public key with only a fingerprint
             throw new \Exception('Tried to add a certificate fingerprint in a signature.');
@@ -193,11 +184,8 @@ class Signer
      * @throws \Exception
      * @return void
      */
-    public function loadCertificate($file, $full_path = false)
+    public function loadCertificate(string $file, bool $full_path = false): void
     {
-        Assert::string($file);
-        Assert::boolean($full_path);
-
         if (!$full_path) {
             $certFile = Utils\Config::getCertPath($file);
         } else {
@@ -222,10 +210,8 @@ class Signer
      * @param string $idAttrName  The name of the attribute which contains the id.
      * @return void
      */
-    public function setIDAttribute($idAttrName)
+    public function setIDAttribute(string $idAttrName): void
     {
-        Assert::string($idAttrName);
-
         $this->idAttrName = $idAttrName;
     }
 
@@ -242,11 +228,8 @@ class Signer
      * @throws \Exception
      * @return void
      */
-    public function addCertificate($file, $full_path = false)
+    public function addCertificate(string $file, bool $full_path = false): void
     {
-        Assert::string($file);
-        Assert::boolean($full_path);
-
         if (!$full_path) {
             $certFile = Utils\Config::getCertPath($file);
         } else {
@@ -273,16 +256,14 @@ class Signer
      *
      * @param \DOMElement $node  The DOMElement we should generate a signature for.
      * @param \DOMElement $insertInto  The DOMElement we should insert the signature element into.
-     * @param \DOMElement $insertBefore  The element we should insert the signature element before. Defaults to NULL,
-     *                                   in which case the signature will be appended to the element spesified in
-     *                                   $insertInto.
+     * @param \DOMElement|\DOMComment|\DOMText $insertBefore
+     *  The element we should insert the signature element before. Defaults to NULL,
+     *  in which case the signature will be appended to the element spesified in $insertInto.
      * @throws \Exception
      * @return void
      */
-    public function sign($node, $insertInto, $insertBefore = null)
+    public function sign(DOMElement $node, DOMElement $insertInto, $insertBefore = null): void
     {
-        Assert::isInstanceOf($node, DOMElement::class);
-        Assert::isInstanceOf($insertInto, DOMElement::class);
         Assert::nullOrInstanceOfAny($insertBefore, [DOMElement::class, DOMComment::class, DOMText::class]);
 
         $privateKey = $this->privateKey;
diff --git a/lib/SimpleSAML/XML/Validator.php b/lib/SimpleSAML/XML/Validator.php
index f2f0ac1ab..91cbbe581 100644
--- a/lib/SimpleSAML/XML/Validator.php
+++ b/lib/SimpleSAML/XML/Validator.php
@@ -12,6 +12,7 @@ declare(strict_types=1);
 namespace SimpleSAML\XML;
 
 use DOMNode;
+use DOMDocument;
 use RobRichards\XMLSecLibs\XMLSecEnc;
 use RobRichards\XMLSecLibs\XMLSecurityDSig;
 use SimpleSAML\Logger;
@@ -49,10 +50,8 @@ class Validator
      * @param array|false $publickey The public key / certificate which should be used to validate the XML node.
      * @throws \Exception
      */
-    public function __construct($xmlNode, $idAttribute = null, $publickey = false)
+    public function __construct(DOMDocument $xmlNode, $idAttribute = null, $publickey = false)
     {
-        Assert::isInstanceOf($xmlNode, DOMNode::class);
-
         if ($publickey === null) {
             $publickey = false;
         } elseif (is_string($publickey)) {
@@ -131,7 +130,7 @@ class Validator
      *
      * @return string|null  The certificate as a PEM-encoded string, or NULL if not signed with an X509 certificate.
      */
-    public function getX509Certificate()
+    public function getX509Certificate(): ?string
     {
         return $this->x509Certificate;
     }
@@ -144,10 +143,8 @@ class Validator
      *
      * @return bool  TRUE if this node (or a parent node) was signed. FALSE if not.
      */
-    public function isNodeValidated($node)
+    public function isNodeValidated(DOMNode $node): bool
     {
-        Assert::isInstanceOf($node, DOMNode::class);
-
         if ($this->validNodes !== null) {
             while ($node !== null) {
                 if (in_array($node, $this->validNodes, true)) {
-- 
GitLab