You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by "sagarmiglani (via GitHub)" <gi...@apache.org> on 2023/06/21 08:14:39 UTC

[GitHub] [sling-org-apache-sling-serviceusermapper] sagarmiglani opened a new pull request, #8: SLING-11912 - Empty configuration in ServiceUserMapperImpl's Required…

sagarmiglani opened a new pull request, #8:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/8

   … Principal/User validators results in 503
   
   This PR is filtering out the empty values provided in `required.user.validators` and`required.principal.validators`.
   Empty values present in these configuration make sling request to return:
   `HTTP ERROR 503 ServletResolver service missing, cannot service requests`
   For more information, please have a look at SLING-11912


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

To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org

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


[GitHub] [sling-org-apache-sling-serviceusermapper] kwin commented on a diff in pull request #8: SLING-11912 - Empty configuration in ServiceUserMapperImpl's Required…

Posted by "kwin (via GitHub)" <gi...@apache.org>.
kwin commented on code in PR #8:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/8#discussion_r1236737027


##########
src/main/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImpl.java:
##########
@@ -559,6 +560,10 @@ private List<ServicePrincipalsValidator> getPrincipalsValidators() {
         return getValidatorsIfPresent(principalsValidators);
     }
 
+    private List<String> filterEmptyStringValues(String[] arr) {

Review Comment:
   I would add the null check on the array in here instead of outside this method.



##########
src/main/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImpl.java:
##########
@@ -559,6 +560,10 @@ private List<ServicePrincipalsValidator> getPrincipalsValidators() {
         return getValidatorsIfPresent(principalsValidators);
     }
 
+    private List<String> filterEmptyStringValues(String[] arr) {
+        return Arrays.stream(arr).filter(p -> !p.trim().isEmpty()).collect(Collectors.toList());

Review Comment:
   I would at least log that empty values are disregarded.



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

To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org

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


[GitHub] [sling-org-apache-sling-serviceusermapper] cziegeler commented on pull request #8: SLING-11912 - Empty configuration in ServiceUserMapperImpl's Required…

Posted by "cziegeler (via GitHub)" <gi...@apache.org>.
cziegeler commented on PR #8:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/8#issuecomment-1643193182

   @sagarmiglani Could you maybe add the logging as requested by @kwin ?


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

To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org

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


[GitHub] [sling-org-apache-sling-serviceusermapper] cziegeler merged pull request #8: SLING-11912 - Empty configuration in ServiceUserMapperImpl's Required…

Posted by "cziegeler (via GitHub)" <gi...@apache.org>.
cziegeler merged PR #8:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/8


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

To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org

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


[GitHub] [sling-org-apache-sling-serviceusermapper] sagarmiglani commented on pull request #8: SLING-11912 - Empty configuration in ServiceUserMapperImpl's Required…

Posted by "sagarmiglani (via GitHub)" <gi...@apache.org>.
sagarmiglani commented on PR #8:
URL: https://github.com/apache/sling-org-apache-sling-serviceusermapper/pull/8#issuecomment-1600414055

   @anchela @kwin @cziegeler Could you please help with the review? Thanks


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

To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org

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