Re: [PR] [#2804] bugfix: fix login() once again accepts null subject [shiro]

["steinarb (via GitHub)" <gi...@apache.org>]

steinarb commented on code in PR #2807:
URL: https://github.com/apache/shiro/pull/2807#discussion_r3454582497


##########
core/src/main/java/org/apache/shiro/mgt/DefaultSecurityManager.java:
##########
@@ -305,7 +305,7 @@ public Subject login(Subject subject, AuthenticationToken token) throws Authenti
      * @param subject Subject
      */
     protected void beforeSuccessfulLogin(Subject subject) {
-        Session session = subject.getSession(false);
+        Session session = subject != null ? subject.getSession(false) : null;

Review Comment:
   I am partial to using Optional for stuff that require null-checks
   I.e. I would have done this like this (yeah, I would have used var for the local variable as well, but I'm leaving that off for now):
   ```
   Session session = Optional.ofNullable(subject).map(s -> s.getSession(false)).orElse(null);
   ```
   (I guess it is debatable whether this is more readable than the code from the PR, but it is the practice I'm following these days. It feels more robust. But it works better when it is integrated with the API, i.e when the API's functions returns Optional<something> instead of a nullable something)
   
   https://2022.javazone.no/#/program/7a7de418-d373-415c-8bb9-27cd5cd5900d
   (FWIW I've never seen it myself...)
   
   Here is the reasoning behind the talk
     https://steinar.bang.priv.no/2021/12/28/how-i-learnt-to-like-optional/
     https://steinar.bang.priv.no/2022/01/18/chaining-optionals-using-flatmap-and-map/
    
   



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