SIMJIYEON93 commented on PR #2066: URL: https://github.com/apache/shiro/pull/2066#issuecomment-2780623972
Thank you so much for your review and for carefully checking the details! 🙏 You're absolutely right — a `NullPointerException` does not actually occur because `instanceof` safely handles `null` values. However, the main purpose of this PR was not just to prevent a potential NPE, but to improve the overall **API robustness and developer experience**. Currently, if `AuthenticationToken.getPrincipal()` returns `null`, the method silently returns `null`, which might lead to subtle bugs later on because API users may not expect `null` in this context. Explicitly throwing an `AuthenticationException` would make the method's behavior **clearer, safer, and more consistent** with its purpose — validating authentication input. This change also aligns with the principle of **fail fast** and **make invalid states unrepresentable**, both of which improve API reliability. That said, if the maintainers prefer keeping the current behavior, I completely respect that and am happy to adjust or close the PR if needed. 🙏 Please let me know how you'd like to proceed, and thank you again for your time and feedback! -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
