Skip to content

Commit 2c209e8

Browse files
tvdijentvdijen
committed
Fix nitpicks
1 parent 7b76e53 commit 2c209e8

5 files changed

Lines changed: 42 additions & 24 deletions

File tree

‎src/XML/ds/Transform.php‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
namespace SimpleSAML\XMLSecurity\XML\ds;
66

77
use DOMElement;
8+
use SimpleSAML\XML\Exception\InvalidDOMElementException;
89
use SimpleSAML\XMLSecurity\Constants as C;
910
use SimpleSAML\XMLSecurity\XML\ec\InclusiveNamespaces;
1011
use Webmozart\Assert\Assert;
@@ -26,14 +27,14 @@ class Transform extends AbstractDsElement
2627
/**
2728
* An XPath object.
2829
*
29-
* @var XPath|null
30+
* @var \SimpleSAML\XMLSecurity\XML\ds\XPath|null
3031
*/
3132
protected ?XPath $xpath = null;
3233

3334
/**
3435
* An InclusiveNamespaces object.
3536
*
36-
* @var InclusiveNamespaces|null
37+
* @var \SimpleSAML\XMLSecurity\XML\ec\InclusiveNamespaces|null
3738
*/
3839
protected ?InclusiveNamespaces $inclusiveNamespaces = null;
3940

