[GitHub] [shiro] lprimak commented on a diff in pull request #372: [SHIRO-512] catch SessionException in getRunAsPrincipalsStack

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

lprimak commented on code in PR #372:
URL: https://github.com/apache/shiro/pull/372#discussion_r993627018


##########
core/src/main/java/org/apache/shiro/subject/support/DelegatingSubject.java:
##########
@@ -471,7 +471,16 @@ public PrincipalCollection releaseRunAs() {
     private List<PrincipalCollection> getRunAsPrincipalsStack() {
         Session session = getSession(false);
         if (session != null) {
-            return (List<PrincipalCollection>) session.getAttribute(RUN_AS_PRINCIPALS_SESSION_KEY);
+            try {
+                return (List<PrincipalCollection>) session.getAttribute(RUN_AS_PRINCIPALS_SESSION_KEY);
+            } catch (SessionException se) {
+                // There could be a rare race condition when a session is invalidated in another thread,
+                // this thread could throw this exception, so we catch it
+                // similar issue as in clearRunAsIdentitiesInternal()
+                // See https://issues.apache.org/jira/browse/SHIRO-512
+                log.debug("Encountered session exception trying to get 'runAs' principal stack.  This "

Review Comment:
   @adamenveil Think of a session is an implementation detail, and that's all.
   Previous code had an exception leak through because of the session invalidation.
   Some of which could never be caught (if it was in the web flow) and thus random exception logs would appear,
   as stated in the multiple commenters in the JIRA
   It's not`getRunAsPrincipalsStack` job to check if the session is invalid. If it is, it just returns null in the new code,
   which is the correct behavior.
   
   Think about it like this: old code forced to check an (unrelated) session validity in every call to Shiro, which is not the right thing for an API to do.
   
   API needs to handle it's own errors.



-- 
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: commits-unsubscribe@shiro.apache.org

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