Skip to content

Commit 2d91732

Browse files
committed
Fix several issues reported by scrutinizer
1 parent 2ac79bd commit 2d91732

5 files changed

Lines changed: 19 additions & 11 deletions

File tree

modules/core/src/Controller/ErrorReport.php

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use SimpleSAML\Error;
1010
use SimpleSAML\HTTP\RunnableResponse;
1111
use SimpleSAML\Logger;
12+
use SimpleSAML\Session;
1213
use SimpleSAML\Utils;
1314
use SimpleSAML\XHTML\Template;
1415
use Symfony\Component\HttpFoundation\Request;
@@ -30,18 +31,23 @@ class ErrorReport
3031
/** @var \SimpleSAML\Configuration */
3132
protected Configuration $config;
3233

34+
/** @var \SimpleSAML\Session */
35+
protected Configuration $session;
36+
3337

3438
/**
3539
* Controller constructor.
3640
*
3741
* It initializes the global configuration for the controllers implemented here.
3842
*
3943
* @param \SimpleSAML\Configuration $config The configuration to use by the controllers.
44+
* @param \SimpleSAML\Session $config The session to use by the controllers.
4045
*/
4146
public function __construct(
4247
Configuration $config
4348
) {
4449
$this->config = $config;
50+
$this->session = $session;
4551
}
4652

4753

@@ -66,10 +72,10 @@ public function main(Request $request): Response
6672
throw new Error\Exception('Invalid reportID');
6773
}
6874

69-
$data = null;
7075
try {
7176
$data = $this->session->getData('core:errorreport', $reportId);
7277
} catch (Exception $e) {
78+
$data = null;
7379
Logger::error('Error loading error report data: ' . var_export($e->getMessage(), true));
7480
}
7581

@@ -82,7 +88,7 @@ public function main(Request $request): Response
8288
'referer' => 'not set',
8389
];
8490

85-
if (isset($session)) {
91+
if (isset($this->session)) {
8692
$data['trackId'] = $session->getTrackID();
8793
}
8894
}

modules/cron/src/Controller/Cron.php

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -129,14 +129,12 @@ public function run(string $tag, string $key, string $output = 'xhtml'): Respons
129129
{
130130
$configKey = $this->cronconfig->getOptionalString('key', 'secret');
131131
if ($key !== $configKey) {
132-
Logger::error('Cron - Wrong key provided. Cron will not run.');
133-
exit;
132+
throw new Error\Exception('Cron - Wrong key provided. Cron will not run.');
134133
}
135134

136135
$cron = new \SimpleSAML\Module\cron\Cron();
137136
if (!$cron->isValidTag($tag)) {
138-
Logger::error('Cron - Illegal tag [' . $tag . '].');
139-
exit;
137+
throw new Error\Exception(sprintf('Cron - Illegal tag [%s].', $tag));
140138
}
141139

142140
$httpUtils = new Utils\HTTP();

modules/saml/src/Controller/WebBrowserSingleSignOn.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,5 @@ public function singleSignOnService(): RunnableResponse
137137
} catch (UnsupportedBindingException $e) {
138138
throw new Error\Error('SSOPARAMS', $e, 400);
139139
}
140-
Assert::true(false);
141140
}
142141
}

src/SimpleSAML/Error/Error.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,6 @@ public function show(): void
247247
) {
248248
// enable error reporting
249249
$httpUtils = new Utils\HTTP();
250-
$baseurl = $httpUtils->getBaseURL();
251250
$data['errorReportAddress'] = Module::getModuleURL('core/errorReport');
252251
}
253252

tests/modules/core/src/Controller/ErrorReportTest.php

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use SimpleSAML\Error;
1010
use SimpleSAML\HTTP\RunnableResponse;
1111
use SimpleSAML\Module\core\Controller;
12+
use SimpleSAML\Session;
1213
use SimpleSAML\XHTML\Template;
1314
use Symfony\Component\HttpFoundation\Request;
1415

@@ -23,6 +24,9 @@ class ErrorReportTest extends TestCase
2324
/** @var \SimpleSAML\Configuration */
2425
protected Configuration $config;
2526

27+
/** @var \SimpleSAML\Session */
28+
protected Session $session;
29+
2630

2731
/**
2832
* Set up for each test.
@@ -41,6 +45,8 @@ protected function setUp(): void
4145
);
4246

4347
Configuration::setPreLoadedConfig($this->config, 'config.php');
48+
49+
$this->session = Session::getSessionFromRequest();
4450
}
4551

4652

@@ -54,7 +60,7 @@ public function testErrorReportSent(): void
5460
'GET',
5561
);
5662

57-
$c = new Controller\ErrorReport($this->config);
63+
$c = new Controller\ErrorReport($this->config, $this->session);
5864

5965
$response = $c->main($request);
6066

@@ -74,7 +80,7 @@ public function testErrorReportIncorrectReportID(): void
7480
['reportId' => 'abc123'],
7581
);
7682

77-
$c = new Controller\ErrorReport($this->config);
83+
$c = new Controller\ErrorReport($this->config, $this->session);
7884

7985
$this->expectException(Error\Exception::class);
8086
$this->expectExceptionMessage('Invalid reportID');
@@ -98,7 +104,7 @@ public function testErrorReport(): void
98104
],
99105
);
100106

101-
$c = new Controller\ErrorReport($this->config);
107+
$c = new Controller\ErrorReport($this->config, $this->session);
102108

103109
$response = $c->main($request);
104110

0 commit comments

Comments
 (0)