Skip to content

Conversation

@jaimeperez
Copy link
Contributor

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).

Psalm complains that arguments 3 and 4 of DOMNode::C14N() need to be an array, and null might be given. However, those arguments are optional, so it is fine to pass null there. This must be a bug in psalm.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants