Catalina [StandardSession] bug id=66680 (misleading warn about non-serializable principal) #638
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Misleading warning message is logged in the following scenario:
When persistAuthentication="true".
When a user is logged-out of the system and redirected to the login page, they are given a session whose principle object is null.
If in that period the tomcat server is restarted, the doWriteObject from org.apache.catalina.session.StandardSession is called where on line 1489 it tries to check if the sessionPrincipal is serializable, but does not check if it is null before that - (https://github.com/apache/tomcat/blob/10.1.x/java/org/apache/catalina/session/StandardSession.java#L1489 ).
If the principal is null (like in the above-described scenario) - then the manager logs a warning message saying it cannot serialize the principal for the session. This is somewhat misleading as there is simply no principal to serialize.
Suggestion here would be to either add a null-check before logging the warning message, or add a configuration option where this particular case (one of a null principal) can be toggled, perhaps something like 'warnNullPrincipalSerialize'.
Implemented change in this PR- adding the null-check.
Without such an ability we would get a lot of these warning messages in our production and we would not know which ones came from a session that is simply unauthenticated (i.e. principal is null), and which ones came from an actual issue with serializing the principal of an authenticated user.