robrichards / xmlseclibs

A PHP library for XML Security
BSD 3-Clause "New" or "Revised" License
388 stars 181 forks source link

Fix issues detected with static analyzer. #188

Open jaimeperez opened 5 years ago

jaimeperez commented 5 years ago

Running vimeo/psalm on the library yields lots of errors. This PR fixes all the errors. In general, most of them are trivial to fix (type confusions, default values that don't make sense, etc).

The most difficult cases to deal with are those where we need to change the signature of a public method, namely XMLSecEnc::locateKeyInfo()and XMLSecEnc::staticLocateKeyInfo(). However, in both cases it doesn't really make sense to have the base key as an optional parameter (best-case-scenario, the method returns null; worst-case-scenario, the code breaks because we try to call the loadKey() method on a null variable).

Another slightly more complicated case is XMLSecurityDSig::getXPathObj(). Originally, it was returning null when xPathCtx and sigNode were both null. That's only possible if someone is messing up with sigNode manually (since the property is public), setting it to null, which wouldn't make any sense (you cannot expect the library to give you an XPath context that you can use to search a node, if you have actually cleared that node and there's nowhere you can search in). The proposed change assumes getXPathObj() will always return a valid XPath object, or raise an exception if none can be created (as per the case described above).