@@ -42,8 +43,8 @@ class Transform extends AbstractDsElement
4243
* Initialize the Transform element.
4344
*
4445
* @param string $algorithm
45-
* @param XPath|null $xpath
46-
* @param InclusiveNamespaces|null $prefixes
46+
* @param \SimpleSAML\XMLSecurity\XML\ds\XPath|null $xpath
47+
* @param \SimpleSAML\XMLSecurity\XML\ec\InclusiveNamespaces|null $prefixes
4748
*/
4849
public function __construct(
4950
string $algorithm,
@@ -81,7 +82,7 @@ private function setAlgorithm(string $algorithm): void
8182
/**
8283
* Get the XPath associated with this transform.
8384
*
84-
* @return XPath|null
85+
* @return \SimpleSAML\XMLSecurity\XML\ds\XPath|null
8586
*/
8687
public function getXPath(): ?XPath
8788
{
@@ -92,14 +93,15 @@ public function getXPath(): ?XPath
9293
/**
9394
* Set and validate the XPath object.
9495
*
95-
* @param XPath|null $XPath
96+
* @param \SimpleSAML\XMLSecurity\XML\ds\XPath|null $XPath
9697
*/
9798
private function setXPath(?XPath $xpath): void
9899
{
99100
if ($xpath === null) {
100101
return;
101102
}
102-
Assert::eq(
103+
104+
Assert::nullOrEq(
103105
$this->algorithm,
104106
C::XPATH_URI,
105107
'Transform algorithm "' . C::XPATH_URI . '" required if XPath provided.'
@@ -111,7 +113,7 @@ private function setXPath(?XPath $xpath): void
111113
/**
112114
* Get the InclusiveNamespaces associated with this transform.
113115
*
114-
* @return InclusiveNamespaces|null
116+
* @return \SimpleSAML\XMLSecurity\XML\ec\InclusiveNamespaces|null
115117
*/
116118
public function getInclusiveNamespaces(): ?InclusiveNamespaces
117119
{
@@ -122,13 +124,14 @@ public function getInclusiveNamespaces(): ?InclusiveNamespaces
122124
/**
123125
* Set and validate the InclusiveNamespaces object.
124126
*
125-
* @param InclusiveNamespaces|null $inclusiveNamespaces
127+
* @param \SimpleSAML\XMLSecurity\XML\ec\InclusiveNamespaces|null $inclusiveNamespaces
126128
*/
127129
private function setInclusiveNamespaces(?InclusiveNamespaces $inclusiveNamespaces)
128130
{
129131
if ($inclusiveNamespaces === null) {
130132
return;
131133
}
134+
132135
Assert::oneOf(
133136
$this->algorithm,
134137
[
@@ -138,6 +141,7 @@ private function setInclusiveNamespaces(?InclusiveNamespaces $inclusiveNamespace
138141
'Transform algorithm "' . C::C14N_EXCLUSIVE_WITH_COMMENTS . '" or "' .
139142
C::C14N_EXCLUSIVE_WITHOUT_COMMENTS . '" required if InclusiveNamespaces provided.'
140143
);
144+
141145
$this->inclusiveNamespaces = $inclusiveNamespaces;
142146
}
143147

@@ -150,6 +154,9 @@ private function setInclusiveNamespaces(?InclusiveNamespaces $inclusiveNamespace
150154
*/
151155
public static function fromXML(DOMElement $xml): object
152156
{
157+
Assert::same($xml->localName, 'Transform', InvalidDOMElementException::class);
158+
Assert::same($xml->namespaceURI, Transform::NS, InvalidDOMElementException::class);
159+
153160
$alg = self::getAttribute($xml, 'Algorithm');
154161
$xpath = XPath::getChildrenOfClass($xml);
155162
Assert::maxCount($xpath, 1, 'Only one XPath element supported per Transform.');

‎src/XML/ds/XPath.php‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ class XPath extends AbstractDsElement
3636
* @param string $expression The XPath expression itself.
3737
* @param string[] $namespaces A key - value array with namespace definitions.
3838
*/
39-
public function __construct(string $expression, array $namespaces = null)
39+
public function __construct(string $expression, array $namespaces = [])
4040
{
4141
$this->setExpression($expression);
4242
$this->setNamespaces($namespaces);
@@ -82,13 +82,11 @@ public function getNamespaces(): array
8282
*
8383
* @param string[] $namespaces
8484
*/
85-
private function setNamespaces(?array $namespaces): void
85+
private function setNamespaces(array $namespaces): void
8686
{
87-
if ($namespaces === null) {
88-
return;
89-
}
9087
Assert::allString($namespaces);
9188
Assert::allString(array_keys($namespaces));
89+
9290
$this->namespaces = $namespaces;
9391
}
9492

@@ -135,4 +133,4 @@ public function toXML(DOMElement $parent = null): DOMElement
135133
}
136134
return $e;
137135
}
138-
}
136+
}

‎tests/SignatureTest.php‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use DOMDocument;
66
use PHPUnit\Framework\TestCase;
7+
use ReflectionMethod;
78
use SimpleSAML\XML\DOMDocumentFactory;
89
use SimpleSAML\XMLSecurity\Constants as C;
910
use SimpleSAML\XMLSecurity\Exception\InvalidArgumentException;
@@ -21,10 +22,10 @@ final class SignatureTest extends TestCase
2122
/** @var \DOMDocument */
2223
protected DOMDocument $basicDoc;
2324

24-
/** @var PrivateKey */
25+
/** @var \SimpleSAML\XMLSecurity\Key\PrivateKey */
2526
protected PrivateKey $privKey;
2627

27-
/** @var X509Certificate */
28+
/** @var \SimpleSAML\XMLSecurity\Key\X509Certificate */
2829
protected X509Certificate $cert;
2930

3031

@@ -402,7 +403,7 @@ public function testReferenceWithCommentsRemovedWhenIDReferenced()
402403
$signature = Signature::fromXML($doc->documentElement);
403404
$signature->setIdAttributes(['xml:id']);
404405
$this->assertEquals(['xml:id'], $signature->getIdAttributes());
405-
$validateReference = new \ReflectionMethod(Signature::class, 'validateReferences');
406+
$validateReference = new ReflectionMethod(Signature::class, 'validateReferences');
406407
$validateReference->setAccessible(true);
407408
$this->assertTrue($validateReference->invokeArgs($signature, []));
408409
}
@@ -423,7 +424,7 @@ public function testReferenceWithCommentsRemovedWhenEmptyID()
423424
$doc = DOMDocumentFactory::fromFile('tests/resources/xml/withcomment-empty-uri.xml');
424425
$signature = Signature::fromXML($doc->documentElement);
425426
$signature->setIdAttributes(['xml:id']);
426-
$validateReference = new \ReflectionMethod(Signature::class, 'validateReferences');
427+
$validateReference = new ReflectionMethod(Signature::class, 'validateReferences');
427428
$validateReference->setAccessible(true);
428429
$this->assertTrue($validateReference->invokeArgs($signature, []));
429430
}
@@ -439,7 +440,7 @@ public function testReferenceWithCommentsRemovedWhenObjectID()
439440
$doc = DOMDocumentFactory::fromFile('tests/resources/xml/withcomment-id-uri-object.xml');
440441
$signature = Signature::fromXML($doc);
441442
$signature->setIdAttributes(['xml:id']);
442-
$validateReference = new \ReflectionMethod(Signature::class, 'validateReferences');
443+
$validateReference = new ReflectionMethod(Signature::class, 'validateReferences');
443444
$validateReference->setAccessible(true);
444445
$this->assertTrue($validateReference->invokeArgs($signature, []));
445446
}
@@ -557,7 +558,7 @@ public function testVerifySignature()
557558

558559
$xml = DOMDocumentFactory::fromFile('tests/resources/xml/alt/sign-sha512-rsa-sha512-test.xml');
559560
$signature = Signature::fromXML($xml->documentElement);
560-
$validateRef = new \ReflectionMethod(Signature::class, 'validateReferences');
561+
$validateRef = new ReflectionMethod(Signature::class, 'validateReferences');
561562
$validateRef->setAccessible(true);
562563
$this->assertTrue($validateRef->invokeArgs($signature, []), 'Failed to verify formatted RSA-SHA512 signature.');
563564
}

‎tests/XML/ds/SignatureTest.php‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,13 @@ public function testMarshalling(): void
6161
dirname(dirname(dirname(__FILE__))) . '/resources/xml/ds_KeyInfo.xml'
6262
)->documentElement
6363
),
64-
[new Chunk(DOMDocumentFactory::fromString('<ds:Object xmlns:ds="http://www.w3.org/2000/09/xmldsig#"><some>Chunk</some></ds:Object>')->documentElement)],
64+
[
65+
new Chunk(
66+
DOMDocumentFactory::fromString(
67+
'<ds:Object xmlns:ds="http://www.w3.org/2000/09/xmldsig#"><some>Chunk</some></ds:Object>'
68+
)->documentElement
69+
)
70+
],
6571
'def456'
6672
);
6773

