Settings variables in assert()

Lines 156 to 164 in 94d61b2

    
           assert(is_array($IPHint = $this->getIPHint())); 
        
           assert(is_array($DomainHint = $this->getDomainHint())); 
        
           assert(is_array($GeolocationHint = $this->getGeolocationHint())); 
        
           assert(is_array($children = $this->getChildren())); 
        
           if (!empty($IPHint) 
        
            || !empty($DomainHint) 
        
            || !empty($GeolocationHint) 
        
            || !empty($children)) {

This section doesn't work when PHP's zend.assertions option is < 1, which is the case if you're using the php.ini-production file which ships with PHP 7.2 (or anything derived from it). Because PHP doesn't execute the assert(), the local variables are never set, so the if condition can never be true.

This affects SimpleSAMLphp 1.17.1: even if I set DiscoHints in metadata/saml20-idp-hosted.php, they simply aren't included in the metadata that's output.

Thanks for reporting this. I will fix & tag a new release ASAP.
If my memory serves me well, there are a few more of these around, so I have to go over the whole source

@dsieborger Would you be able to test if v.3.3.9 solves this?

@dsieborger Would you be able to test if v.3.3.9 solves this?

@tvdijen, when I upgraded to 3.3.9, I started getting this exception when loading saml2/idp/metadata.php:

Mar 13 18:27:14 testlogin simplesamlphp[32051]: 3 [5486a041a4] SimpleSAML\Error\Exception: Error 1 - Too few arguments to function Webmozart\Assert\Assert::isInstanceOf(), 1 passed and at least 2 expected at /usr/ru/www/simplesamlphp-1.17.1-saml2-3.3.9/vendor/webmozart/assert/src/Assert.php:371
Mar 13 18:27:14 testlogin simplesamlphp[32051]: 3 [5486a041a4] Backtrace:
Mar 13 18:27:14 testlogin simplesamlphp[32051]: 3 [5486a041a4] 2 /usr/ru/www/simplesamlphp-1.17.1-saml2-3.3.9/www/_include.php:48 (SimpleSAML_error_handler)
Mar 13 18:27:14 testlogin simplesamlphp[32051]: 3 [5486a041a4] 1 /usr/ru/www/simplesamlphp-1.17.1-saml2-3.3.9/www/_include.php:25 (SimpleSAML_exception_handler)
Mar 13 18:27:14 testlogin simplesamlphp[32051]: 3 [5486a041a4] 0 [builtin] (N/A)

The cause appears to be:

saml2/src/SAML2/XML/md/EntityDescriptor.php

Lines 460 to 461 in 67b5a9b

    
           Assert::nullOrIsInstanceOf($this->getAffiliationDescriptor()); 
        
           Assert::nullOrIsInstanceOf($this->getOrganization());

Those lines need a second argument with the expected class name. (There may be others elsewhere -- I haven't searched.)

I'll do more testing later to confirm that the original problem is fixed besides that, but I'm afraid it's taken me too long to get to the bottom of that problem for now.

Gah, I'm such an idiot. I have released v3.3.10 to fix this.

Sorry, @tvdijen, I've done more testing and it's brought more bugs to light, I'm afraid. :-P

We're missing a colon here:

saml2/src/SAML2/XML/md/SPSSODescriptor.php

Line 192 in 67b5a9b

Assert:isArray($this->getAssertionConsumerService());

We'll be better off without the accent:

saml2/src/SAML2/XML/md/RequestedAttribute.php

Line 59 in a38fdd1

Ássert::nullOrBoolean($flag);

I hope that's the last of them. My test IdP seems to be fully functional after I fixed those. The original problem (missing DiscoHints in IdP metadata) is certainly fixed.

This is just embarassing :-(
Tagged v3.3.11

	assert(is_array($IPHint = $this->getIPHint()));
	assert(is_array($DomainHint = $this->getDomainHint()));
	assert(is_array($GeolocationHint = $this->getGeolocationHint()));
	assert(is_array($children = $this->getChildren()));

	if (!empty($IPHint)
	\|\| !empty($DomainHint)
	\|\| !empty($GeolocationHint)
	\|\| !empty($children)) {

	Assert::nullOrIsInstanceOf($this->getAffiliationDescriptor());
	Assert::nullOrIsInstanceOf($this->getOrganization());