Skip to content

Commit a614b34

Browse files
tvdijentvdijen
authored
Update for use with SSP 1.17 & Psalm (#2)
* Start testing with Psalm
1 parent 63bd369 commit a614b34

File tree

9 files changed

+95
-32
lines changed

9 files changed

+95
-32
lines changed

‎.php_cd.dist‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<?php
2+
$finder = PhpCsFixer\Finder::create()
3+
->in([
4+
__DIR__ . '/config-templates',
5+
__DIR__ . '/lib',
6+
__DIR__ . '/hooks',
7+
__DIR__ . '/tests',
8+
__DIR__ . '/www',
9+
])
10+
;
11+
return PhpCsFixer\Config::create()
12+
->setRules([
13+
'@PSR2' => true,
14+
'@PSR4' => true,
15+
'@PSR5' => true,
16+
])
17+
->setFinder($finder)
18+
;
19+

‎.travis.yml‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ php:
1111
- 7.3
1212

1313
env:
14-
- SIMPLESAMLPHP_VERSION=1.16.*
14+
- SIMPLESAMLPHP_VERSION=1.17.*
1515

1616
matrix:
1717
include:
@@ -28,15 +28,16 @@ matrix:
2828
allow_failures:
2929
- env: SIMPLESAMLPHP_VERSION=dev-master
3030
- php: 7.3
31-
- php: hhvm
3231

3332
before_script:
3433
- composer require "simplesamlphp/simplesamlphp:${SIMPLESAMLPHP_VERSION}" --no-update
3534
- composer update --no-interaction
35+
- if [[ "$TRAVIS_PHP_VERSION" == "7.3" ]]; then composer require --dev vimeo/psalm; fi
3636

3737
script:
3838
- bin/check-syntax.sh
3939
- if [[ "$TRAVIS_PHP_VERSION" == "5.6" ]]; then php vendor/phpunit/phpunit/phpunit; else php vendor/phpunit/phpunit/phpunit --no-coverage; fi
40+
- if [[ "$TRAVIS_PHP_VERSION" == "7.3" ]]; then vendor/bin/psalm; fi
4041

4142
after_success:
4243
# Codecov, need to edit bash uploader for incorrect TRAVIS_PYTHON_VERSION environment variable matching, at least until codecov/codecov-bash#133 is resolved

‎bin/check-syntax.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ PHP='/usr/bin/env php'
44
RETURN=0
55

66
# check PHP files
7-
for FILE in `find config-templates hooks lib templates www -name "*.php"`; do
7+
for FILE in `find config-templates hooks lib www -name "*.php"`; do
88
$PHP -l $FILE > /dev/null 2>&1
99
if [ $? -ne 0 ]; then
1010
echo "Syntax check failed for ${FILE}"

‎composer.json‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,11 @@
1616
}
1717
],
1818
"require": {
19-
"simplesamlphp/composer-module-installer": "~1.0"
19+
"simplesamlphp/composer-module-installer": "~1.1"
2020
},
2121
"require-dev": {
22-
"simplesamlphp/simplesamlphp": "^1.16",
23-
"phpunit/phpunit": "~4.8.35"
22+
"simplesamlphp/simplesamlphp": "^1.17",
23+
"phpunit/phpunit": "~4.8.36"
2424
}
2525
}
2626