@@ -92,7 +98,13 @@ public function testMarshallingElementOrdering(): void
9298
dirname(dirname(dirname(__FILE__))) . '/resources/xml/ds_KeyInfo.xml'
9399
)->documentElement
94100
),
95-
[new Chunk(DOMDocumentFactory::fromString('<ds:Object xmlns:ds="http://www.w3.org/2000/09/xmldsig#"><some>Chunk</some></ds:Object>')->documentElement)],
101+
[
102+
new Chunk(
103+
DOMDocumentFactory::fromString(
104+
'<ds:Object xmlns:ds="http://www.w3.org/2000/09/xmldsig#"><some>Chunk</some></ds:Object>'
105+
)->documentElement
106+
)
107+
],
96108
'def456'
97109
);
98110

‎tests/XML/ec/InclusiveNamespacesTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44

55
namespace SimpleSAML\XMLSecurity\Test\XML\ec;
66

7-
use SimpleSAML\Test\XML\SerializableXMLTestTrait;
87
use PHPUnit\Framework\TestCase;
8+
use SimpleSAML\Test\XML\SerializableXMLTestTrait;
99
use SimpleSAML\XML\DOMDocumentFactory;
1010
use SimpleSAML\XMLSecurity\XML\ec\InclusiveNamespaces;
1111

0 commit comments

Comments
 (0)