Handle null value the same as unset (#1740)

tvdijen · web-flow · commit cb0e9ac59a8a · 2023-01-05T17:40:49.000+01:00
* Handle null value the same as unset

* Fix tests

* Remove obsolete if-statement
diff --git a/src/SimpleSAML/Configuration.php b/src/SimpleSAML/Configuration.php
@@ -18,6 +18,7 @@
 use function interface_exists;
 use function is_array;
 use function is_int;
+use function is_null;
 use function is_string;
 use function ob_end_clean;
 use function ob_get_length;
@@ -415,7 +416,7 @@ public function getOptionalValue(string $name, $default)
      */
     public function hasValue(string $name): bool
     {
-        return array_key_exists($name, $this->configuration);
+        return array_key_exists($name, $this->configuration) && !is_null($this->configuration[$name]);
     }
 
 
diff --git a/src/SimpleSAML/SessionHandlerPHP.php b/src/SimpleSAML/SessionHandlerPHP.php
@@ -54,7 +54,7 @@ protected function __construct()
         );
 
         if (session_status() === PHP_SESSION_ACTIVE) {
-            if (session_name() === $this->cookie_name || $this->cookie_name === null) {
+            if (session_name() === $this->cookie_name) {
                 Logger::warning(
                     'There is already a PHP session with the same name as SimpleSAMLphp\'s session, or the ' .
                     "'session.phpsession.cookiename' configuration option is not set. Make sure to set " .
diff --git a/tests/src/SimpleSAML/ConfigurationTest.php b/tests/src/SimpleSAML/ConfigurationTest.php
@@ -72,7 +72,10 @@ public function testGetValue(): void
 
         // Normal use
         $this->assertTrue($c->getValue('exists_true'));
-        $this->assertNull($c->getValue('exists_null'));
+
+        // Null option
+        $this->expectException(AssertionFailedException::class);
+        $c->getValue('exists_null');
 
         // Missing option
         $this->expectException(AssertionFailedException::class);
@@ -108,7 +111,7 @@ public function testHasValue(): void
         ]);
         $this->assertEquals($c->hasValue('missing'), false);
         $this->assertEquals($c->hasValue('exists_true'), true);
-        $this->assertEquals($c->hasValue('exists_null'), true);
+        $this->assertEquals($c->hasValue('exists_null'), false);
     }
 
 
@@ -124,7 +127,7 @@ public function testHasValueOneOf(): void
         $this->assertEquals($c->hasValueOneOf([]), false);
         $this->assertEquals($c->hasValueOneOf(['missing']), false);
         $this->assertEquals($c->hasValueOneOf(['exists_true']), true);
-        $this->assertEquals($c->hasValueOneOf(['exists_null']), true);
+        $this->assertEquals($c->hasValueOneOf(['exists_null']), false);
 
         $this->assertEquals($c->hasValueOneOf(['missing1', 'missing2']), false);
         $this->assertEquals($c->hasValueOneOf(['exists_true', 'missing']), true);

Original file line number	Diff line number	Diff line change
`@@ -18,6 +18,7 @@`
`18`	`18`	`use function interface_exists;`
`19`	`19`	`use function is_array;`
`20`	`20`	`use function is_int;`
	`21`	`+use function is_null;`
`21`	`22`	`use function is_string;`
`22`	`23`	`use function ob_end_clean;`
`23`	`24`	`use function ob_get_length;`
`@@ -415,7 +416,7 @@ public function getOptionalValue(string $name, $default)`
`415`	`416`	`*/`
`416`	`417`	`public function hasValue(string $name): bool`
`417`	`418`	`{`
`418`		`- return array_key_exists($name, $this->configuration);`
	`419`	`+ return array_key_exists($name, $this->configuration) && !is_null($this->configuration[$name]);`
`419`	`420`	`}`
`420`	`421`
`421`	`422`