glassfish
  1. glassfish
  2. GLASSFISH-896

[OPTIMIZATION] Do not include cookie for invalidated HTTP session in the response

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 9.0pe
    • Fix Version/s: 9.1pe_dev
    • Component/s: web_container
    • Labels:
      None
    • Environment:

      Operating System: All
      Platform: Sun

    • Issuezilla Id:
      896
    • Status Whiteboard:
      Hide

      fixed-pwc12

      Show
      fixed-pwc12

      Description

      The current impl appends a cookie to the response at the time when a session is
      created. The cookie is never removed from the response should the session later
      be invalidated. This causes an unnecessary lookup for the already invalidated
      session id on a subsequent request that carries the cookie with the invalidated
      session id.

      By not appending the cookie for the invalidated session to the response, or by
      removing it from the response (if already added) before the response is
      committed, the redundant session lookup will be avoided by the subsequent request.

      In cases where the session lookup is expensive (i.e., HADB), the resulting
      performance gains will be substantial.

        Activity

        Hide
        jluehe added a comment -

        Incremental fix:

        • Based on feedback from WS team, CoyoteRequest.initSessionTracker()
          is no longer an exposed interface, but now has private scope and is
          invoked as part of CoyoteRequest.setContext(). Leaving it as an
          exposed interface would have resulted in a backwards-incompatible
          change with respect to PWC's arch'ed interfaces.
        • CoyoteRequest.initSessionTracker() no longer calls setAttribute() to
          store the session tracker in the request. Instead, it adds the
          session tracker to the request attributes hashmap directly, avoiding
          notification of any ServletRequestAttributeListeners. Since
          session tracker is an implementation detail, there is no need to
          issue any notification. This also fixes 6467897 ("NamingException in
          Servlet 2.5 Request Attribute Listener test").

        Checking in CoyoteAdapter.java;
        /cvs/glassfish/appserv-webtier/src/java/org/apache/coyote/tomcat5/CoyoteAdapter.java,v
        <-- CoyoteAdapter.java
        new revision: 1.20; previous revision: 1.19
        done
        Checking in CoyoteRequest.java;
        /cvs/glassfish/appserv-webtier/src/java/org/apache/coyote/tomcat5/CoyoteRequest.java,v
        <-- CoyoteRequest.java
        new revision: 1.36; previous revision: 1.35
        done
        Checking in CoyoteResponse.java;
        /cvs/glassfish/appserv-webtier/src/java/org/apache/coyote/tomcat5/CoyoteResponse.java,v
        <-- CoyoteResponse.javanew revision: 1.16; previous revision: 1.15
        done

        Show
        jluehe added a comment - Incremental fix: Based on feedback from WS team, CoyoteRequest.initSessionTracker() is no longer an exposed interface, but now has private scope and is invoked as part of CoyoteRequest.setContext(). Leaving it as an exposed interface would have resulted in a backwards-incompatible change with respect to PWC's arch'ed interfaces. CoyoteRequest.initSessionTracker() no longer calls setAttribute() to store the session tracker in the request. Instead, it adds the session tracker to the request attributes hashmap directly, avoiding notification of any ServletRequestAttributeListeners. Since session tracker is an implementation detail, there is no need to issue any notification. This also fixes 6467897 ("NamingException in Servlet 2.5 Request Attribute Listener test"). Checking in CoyoteAdapter.java; /cvs/glassfish/appserv-webtier/src/java/org/apache/coyote/tomcat5/CoyoteAdapter.java,v <-- CoyoteAdapter.java new revision: 1.20; previous revision: 1.19 done Checking in CoyoteRequest.java; /cvs/glassfish/appserv-webtier/src/java/org/apache/coyote/tomcat5/CoyoteRequest.java,v <-- CoyoteRequest.java new revision: 1.36; previous revision: 1.35 done Checking in CoyoteResponse.java; /cvs/glassfish/appserv-webtier/src/java/org/apache/coyote/tomcat5/CoyoteResponse.java,v <-- CoyoteResponse.javanew revision: 1.16; previous revision: 1.15 done
        Hide
        jluehe added a comment -

        Added protection against accidental removal of session cookie from response in
        very unlikely scenario in which:

        1. A servlet in Context A creates a session. The SessionTracker tracks
        it. The request returns before the session expires.

        2. While A's session is still active, a servlet in Context B creates a
        session.
        It so happens that the session in B has the same sessionid as the one
        that was generated in A. It also so happens that the request on which
        the session is created in B is the same (pooled) request (with the
        same SessionTracker!) on which the session in A had been created. It
        also so happens that the session created in A expires before the
        request returns from B.

        In this case, the SessionTracker will get notfified of the session expiration,
        and since the id of the expired session is the same as the session id it is
        currently tracking, the SessionTracker will decrement its counter of tracked
        sessions and remove the JSESSIONID cookie from the response if the counter has
        dropped to zero.

        The added protection will prevent this from happening, by having a
        SessionTracker not only keep track of the session id it is tracking, but also of
        the list of contexts whose sessions it is tracking (this list will be reset when
        a request and its SessionTracker are recycled).

        With this, when a SessionTracker is notified of a session destroyed event, it
        will decrement its counter of tracked sessions (and remove the cookie from the
        response if the counter has dropped to zero) only if

        • the id of the destroyed session is equal to the session id being
          tracked (existing check), AND
        • the context of the destroyed session is amongst the contexts of the
          sessions being tracked (new check).

        Checking in SessionTracker.java;
        /cvs/glassfish/appserv-webtier/src/java/org/apache/coyote/tomcat5/SessionTracker.java,v
        <-- SessionTracker.java
        new revision: 1.7; previous revision: 1.6
        done

        Show
        jluehe added a comment - Added protection against accidental removal of session cookie from response in very unlikely scenario in which: 1. A servlet in Context A creates a session. The SessionTracker tracks it. The request returns before the session expires. 2. While A's session is still active, a servlet in Context B creates a session. It so happens that the session in B has the same sessionid as the one that was generated in A. It also so happens that the request on which the session is created in B is the same (pooled) request (with the same SessionTracker!) on which the session in A had been created. It also so happens that the session created in A expires before the request returns from B. In this case, the SessionTracker will get notfified of the session expiration, and since the id of the expired session is the same as the session id it is currently tracking, the SessionTracker will decrement its counter of tracked sessions and remove the JSESSIONID cookie from the response if the counter has dropped to zero. The added protection will prevent this from happening, by having a SessionTracker not only keep track of the session id it is tracking, but also of the list of contexts whose sessions it is tracking (this list will be reset when a request and its SessionTracker are recycled). With this, when a SessionTracker is notified of a session destroyed event, it will decrement its counter of tracked sessions (and remove the cookie from the response if the counter has dropped to zero) only if the id of the destroyed session is equal to the session id being tracked (existing check), AND the context of the destroyed session is amongst the contexts of the sessions being tracked (new check). Checking in SessionTracker.java; /cvs/glassfish/appserv-webtier/src/java/org/apache/coyote/tomcat5/SessionTracker.java,v <-- SessionTracker.java new revision: 1.7; previous revision: 1.6 done
        Hide
        jluehe added a comment -

        Incremental fix:
        Make sure that any custom response cookies (those whose cookie names are
        different from JSESSIONID and JSESSIONIDSSO) are preserved in the response, even
        in the event of the session getting invalidated before the response returns.

        Checking in java/org/apache/coyote/Response.java;
        /cvs/glassfish/appserv-webtier/src/java/org/apache/coyote/Response.java,v <--
        Response.java
        new revision: 1.5; previous revision: 1.4
        done
        Checking in java/org/apache/coyote/tomcat5/CoyoteResponse.java;
        /cvs/glassfish/appserv-webtier/src/java/org/apache/coyote/tomcat5/CoyoteResponse.java,v
        <-- CoyoteResponse.java
        new revision: 1.18; previous revision: 1.17
        done
        Checking in java/org/apache/coyote/tomcat5/SessionTracker.java;
        /cvs/glassfish/appserv-webtier/src/java/org/apache/coyote/tomcat5/SessionTracker.java,v
        <-- SessionTracker.java
        new revision: 1.8; previous revision: 1.7
        done
        Checking in java/org/apache/tomcat/util/http/MimeHeaders.java;
        /cvs/glassfish/appserv-webtier/src/java/org/apache/tomcat/util/http/MimeHeaders.java,v
        <-- MimeHeaders.java
        new revision: 1.5; previous revision: 1.4
        done

        Show
        jluehe added a comment - Incremental fix: Make sure that any custom response cookies (those whose cookie names are different from JSESSIONID and JSESSIONIDSSO) are preserved in the response, even in the event of the session getting invalidated before the response returns. Checking in java/org/apache/coyote/Response.java; /cvs/glassfish/appserv-webtier/src/java/org/apache/coyote/Response.java,v <-- Response.java new revision: 1.5; previous revision: 1.4 done Checking in java/org/apache/coyote/tomcat5/CoyoteResponse.java; /cvs/glassfish/appserv-webtier/src/java/org/apache/coyote/tomcat5/CoyoteResponse.java,v <-- CoyoteResponse.java new revision: 1.18; previous revision: 1.17 done Checking in java/org/apache/coyote/tomcat5/SessionTracker.java; /cvs/glassfish/appserv-webtier/src/java/org/apache/coyote/tomcat5/SessionTracker.java,v <-- SessionTracker.java new revision: 1.8; previous revision: 1.7 done Checking in java/org/apache/tomcat/util/http/MimeHeaders.java; /cvs/glassfish/appserv-webtier/src/java/org/apache/tomcat/util/http/MimeHeaders.java,v <-- MimeHeaders.java new revision: 1.5; previous revision: 1.4 done
        Hide
        kmeduri added a comment -

        Merged the fix to PWC12Dev_Branch

        Show
        kmeduri added a comment - Merged the fix to PWC12Dev_Branch
        Hide
        kmeduri added a comment -

        Merged the fix to PWC12Dev_Branch:

        Checking in
        appserv-webtier/src/java/org/apache/coyote/tomcat5/CoyoteResponse.java;
        /cvs/glassfish/appserv-webtier/src/java/org/apache/coyote/tomcat5/CoyoteResponse.java,v

        <-- CoyoteResponse.java
        new revision: 1.13.6.3; previous revision: 1.13.6.2
        done
        Checking in
        appserv-webtier/src/java/org/apache/coyote/tomcat5/SessionTracker.java;
        /cvs/glassfish/appserv-webtier/src/java/org/apache/coyote/tomcat5/SessionTracker.java,v

        <-- SessionTracker.java
        new revision: 1.5.6.2; previous revision: 1.5.6.1
        done
        Checking in
        appserv-webtier/src/java/org/apache/tomcat/util/http/MimeHeaders.java;
        /cvs/glassfish/appserv-webtier/src/java/org/apache/tomcat/util/http/MimeHeaders.java,v

        <-- MimeHeaders.java
        new revision: 1.2.16.2; previous revision: 1.2.16.1
        done
        Checking in appserv-webtier/src/java/org/apache/coyote/Response.java;
        /cvs/glassfish/appserv-webtier/src/java/org/apache/coyote/Response.java,v
        <-- Response.java
        new revision: 1.4.6.1; previous revision: 1.4
        done

        Show
        kmeduri added a comment - Merged the fix to PWC12Dev_Branch: Checking in appserv-webtier/src/java/org/apache/coyote/tomcat5/CoyoteResponse.java; /cvs/glassfish/appserv-webtier/src/java/org/apache/coyote/tomcat5/CoyoteResponse.java,v <-- CoyoteResponse.java new revision: 1.13.6.3; previous revision: 1.13.6.2 done Checking in appserv-webtier/src/java/org/apache/coyote/tomcat5/SessionTracker.java; /cvs/glassfish/appserv-webtier/src/java/org/apache/coyote/tomcat5/SessionTracker.java,v <-- SessionTracker.java new revision: 1.5.6.2; previous revision: 1.5.6.1 done Checking in appserv-webtier/src/java/org/apache/tomcat/util/http/MimeHeaders.java; /cvs/glassfish/appserv-webtier/src/java/org/apache/tomcat/util/http/MimeHeaders.java,v <-- MimeHeaders.java new revision: 1.2.16.2; previous revision: 1.2.16.1 done Checking in appserv-webtier/src/java/org/apache/coyote/Response.java; /cvs/glassfish/appserv-webtier/src/java/org/apache/coyote/Response.java,v <-- Response.java new revision: 1.4.6.1; previous revision: 1.4 done

          People

          • Assignee:
            jluehe
            Reporter:
            jluehe
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: