Bug 60380 - HttpServletRequest#logout() never calls TomcatPrincipal#logout()
Summary: HttpServletRequest#logout() never calls TomcatPrincipal#logout()
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 8
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 8.5.x-trunk
Hardware: All All
: P2 major (vote)
Target Milestone: ----
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks: 60379
  Show dependency tree
 
Reported: 2016-11-16 10:29 UTC by Michael Osipov
Modified: 2016-11-22 09:54 UTC (History)
0 users



Attachments
Patch calling TomcatPrincipal#logout() (2.73 KB, patch)
2016-11-20 18:51 UTC, Michael Osipov
Details | Diff
Patch calling TomcatPrincipal#logout() (2.16 KB, patch)
2016-11-20 18:54 UTC, Michael Osipov
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Osipov 2016-11-16 10:29:32 UTC
If the client code calls HttpServletRequest#logout(), it is delegated to getContext().getAuthenticator().logout(this); but AuthenticatorBase#logout(Request) never calls TomcatPrincipal#logout() to free resources. The only spot where this method is called is in StandardSession#expire(boolean).

A completely request-based application cannot free the principal without ugly hacks.
Comment 1 Michael Osipov 2016-11-20 18:51:07 UTC
Created attachment 34462 [details]
Patch calling TomcatPrincipal#logout()
Comment 2 Michael Osipov 2016-11-20 18:54:26 UTC
Created attachment 34463 [details]
Patch calling TomcatPrincipal#logout()
Comment 3 Mark Thomas 2016-11-21 21:18:42 UTC
Thanks for the report and the patch. I applied a slightly modified patch that used Tomcat's standard(ish) style of exception handling.

Fixed in:
- trunk for 9.0.0.M14 onwards
- 8.5.x for 8.5.9 onwards
- 8.0.x for 8.0.40 onwards
- 7.0.x for 7.0.74 onwards
Comment 4 Michael Osipov 2016-11-21 21:29:59 UTC
(In reply to Mark Thomas from comment #3)
> Thanks for the report and the patch. I applied a slightly modified patch
> that used Tomcat's standard(ish) style of exception handling.

Any reason not to keep "catch (Exception e)" because Exception extends Throwable and the ExceptionUtils still can do their work? Anything but Exception indicates some severe VM error.
Comment 5 Mark Thomas 2016-11-22 09:54:18 UTC
The reason is java.lang.StackOverflowError and anything similar that may be added / discovered.