[GitHub] [guacamole-client] mike-jumper commented on a change in pull request #534: GUACAMOLE-103: Adjust checks for entity ID and ACS URL properties.

[GitBox <gi...@apache.org>]

mike-jumper commented on a change in pull request #534:
URL: https://github.com/apache/guacamole-client/pull/534#discussion_r445244498



##########
File path: extensions/guacamole-auth-saml/src/main/java/org/apache/guacamole/auth/saml/conf/ConfigurationService.java
##########
@@ -159,11 +159,10 @@
      *     The URL to send to the SAML IdP as the Client Identifier.
      *
      * @throws GuacamoleException
-     *     If guacamole.properties cannot be parsed, or if the
-     *     property is missing.
+     *     If guacamole.properties cannot be parsed.
      */
     private URI getEntityId() throws GuacamoleException {
-        return environment.getRequiredProperty(SAML_ENTITY_ID);
+        return environment.getProperty(SAML_ENTITY_ID);

Review comment:
       It looks like `URIGuacamoleProperty` was not written to handle `null` values, and a `NullPointerException` is thrown in the case that this property is actually missing.
   
   I think you'll have to add a `null` check within `parseValue()` of `URIGuacamoleProperty`, either as part of this or part of [GUACAMOLE-678](https://issues.apache.org/jira/browse/GUACAMOLE-678).




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org