‎hooks/hook_cron.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
* cron hook to update aggregator2 metadata.
55
*
66
* @param array &$croninfo Output
7+
* @return void
78
*/
89
function aggregator2_hook_cron(&$croninfo)
910
{

‎hooks/hook_frontpage.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
* Hook to add the aggregator2 lik to the frontpage.
55
*
66
* @param array &$links The links on the frontpage, split into sections.
7+
* @return void
78
*/
89
function aggregator2_hook_frontpage(&$links)
910
{

‎lib/Aggregator.php‎

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ class Aggregator
8484
/**
8585
* Duration we should cache generated metadata.
8686
*
87-
* @var int
87+
* @var int|null
8888
*/
8989
protected $cacheGenerated;
9090

@@ -160,7 +160,7 @@ class Aggregator
160160
*
161161
* @var string
162162
*/
163-
protected $cacheId;
163+
protected $cacheId = 'dummy';
164164

165165
/**
166166
* The cache tag for our generated metadata.
@@ -170,7 +170,7 @@ class Aggregator
170170
*
171171
* @var string
172172
*/
173-
protected $cacheTag;
173+
protected $cacheTag = 'dummy';
174174

175175
/**
176176
* The registration information for our generated metadata.
@@ -220,21 +220,23 @@ protected function __construct($id, Configuration $config)
220220
$signKey = $config->getString('sign.privatekey', null);
221221
if ($signKey !== null) {
222222
$signKey = System::resolvePath($signKey, $certDir);
223-
$this->signKey = @file_get_contents($signKey);
224-
if ($this->signKey === null) {
223+
$sk = @file_get_contents($signKey);
224+
if ($sk === false) {
225225
throw new Exception('Unable to load private key from '.var_export($signKey, true));
226226
}
227+
$this->signKey = $sk;
227228
}
228229

229230
$this->signKeyPass = $config->getString('sign.privatekey_pass', null);
230231

231232
$signCert = $config->getString('sign.certificate', null);
232233
if ($signCert !== null) {
233234
$signCert = System::resolvePath($signCert, $certDir);
234-
$this->signCert = @file_get_contents($signCert);
235-
if ($this->signCert === null) {
235+
$sc = @file_get_contents($signCert);
236+
if ($sc === false) {
236237
throw new Exception('Unable to load certificate file from '.var_export($signCert, true));
237238
}
239+
$this->signCert = $sc;
238240
}
239241

240242
$this->signAlg = $config->getString('sign.algorithm', XMLSecurityKey::RSA_SHA1);
@@ -244,7 +246,7 @@ protected function __construct($id, Configuration $config)
244246

245247
$this->sslCAFile = $config->getString('ssl.cafile', null);
246248

247-
$this->regInfo = $config->getArray('RegistrationInfo', null);
249+
$this->regInfo = $config->getArray('RegistrationInfo', []);
248250

249251
$this->initSources($config->getConfigList('sources'));
250252
}
@@ -256,6 +258,7 @@ protected function __construct($id, Configuration $config)
256258
* This is called from the constructor, and can be overridden in subclasses.
257259
*
258260
* @param array $sources The sources as an array of SimpleSAML_Configuration objects.
261+
* @return void
259262
*/
260263
protected function initSources(array $sources)
261264
{
@@ -269,6 +272,7 @@ protected function initSources(array $sources)
269272
* Return an instance of the aggregator with the given id.
270273
*
271274
* @param string $id The id of the aggregator.
275+
* @return Aggregator
272276
*/
273277
public static function getAggregator($id)
274278
{
@@ -297,6 +301,7 @@ public function getId()
297301
* @param string $data The data.
298302
* @param int $expires The timestamp the data expires.
299303
* @param string|null $tag An extra tag that can be used to verify the validity of the cached data.
304+
* @return void
300305
*/
301306
public function addCacheItem($id, $data, $expires, $tag = null)
302307
{
@@ -305,7 +310,7 @@ public function addCacheItem($id, $data, $expires, $tag = null)
305310
assert('is_int($expires)');
306311
assert('is_null($tag) || is_string($tag)');
307312

308-
$cacheFile = $this->cacheDirectory.'/'.$id;
313+
$cacheFile = strval($this->cacheDirectory).'/'.$id;
309314
try {
310315
System::writeFile($cacheFile, $data);
311316
} catch (\Exception $e) {
@@ -339,7 +344,7 @@ public function isCacheValid($id, $tag = null)
339344
assert('is_string($id)');
340345
assert('is_null($tag) || is_string($tag)');
341346

342-
$cacheFile = $this->cacheDirectory.'/'.$id;
347+
$cacheFile = strval($this->cacheDirectory).'/'.$id;
343348
if (!file_exists($cacheFile)) {
344349
return false;
345350
}
@@ -390,7 +395,7 @@ public function getCacheItem($id, $tag = null)
390395
return null;
391396
}
392397

393-
$cacheFile = $this->cacheDirectory.'/'.$id;
398+
$cacheFile = strval($this->cacheDirectory).'/'.$id;
394399
return @file_get_contents($cacheFile);
395400
}
396401

@@ -405,7 +410,7 @@ public function getCacheFile($id)
405410
{
406411
assert('is_string($id)');
407412

408-
$cacheFile = $this->cacheDirectory.'/'.$id;
413+
$cacheFile = strval($this->cacheDirectory).'/'.$id;
409414
if (!file_exists($cacheFile)) {
410415
return null;
411416
}
@@ -427,13 +432,15 @@ public function getCAFile()
427432

428433
/**
429434
* Sign the generated EntitiesDescriptor.
435+
* @return void
430436
*/
431437
protected function addSignature(SignedElement $element)
432438
{
433439
if ($this->signKey === null) {
434440
return;
435441
}
436442

443+
/** @var string $this->signAlg */
437444
$privateKey = new XMLSecurityKey($this->signAlg, ['type' => 'private']);
438445
if ($this->signKeyPass !== null) {
439446
$privateKey->passphrase = $this->signKeyPass;
@@ -456,14 +463,8 @@ protected function addSignature(SignedElement $element)
456463
*
457464
* @return array An array containing all the EntityDescriptors found.
458465
*/
459-
private static function extractEntityDescriptors($entity)
466+
private static function extractEntityDescriptors(EntitiesDescriptor $entity)
460467
{
461-
assert('$entity instanceof EntitiesDescriptor');
462-
463-
if (!($entity instanceof EntitiesDescriptor)) {
464-
return [];
465-
}
466-
467468
$results = [];
468469
foreach ($entity->children as $child) {
469470
if ($child instanceof EntityDescriptor) {
@@ -488,7 +489,7 @@ protected function getEntitiesDescriptor()
488489
$now = time();
489490

490491
// add RegistrationInfo extension if enabled
491-
if ($this->regInfo !== null) {
492+
if (!empty($this->regInfo)) {
492493
$ri = new RegistrationInfo();
493494
$ri->registrationInstant = $now;
494495
foreach ($this->regInfo as $riName => $riValues) {
@@ -607,6 +608,7 @@ protected function filter(EntitiesDescriptor $descriptor)
607608
* Set this aggregator to exclude a set of entities from the resulting aggregate.
608609
*
609610
* @param array|null $entities The entity IDs of the entities to exclude.
611+
* @return void
610612
*/
611613
public function excludeEntities($entities)
612614
{
@@ -634,6 +636,7 @@ public function excludeEntities($entities)
634636
* - 'shib13-aa': all SHIB1.3-capable attribute authorities.
635637
*
636638
* @param array|null $set An array of the different roles and protocols to filter by.
639+
* @return void
637640
*/
638641
public function setFilters($set)
639642
{
@@ -728,6 +731,7 @@ public function getMetadata()
728731

729732
/**
730733
* Update the cached copy of our metadata.
734+
* @return void
731735
*/
732736
public function updateCache()
733737
{

‎lib/EntitySource.php‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ class EntitySource
6161
*
6262
* @var \SAML2\XML\md\EntitiesDescriptor|\SAML2\XML\md\EntityDescriptor|null
6363
*/
64-
protected $metadata;
64+
protected $metadata = null;
6565

6666
/**
6767
* The cache ID.
@@ -82,7 +82,7 @@ class EntitySource
8282
*
8383
* @var bool
8484
*/
85-
protected $updateAttempted;
85+
protected $updateAttempted = false;
8686

8787

8888
/**
@@ -127,13 +127,15 @@ private function downloadMetadata()
127127
$context['ssl']['CN_match'] = parse_url($this->url, PHP_URL_HOST);
128128
}
129129

130-
$data = HTTP::fetch($this->url, $context);
131-
if ($data === false || $data === null) {
130+
try {
131+
$data = HTTP::fetch($this->url, $context, false);
132+
} catch (\SimpleSAML\Error\Exception $e) {
132133
Logger::error($this->logLoc.'Unable to load metadata from '.var_export($this->url, true));
133134
return null;
134135
}
135136

136137
$doc = new \DOMDocument();
138+
/** @var string $data */
137139
$res = $doc->loadXML($data);
138140
if (!$res) {
139141
Logger::error($this->logLoc.'Error parsing XML from '.var_export($this->url, true));
@@ -190,6 +192,7 @@ private function downloadMetadata()
190192

191193
/**
192194
* Attempt to update our cache file.
195+
* @return void
193196
*/
194197
public function updateCache()
195198
{
@@ -231,6 +234,7 @@ public function getMetadata()
231234

232235
if (!$this->aggregator->isCacheValid($this->cacheId, $this->cacheTag)) {
233236
$this->updateCache();
237+
/** @psalm-suppress TypeDoesNotContainType */
234238
if ($this->metadata !== null) {
235239
return $this->metadata;
236240
}
@@ -239,15 +243,15 @@ public function getMetadata()
239243

240244
$cacheFile = $this->aggregator->getCacheFile($this->cacheId);
241245

242-
if (!file_exists($cacheFile)) {
246+
if (is_null($cacheFile) || !file_exists($cacheFile)) {
243247
Logger::error($this->logLoc . 'No cached metadata available.');
244248
return null;
245249
}
246250

247251
Logger::debug($this->logLoc.'Using cached metadata from '.var_export($cacheFile, true));
248252

249253
$metadata = file_get_contents($cacheFile);
250-
if ($metadata !== null) {
254+
if ($metadata !== false) {
251255
$this->metadata = unserialize($metadata);
252256
return $this->metadata;
253257
}

‎psalm.xml‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
<?xml version="1.0"?>
2+
<psalm
3+
name="SimpleSAMLphp Module Aggregator2"
4+
useDocblockTypes="true"
5+
totallyTyped="false"
6+
>
7+
<projectFiles>
8+
<directory name="lib" />
9+
<directory name="hooks" />
10+
<directory name="www" />
11+
</projectFiles>
12+
13+
<issueHandlers>
14+
<LessSpecificReturnType errorLevel="info" />
15+
16+
<!-- level 3 issues - slightly lazy code writing, but probably low false-negatives -->
17+
<DeprecatedMethod errorLevel="info" />
18+
19+
<MissingClosureReturnType errorLevel="info" />
20+
<MissingReturnType errorLevel="info" />
21+
<MissingPropertyType errorLevel="info" />
22+
<InvalidDocblock errorLevel="info" />
23+
<MisplacedRequiredParam errorLevel="info" />
24+
25+
<PropertyNotSetInConstructor errorLevel="info" />
26+
<MissingConstructor errorLevel="info" />
27+
<MissingClosureParamType errorLevel="info" />
28+
<MissingParamType errorLevel="info" />
29+
<UnusedClass errorLevel="info" />
30+
<PossiblyUnusedMethod errorLevel="info" />
31+
</issueHandlers>
32+
</psalm>
33+

0 commit comments

Comments
 (0)