Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issues detected with static analyzer. #188

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,13 @@ matrix:
allow_failures:
- php: hhvm

before_script:
- if [[ "$TRAVIS_PHP_VERSION" == "7.3" ]]; then composer require --dev vimeo/psalm; fi

script:
- if [[ $EXECUTE_COVERAGE == 'true' ]]; then phpunit --coverage-clover clover.xml tests; fi
- if [[ $EXECUTE_COVERAGE != 'true' ]]; then phpunit tests; fi
- if [[ "$TRAVIS_PHP_VERSION" == "7.3" ]]; then vendor/bin/psalm; fi

after_success:
- if [[ $EXECUTE_COVERAGE == 'true' ]]; then bash <(curl -s https://codecov.io/bash); fi
50 changes: 50 additions & 0 deletions psalm.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
<?xml version="1.0"?>
<psalm
totallyTyped="false"
>
<projectFiles>
<directory name="src" />
<ignoreFiles>
<directory name="vendor" />
</ignoreFiles>
</projectFiles>

<issueHandlers>
<LessSpecificReturnType errorLevel="info" />

<!-- level 3 issues - slightly lazy code writing, but probably low false-negatives -->

<DeprecatedMethod errorLevel="info" />
<DeprecatedProperty errorLevel="info" />
<DeprecatedClass errorLevel="info" />
<DeprecatedConstant errorLevel="info" />
<DeprecatedInterface errorLevel="info" />
<DeprecatedTrait errorLevel="info" />

<InternalMethod errorLevel="info" />
<InternalProperty errorLevel="info" />
<InternalClass errorLevel="info" />

<MissingClosureReturnType errorLevel="info" />
<MissingReturnType errorLevel="info" />
<MissingPropertyType errorLevel="info" />
<InvalidDocblock errorLevel="info" />
<MisplacedRequiredParam errorLevel="info" />

<PropertyNotSetInConstructor errorLevel="info" />
<MissingConstructor errorLevel="info" />
<MissingClosureParamType errorLevel="info" />
<MissingParamType errorLevel="info" />

<RedundantCondition errorLevel="info" />

<DocblockTypeContradiction errorLevel="info" />
<RedundantConditionGivenDocblockType errorLevel="info" />

<UnresolvableInclude errorLevel="info" />

<RawObjectIteration errorLevel="info" />

<InvalidStringClass errorLevel="info" />
</issueHandlers>
</psalm>
68 changes: 40 additions & 28 deletions src/XMLSecEnc.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
namespace RobRichards\XMLSecLibs;

use DOMDocument;
use DOMElement;
use DOMNode;
use DOMXPath;
use Exception;
Expand Down Expand Up @@ -60,8 +61,8 @@ class XMLSecEnc
const URI = 3;
const XMLENCNS = 'http://www.w3.org/2001/04/xmlenc#';

/** @var null|DOMDocument */
private $encdoc = null;
/** @var DOMDocument */
private $encdoc;

/** @var null|DOMNode */
private $rawNode = null;
Expand Down Expand Up @@ -122,7 +123,7 @@ public function setNode($node)
* @param bool $replace Whether the encrypted node should be replaced in the original tree. Default is true.
* @throws Exception
*
* @return DOMElement The <xenc:EncryptedData>-element.
* @return DOMNode The <xenc:EncryptedData>-element.
*/
public function encryptNode($objKey, $replace = true)
{
Expand Down Expand Up @@ -153,6 +154,7 @@ public function encryptNode($objKey, $replace = true)
$this->encdoc->documentElement->setAttribute('Type', self::Content);
break;
default:
// from now on, this guarantees that $this->type is either self::Element or self::Content
throw new Exception('Type is currently not supported');
}

Expand All @@ -165,21 +167,23 @@ public function encryptNode($objKey, $replace = true)
$cipherValue->appendChild($value);

if ($replace) {
switch ($this->type) {
case (self::Element):
if ($this->rawNode->nodeType == XML_DOCUMENT_NODE) {
return $this->encdoc;
}
$importEnc = $this->rawNode->ownerDocument->importNode($this->encdoc->documentElement, true);
$this->rawNode->parentNode->replaceChild($importEnc, $this->rawNode);
return $importEnc;
case (self::Content):
$importEnc = $this->rawNode->ownerDocument->importNode($this->encdoc->documentElement, true);
while ($this->rawNode->firstChild) {
$this->rawNode->removeChild($this->rawNode->firstChild);
}
$this->rawNode->appendChild($importEnc);
return $importEnc;
if ($this->type === self::Element) {
if ($this->rawNode->nodeType == XML_DOCUMENT_NODE) {
return $this->encdoc;
}
/** @var DOMNode $importEnc */
$importEnc = $this->rawNode->ownerDocument->importNode($this->encdoc->documentElement, true);
$this->rawNode->parentNode->replaceChild($importEnc, $this->rawNode);
return $importEnc;

} else { // self::Content
/** @var DOMNode $importEnc */
$importEnc = $this->rawNode->ownerDocument->importNode($this->encdoc->documentElement, true);
while ($this->rawNode->firstChild) {
$this->rawNode->removeChild($this->rawNode->firstChild);
}
$this->rawNode->appendChild($importEnc);
return $importEnc;
}
} else {
return $this->encdoc->documentElement;
Expand Down Expand Up @@ -249,10 +253,13 @@ public function getCipherValue()
* @param XMLSecurityKey $objKey The decryption key that should be used when decrypting the node.
* @param boolean $replace Whether we should replace the encrypted node in the XML document with the decrypted data. The default is true.
*
* @return string|DOMElement The decrypted data.
* @return DOMNode|string The decrypted data.
*/
public function decryptNode($objKey, $replace=true)
{
if (empty($this->rawNode)) {
throw new Exception('Node to encrypt has not been set');
}
if (! $objKey instanceof XMLSecurityKey) {
throw new Exception('Invalid Key');
}
Expand All @@ -272,7 +279,7 @@ public function decryptNode($objKey, $replace=true)
$this->rawNode->parentNode->replaceChild($importEnc, $this->rawNode);
return $importEnc;
case (self::Content):
if ($this->rawNode->nodeType == XML_DOCUMENT_NODE) {
if ($this->rawNode instanceof DOMDocument) {
$doc = $this->rawNode;
} else {
$doc = $this->rawNode->ownerDocument;
Expand Down Expand Up @@ -316,7 +323,7 @@ public function encryptKey($srcKey, $rawKey, $append=true)
$this->encKey = $encKey;
}
$encMethod = $encKey->appendChild($this->encdoc->createElementNS(self::XMLENCNS, 'xenc:EncryptionMethod'));
$encMethod->setAttribute('Algorithm', $srcKey->getAlgorith());
$encMethod->setAttribute('Algorithm', $srcKey->getAlgorithm());
if (! empty($srcKey->name)) {
$keyInfo = $encKey->appendChild($this->encdoc->createElementNS('http://www.w3.org/2000/09/xmldsig#', 'dsig:KeyInfo'));
$keyInfo->appendChild($this->encdoc->createElementNS('http://www.w3.org/2000/09/xmldsig#', 'dsig:KeyName', $srcKey->name));
Expand All @@ -336,7 +343,7 @@ public function encryptKey($srcKey, $rawKey, $append=true)

/**
* @param XMLSecurityKey $encKey
* @return DOMElement|string
* @return DOMNode|string
* @throws Exception
*/
public function decryptKey($encKey)
Expand Down Expand Up @@ -389,7 +396,10 @@ public function locateKey($node=null)
$query = ".//xmlsecenc:EncryptionMethod";
$nodeset = $xpath->query($query, $node);
if ($encmeth = $nodeset->item(0)) {
$attrAlgorithm = $encmeth->getAttribute("Algorithm");
if (!$encmeth instanceof DOMElement) {
return null;
}
$attrAlgorithm = $encmeth->getAttribute("Algorithm");
try {
$objKey = new XMLSecurityKey($attrAlgorithm, array('type' => 'private'));
} catch (Exception $e) {
Expand All @@ -402,18 +412,19 @@ public function locateKey($node=null)
}

/**
* @param null|XMLSecurityKey $objBaseKey
* @param XMLSecurityKey $objBaseKey
* @param null|DOMNode $node
* @return null|XMLSecurityKey
* @throws Exception
*/
public static function staticLocateKeyInfo($objBaseKey=null, $node=null)
public static function staticLocateKeyInfo($objBaseKey, $node=null)
{
if (empty($node) || (! $node instanceof DOMNode)) {
return null;
}
/** @var DOMDocument|null $doc */
$doc = $node->ownerDocument;
if (!$doc) {
if ($doc === null) {
return null;
}

Expand Down Expand Up @@ -472,6 +483,7 @@ public static function staticLocateKeyInfo($objBaseKey=null, $node=null)
$id = substr($uri, 1);

$query = '//xmlsecenc:EncryptedKey[@Id="'.XPath::filterAttrValue($id, XPath::DOUBLE_QUOTE).'"]';
/** @var DOMElement|null $keyElement */
$keyElement = $xpath->query($query)->item(0);
if (!$keyElement) {
throw new Exception("Unable to locate EncryptedKey with @Id='$id'.");
Expand All @@ -496,11 +508,11 @@ public static function staticLocateKeyInfo($objBaseKey=null, $node=null)
}

/**
* @param null|XMLSecurityKey $objBaseKey
* @param XMLSecurityKey $objBaseKey
* @param null|DOMNode $node
* @return null|XMLSecurityKey
*/
public function locateKeyInfo($objBaseKey=null, $node=null)
public function locateKeyInfo($objBaseKey, $node=null)
{
if (empty($node)) {
$node = $this->rawNode;
Expand Down
Loading