You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by GitBox <gi...@apache.org> on 2020/06/25 00:00:20 UTC

[GitHub] [guacamole-client] necouchman opened a new pull request #534: GUACAMOLE-103: Adjust checks for entity ID and ACS URL properties.

necouchman opened a new pull request #534:
URL: https://github.com/apache/guacamole-client/pull/534


   


----------------------------------------------------------------
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



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

Posted by GitBox <gi...@apache.org>.
mike-jumper merged pull request #534:
URL: https://github.com/apache/guacamole-client/pull/534


   


----------------------------------------------------------------
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



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

Posted by GitBox <gi...@apache.org>.
mike-jumper commented on pull request #534:
URL: https://github.com/apache/guacamole-client/pull/534#issuecomment-649149759


   Arg. Well ... if you re-PR the same for `staging/1.2.0`, I'll merge, etc. It's a little wonky, but git should figure it out and what's done is done.


----------------------------------------------------------------
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



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

Posted by GitBox <gi...@apache.org>.
necouchman commented on pull request #534:
URL: https://github.com/apache/guacamole-client/pull/534#issuecomment-649149509


   Ahem...I had this set to `master` instead of `staging/1.2.0`.


----------------------------------------------------------------
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



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

Posted by 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



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

Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #534:
URL: https://github.com/apache/guacamole-client/pull/534#discussion_r445246100



##########
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:
       Okay, re-opened GUACAMOLE-678 and did a pull request for that one.




----------------------------------------------------------------
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



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

Posted by GitBox <gi...@apache.org>.
necouchman commented on pull request #534:
URL: https://github.com/apache/guacamole-client/pull/534#issuecomment-649151436


   Done...sorry about that...


----------------------------------------------------------------
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