Uploaded image for project: 'Isis'
  1. Isis
  2. ISIS-1169

Simplify IsisSessionFilter, make more resilient to possible leakage of IsisSession on thread-local, also allow RO to force a logout



    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: core-1.8.0
    • Fix Version/s: 1.9.0
    • Component/s: Isis Core
    • Labels:


      This ticket has been prompted by a report from Nacho [1] about a session being left open. Note that Nacho's app (working with Oscar Bou) doesn't use the wicket viewer, instead they have their own proprietary viewer based on Wavemaker.

      Even so, for normal operation the IsisSessionFilter is used for the Restful Objects API, so this issue could arise more widely.

      Nacho suggested a possible small improvement, though he reported that didn't completely fix the issue. Looking into the code in more detail, it would seem that the issue can arise if the closeSession() is not called somehow, perhaps from an NPE or other RuntimeException being thrown.

      Also looking at IsisSessionFilter, it seems that this code is more complex than required; the SessionState enum only ever takes the value of UNDEFINED. So this can be simplified.

      At the same time (mostly for testing this but also since it is overdue) this seems like a good opportunity to introduce the ability to "logout" of RO. After some experimentation, have decided to implement by introducing a new non-standard /user/logout resource to RO, and using redirect and a special query string to tell the IsisSessionFilter to respond with a 401. It doesn't seem to be possible to do better than this for a pure server-side solution (see eg [2])

      I've also decided - belt and braces - to get rid of the fail-fast EXPLICIT_CLOSE policy for SessionClosePolicy and instead always use AUTO_CLOSE. That way if - for some reason - the session is left open on a thread, then it will be automatically closed the next time that thread is reused. If my above refactoring is ok then this should never happen, but I'm taking the view that I'd rather the code is resilient to a bug elsewhere (and live with a bit of memory leakage) than to blow up (as it currently does).

      [1] http://isis.markmail.org/thread/2dn7tja3r466yd2m
      [2] http://stackoverflow.com/questions/233507/how-to-log-out-user-from-web-site-using-basic-authentication




            • Assignee:
              danhaywood Daniel Keir Haywood
            • Votes:
              0 Vote for this issue
              2 Start watching this issue


              • Created: