You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@tomcat.apache.org by isapir <gi...@git.apache.org> on 2017/10/05 17:50:05 UTC

[GitHub] tomcat pull request #76: added SessionInitializerFilter

GitHub user isapir opened a pull request:

    https://github.com/apache/tomcat/pull/76

    added SessionInitializerFilter

    This Filter initializes the HttpSession object for the Request, which is required for certain operations with WebSocket requests where it is too late to initialize the HttpSession, but the HttpSession object is required to retrieve information during the Handshake process.
    
    One common use case is to retrieve the `ServletContext` by calling `handshakeRequest.getHttpSession.getServletContext()`.  According to the WebSocket specification a call to getHttpSession() should not initialize one if one does not exist.  If the HttpSession is not initalized then this call throws an NPE.
    
    Hopefully the WebSocket EG will offer a better way in the future, but until such time the HttpSession needs to be initialized by a Filter like this one,or a Listener which will run for all URIs and is therefore less optimal.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/isapir/tomcat update-03

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/tomcat/pull/76.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #76
    
----
commit 7f155de0d026b0254b973646e88dde70bb1f8ab6
Author: Igal Sapir <de...@21solutions.net>
Date:   2017-10-05T17:49:19Z

    added SessionInitializerFilter
    
    This Filter initializes the HttpSession object for the Request, which is required for certain operations with WebSocket requests where it is too late to initialize the HttpSession, but the HttpSession object is required to retrieve information during the Handshake process.
    
    One common use case is to retrieve the `ServletContext` by calling `handshakeRequest.getHttpSession.getServletContext()`.  According to the WebSocket specification a call to getHttpSession() should not initialize one if one does not exist.  If the HttpSession is not initalized then this call throws an NPE.
    
    Hopefully the WebSocket EG will offer a better way in the future, but until such time the HttpSession needs to be initialized by a Filter like this one,or a Listener which will run for all URIs and is therefore less optimal.

----


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

[GitHub] tomcat issue #76: added SessionInitializerFilter

Posted by isapir <gi...@git.apache.org>.

Github user isapir commented on the issue:

    https://github.com/apache/tomcat/pull/76
  
    I updated accordingly to save a few CPU cycles and a few bytes of memory for each Request that hits the Filter.  
    
    Cumulative change can be seen at https://github.com/apache/tomcat/pull/76/files?w=1



---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

[GitHub] tomcat issue #76: added SessionInitializerFilter

Posted by isapir <gi...@git.apache.org>.

Github user isapir commented on the issue:

    https://github.com/apache/tomcat/pull/76
  
    Sure, I will add the documentation.
    
    Thanks :)


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

[GitHub] tomcat issue #76: added SessionInitializerFilter

Posted by markt-asf <gi...@git.apache.org>.

Github user markt-asf commented on the issue:

    https://github.com/apache/tomcat/pull/76
  
    I did wonder if this would be better as configuration on WsFilter but on reflection a separate Filter looks to be simpler for users to configure.
    
    The patch needs documentation (webapps/docs/config/filter.xml)
    
    No need to patches for the back-ports. We'll use svn merge and add NO-OP methods as necessary.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

[GitHub] tomcat issue #76: added SessionInitializerFilter

Posted by ChristopherSchultz <gi...@git.apache.org>.

Github user ChristopherSchultz commented on the issue:

    https://github.com/apache/tomcat/pull/76
  
    @markt-asf re: `Filter` versus configuration
    
    This could be something that we add now just as a `Filter` but in the future add a configuration option that just automatically attaches the `Filter` -- just like how using `<auth-method>` in `WEB-INF/web.xml` auto-attaches the appropriate `Valve` to the valve chain.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

[GitHub] tomcat issue #76: added SessionInitializerFilter

Posted by markt-asf <gi...@git.apache.org>.

Github user markt-asf commented on the issue:

    https://github.com/apache/tomcat/pull/76
  
    Looking at this is on my TODO list to look at before the next release


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

[GitHub] tomcat issue #76: added SessionInitializerFilter

Posted by kkolinko <gi...@git.apache.org>.

Github user kkolinko commented on the issue:

https://github.com/apache/tomcat/pull/76

1. Create an entry in Bugzilla and reference this PR, so that this is correctly mentioned in change log.
http://tomcat.apache.org/bugreport.html

2. I do not mind this utility filter being added to Tomcat. It is small and there might be uses for it.
It is the same as the default of "<%@page session="true"%>" in JSPs.

But I think that its use for WebSocket needs some additional explanation / discussion, and maybe a different fix.

> "too late to initialize the HttpSession object"

The HttpSession is a tool that is used to share some information between requests. It is only useful if there is a subsequent request from the same client that is associated with the same session.

If the session is identified via a cookie (there are also different configuration options), it means that the client should receive a Set-Cookie header with a HTTP response and process it (in "101 Switching Protocols" HTTP response).

I have not tested whether the Set-Cookie header in a "101 Switching Protocols" response is recognized by actual clients.

> handshakeRequest.getHttpSession().getServletContext()

The above code is odd. You are not sharing some information, but are accessing some static configuration object. There should be an easier way to obtain a ServletContext. It is not what a session is needed for.

---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

[GitHub] tomcat issue #76: added SessionInitializerFilter

Posted by isapir <gi...@git.apache.org>.

Github user isapir commented on the issue:

    https://github.com/apache/tomcat/pull/76
  
    @markt-asf I added the documentation as per your feedback


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org