Community
Participate
Working Groups
When registering servlets with the HttpService, all registered servlets share the same ServletContext, even when they are registered with different HttpContexts. It would be nice if the Equinox implementation could return different ServletContexts for different HttpContexts, as Jetty does [1]. This would allow to isolate different web applications within one OSGi container. Example: HttpContext httpContext1 = httpService.createDefaultHttpContext(); httpService.registerServlet( "/foo", servlet1, null, httpContext1 ); HttpContext httpContext2 = httpService.createDefaultHttpContext(); httpService.registerServlet( "/bar", servlet2, null, httpContext2 ); Running this code in Equinox, servlet1.getServletContext() == servlet2.getServletContext(). Trying the same with Apache Karaf 2.2.9 using PAX Web, both servlet contexts are different, and consequently, both servlets also get different HttpSessions. [1] http://felix.apache.org/site/apache-felix-http-service.html#ApacheFelixHTTPService-ServletContextNotes
Your message in the equinox forum provides a bit more information: http://www.eclipse.org/forums/index.php/t/383351/ (In reply to comment #0) > Running this code in Equinox, servlet1.getServletContext() == > servlet2.getServletContext(). Trying the same with Apache Karaf 2.2.9 using > PAX Web, both servlet contexts are different, and consequently, both > servlets also get different HttpSessions. > You mention servlet1.getServletContext() == servlet2.getServletContext() here but in the forum message you mention the hashCode() values being equal. Are they really the identical objects (using identity ==) or do they have the same hashCode and perhaps return true from the equals() method? In our implementation of the ServletContext it does not appear that we overwrite the hashCode or equals methods and instead these methods would simply delegate up to the container's SerlvetContext implementation making it appear that the two ServletContext objects are identical when they really are not.
(In reply to comment #1) > Your message in the equinox forum provides a bit more information: I skipped these details because I thought there were distracting. > You mention servlet1.getServletContext() == servlet2.getServletContext() > here but in the forum message you mention the hashCode() values being equal. > Are they really the identical objects (using identity ==) or do they have > the same hashCode and perhaps return true from the equals() method? In a first test I printed out the hashCode because the ServletContext's toString() output doesn't include a hash. But I also made sure that they are really identical by comparing the references (==).
(In reply to comment #2) > In a first test I printed out the hashCode because the ServletContext's > toString() output doesn't include a hash. But I also made sure that they are > really identical by comparing the references (==). In my tests I did not find the ServletContext objects to be idetical (==). Is this in a servlet bridge environment or just a plain equinox framework with the jetty bundles installed?
Created attachment 221496 [details] example project (In reply to comment #3) > (In reply to comment #2) > > In a first test I printed out the hashCode because the ServletContext's > > toString() output doesn't include a hash. But I also made sure that they are > > really identical by comparing the references (==). > > In my tests I did not find the ServletContext objects to be idetical (==). > Is this in a servlet bridge environment or just a plain equinox framework > with the jetty bundles installed? That's interesting. I use plain equinox with jetty: id State Bundle 0 ACTIVE org.eclipse.osgi_3.8.0.v20120529-1548 1 ACTIVE org.eclipse.jetty.security_8.1.3.v20120522 2 ACTIVE org.eclipse.jetty.continuation_8.1.3.v20120522 3 ACTIVE org.apache.felix.gogo.shell_0.8.0.v201110170705 4 ACTIVE org.eclipse.jetty.util_8.1.3.v20120522 5 ACTIVE org.eclipse.equinox.http.jetty_3.0.0.v20120522-1841 6 ACTIVE org.apache.felix.gogo.runtime_0.8.0.v201108120515 7 ACTIVE org.eclipse.jetty.servlet_8.1.3.v20120522 8 ACTIVE org.eclipse.jetty.io_8.1.3.v20120522 9 ACTIVE org.eclipse.jetty.server_8.1.3.v20120522 10 ACTIVE org.apache.felix.gogo.command_0.8.0.v201108120515 11 ACTIVE test.servletcontext_1.0.0.qualifier 12 ACTIVE org.eclipse.equinox.console_1.0.0.v20120522-1841 13 ACTIVE org.eclipse.jetty.http_8.1.3.v20120522 14 ACTIVE org.eclipse.equinox.http.servlet_1.1.300.v20120522-1841 15 ACTIVE javax.servlet_3.0.0.v201112011016 16 ACTIVE org.eclipse.osgi.services_3.3.100.v20120522-1822 I've also attached my example project.
(In reply to comment #4) > That's interesting. I use plain equinox with jetty: > Thanks Ralf. I agree the hashcode values of the two ServletContext objects are equal. That is a bug we should fix (also the equals method is needs to be fixed since it currently returns false even for the same ServletContext object! But the two ServletContext are not == Add the following lines to your test: System.out.println( "ServletContexts ==: " + ( servlet1.getServletContext() == servlet2.getServletContext() ) ); System.out.println( "ServletContext1 equals ServletContext1: " + ( servlet1.getServletContext().equals(servlet1.getServletContext() ))); System.out.println( "ServletContext1 equals ServletContext2: " + ( servlet1.getServletContext().equals(servlet1.getServletContext() ))); So assuming we fix the hashcode and equals methods would that solve your issue? It is unclear to me if it would.
(In reply to comment #5) > Add the following lines to your test: > > > System.out.println( "ServletContexts ==: " > + ( servlet1.getServletContext() == servlet2.getServletContext() ) ); > System.out.println( "ServletContext1 equals ServletContext1: " > + ( servlet1.getServletContext().equals(servlet1.getServletContext() > ))); > System.out.println( "ServletContext1 equals ServletContext2: " > + ( servlet1.getServletContext().equals(servlet1.getServletContext() ^ I had a typo here. that should really be serlvet2 NOT servlet1. > ))); > At any rate I get the following output with this test: HttpContext: false ServletContexts hashcode equals: true ServletContexts ==: false ServletContext1 equals ServletContext1: false ServletContext1 equals ServletContext2: false
(In reply to comment #5) > So assuming we fix the hashcode and equals methods would that solve your > issue? It is unclear to me if it would. Although they will be two different ServletContext objects they still wrap the same actual Jetty ServletContext. Thus, they also share the same HttpSession. I think the request is more about having different contexts underneath so that you also get different HttpSession objects (isolation of the HttpSession).
Now I see they aren't identical. I forgot to remove the .hashCode() in my example and thought I was comparing the real objects. Sorry for that. Now I also checked that both servlet contexts maintain separate attribute stores, which is good. > So assuming we fix the hashcode and equals methods would that solve your > issue? It is unclear to me if it would. I don't think it would, because we obtain the servlet context from the http session, using request.getSession().getServletContext(). Since both servlets share the same HttpSession (don't they?), I'm afraid it's undefined which servlet context we get?
I think this is more about enhancing our HttpSessionAdaptor, but I must admit I don't know the implications of changing that to be HttpContext specific. http://git.eclipse.org/c/equinox/rt.equinox.bundles.git/tree/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/HttpSessionAdaptor.java?h=master I'm going to need help to make progress here.
I was wrong again. The HttpSession is also wrapped, the requests will return different instances of HttpSession, but with the same session ID and the same attribute store, because they are backed by the same HttpSession. Now I'm getting to the root of our problem. We basically store a reference in the HttpSession to something that is stored in the ServletContext. In a the other context, this reference in the session is not valid anymore. It seems that the following assertion of the HttpSession contract is not valid in Equinox: Session information is scoped only to the current web application (ServletContext), so information stored in one context will not be directly visible in another.
(In reply to comment #10) > It seems that the following assertion of the HttpSession contract is not > valid in Equinox: > > Session information is scoped only to the current web application > (ServletContext), so information stored in one context will not > be directly visible in another. Right, so I am wondering if it is valid to somehow store that information in a way that is HttpContext specific in our HttpSessionAdaptor.
(In reply to comment #11) > Right, so I am wondering if it is valid to somehow store that information in > a way that is HttpContext specific in our HttpSessionAdaptor. That would fix our problem, and sounds like a good idea to me, but I'm also not sure about the implications. There may be code that initializes a session when isNew() returns true and relies on the existence of certain attributes when isNew() is false - but I'm sure if this is really an issue...
(In reply to comment #12) > That would fix our problem, and sounds like a good idea to me, but I'm also > not sure about the implications. Oh, there may also be code that relies on the current behavior, i.e. two servlets registered with different HttpContext each (for security reasons) but which share common data in the session (eg. user authentication info).
(In reply to comment #13) > (In reply to comment #12) > > That would fix our problem, and sounds like a good idea to me, but I'm also > > not sure about the implications. > > Oh, there may also be code that relies on the current behavior, i.e. two > servlets registered with different HttpContext each (for security reasons) > but which share common data in the session (eg. user authentication info). True, but I'm not sure we should keep the default behavior, it seems more like a bug to me than the intended design. If we decide to add this enhancement perhaps we should have a flag to disable it, but I would be for enabling it by default.
Changing title to reflect the current discussion.
Great! I'm willing to craft a patch over the next few weeks. Just have to align it with the upcoming conference travel.
Bizarre as it might sounds I think we're going to have to do the logic in the HttpSessionAdaptor instead of asking Jetty for multiple new ServletContexts in order to have consistent behavior with the ServletBridge. But apart from that ... go for it as this indeed does sound correct to me. Please add/flag me as a reviewer when the time comes.
Is Gerrit available for Equinox? Or do you want a classic patch?
So it seems the easiest way is to simply "namespace" the session attributes, i.e. the HttpSessionAdaptor will prefex each attributes using the same method that ProxyContext uses ("hc_" + httpContext.hashCode()). This avoids tracking sessions ourselves and leaves session handling (expiration, etc.) and fail-over logic to the container. However, we still have: getCreationTime invalidate isNew setMaxInactiveInterval I'm not much concerned about getCreationTime() and isNew(). I'm more concerned about invalidate() and setMaxInactiveInterval(). Those are shared between contexts and a servlet invalidating a session will also invalidate it for all others sharing the same session. Note, Servlet/Filter registrations without a HttpContext do get a new default context on each registration. Thus, they will never share any session data after the change. Is this ok or should registrations with a 'null' context of the same bundle share the same context? The contract states: "null if a default HttpContext is to be created and used". It can be read both ways, i.e. to be created but not necessarily for every call.
I started with a test case: https://github.com/eclipseguru/rt.equinox.bundles/commits/gunnar/bug390381
Branch updated with commit to scope session attributes to HttpContext. Simon, you want a pull request on GitHub? :)
One minor questions: should I use "httpContext.hashCode()" for scoping or "System.identidyHashCode(httpContext)"?
Can this fix also go into Juno SR2?
(In reply to comment #20) > So it seems the easiest way is to simply "namespace" the session attributes, > i.e. the HttpSessionAdaptor will prefex each attributes using the same > method that ProxyContext uses ("hc_" + httpContext.hashCode()). ProxyContext uses a Map<HttpContext, Dictionary> where the Dictionary provides the attributes of a ServletContext for a specific HttpContext. Is there a reason you did not use that approach here? Is there an advantage to doing this prefix mapping and storing all the attributes for all the HttpContext in the same Map > This avoids > tracking sessions ourselves and leaves session handling (expiration, etc.) > and fail-over logic to the container. > > However, we still have: > getCreationTime > invalidate > isNew > setMaxInactiveInterval > > I'm not much concerned about getCreationTime() and isNew(). I'm more > concerned about invalidate() and setMaxInactiveInterval(). Those are shared > between contexts and a servlet invalidating a session will also invalidate > it for all others sharing the same session. I don't have a good feeling on this one. As a first pass could we ignore this issue? Otherwise it seems we would have to start tracking each session validity ourselves, but I am not sure how we could go about invalidating just a single session and keeping the others valid since we all know we are sharing the same actual session impl from the container. I'm definitely not an expert here. > > Note, Servlet/Filter registrations without a HttpContext do get a new > default context on each registration. Thus, they will never share any > session data after the change. Is this ok or should registrations with a > 'null' context of the same bundle share the same context? The contract > states: "null if a default HttpContext is to be created and used". It can be > read both ways, i.e. to be created but not necessarily for every call. Could we change our DefaultHttpContext impl to overwrite equals and hashCode such that two DefaultHttpContext for the same bundle are equal? (In reply to comment #21) > I started with a test case: > https://github.com/eclipseguru/rt.equinox.bundles/commits/gunnar/bug390381 Thanks Gunnar! (In reply to comment #23) > One minor questions: should I use "httpContext.hashCode()" for scoping or > "System.identidyHashCode(httpContext)"? I think I would prefer to use the Map<HttpContext,Hashtable<String,Object>> approach instead to track the session attributes. Suggesting Hashtable to avoid Enumeration implementation, but you could easily use a Map and use: Collections.enumeration(map.keySet()) OK, I just wrote all this up questioning why you were using the prefix approach, and I think I know why. Is it because you need to actually store the values in the container's implementation of session so that they are "persisted" across servlet requests? If so I am sorry for the noise, but I thought it better to record my thought process while reviewing your work. Question: are HttpSessions pre-populated with attributes from the container when they are created? If so do we need to let these pre-populated values bleed through to the HttpContext specific sessions?
(In reply to comment #24) > Can this fix also go into Juno SR2? I think we should wait until we have the proper solution in hand so that we can properly assess the risk. I am open to the idea though.
(In reply to comment #25) > OK, I just wrote all this up questioning why you were using the prefix > approach, and I think I know why. Is it because you need to actually store > the values in the container's implementation of session so that they are > "persisted" across servlet requests? If so I am sorry for the noise, but I > thought it better to record my thought process while reviewing your work. Yes. The relation HttpContext (or ProxyContext) to HttpSession is N:M. There will be many sessions for each context. The container (eg., Jetty) assigns and maintains session for each connected client (eg., browser). Sessions are bound to requests using cookies or request parameters. Remember the memory leak in the help center? This was because sessions were never expired by Jetty (because of some default setting) and they grew to millions. > Question: are HttpSessions pre-populated with attributes from the container > when they are created? If so do we need to let these pre-populated values > bleed through to the HttpContext specific sessions? A container may be configured to persist sessions into a database (or some other store) or it may just replicate the sessions to other nodes in a cluster. Thus, session data will be serialized and may be restored at a later point in time and/or on a different container node in the cluster. Using the container session as backing store for session attributes allows the HttpService to participate in the logic and capabilities offered by containers. Bahhh, wait a second, I just figured that session replication won't work reliable. Hash codes will be different across JVMs. D'oh! Just thinking out load ... I could define an extension to the HttpContext interface which allows to define a custom prefix.
(In reply to comment #27) > Just thinking out load ... I could define an extension to the HttpContext > interface which allows to define a custom prefix. s/load/loud
re: Github pull request -- that's great for me for reviewing as I can do it in Orion. Not sure that's so great for the actual commit though. re: session serialization *sigh* that does indeed sound tricky and I suspect context attributes also need some love. I seem to recall String hashcodes are safe across VMs because they're spec'ed so if we can get to a string as a stable representation...
Simon, you need to fork it so that I can send you a pull request. I also pushed a proposal for customizable session namespaces. The workflow is afaik: You pull my commits, and review them and you commit them into the Equinox repo with you committer id and push that.
(In reply to comment #30) > Simon, you need to fork it so that I can send you a pull request. I also > pushed a proposal for customizable session namespaces. The idea seems reasonable to me: - Should HttpContextWithSessionNamespace be a marker interface instead of extending HttpContext? Perhaps this concept may apply to other areas of the container? - Not sure I like the term Namespace, only because we have the ill-named NamespaceException in HttpService API (I think that exception should have been called AliasException). Perhaps HttpContextIdentity with a method getIdentity() - Should we consider having DefaultHttpContext implement HttpContextIdentity and have the identity be some string that includes the Bundle ID for the HttpContext? > > The workflow is afaik: You pull my commits, and review them and you commit > them into the Equinox repo with you committer id and push that. See the following for the complete flow: http://wiki.eclipse.org/Development_Resources/Handling_Git_Contributions#Git
(In reply to comment #31) > - Should HttpContextWithSessionNamespace be a marker interface instead of > extending HttpContext? Perhaps this concept may apply to other areas of the > container? I've no strong opinion But it is coupled to the HttpContext. > - Not sure I like the term Namespace, only because we have the ill-named > NamespaceException in HttpService API (I think that exception should have > been called AliasException). Perhaps HttpContextIdentity with a method > getIdentity() Technically it's a prefix. getSessionAttributePrefix would be better than getId. I just feel the term id is not precise enough and also overloaded. > - Should we consider having DefaultHttpContext implement HttpContextIdentity > and have the identity be some string that includes the Bundle ID for the > HttpContext? Yepp, likely. I think another candidate is the registry bundle. I have to check, but I believe it already defines ids for registered contexts.
Hi, could someone inform me what is the status of this improvement?
(In reply to comment #33) > Hi, could someone inform me what is the status of this improvement? I'm currently waiting on an an update to the HttpService spec. I talked to the spec lead at ECE and it seems that the Felix behavior is not intended by the spec. http://www.mail-archive.com/osgi-dev@mail.osgi.org/msg02500.html
Thanks for the link, Gunnar. Felix Meschberger is going to present the new spec at EclipseCon 2013 [1]. That sounds like a good opportunity to meet and discuss this issue. [1] http://www.eclipsecon.org/2013/sessions/whats-new-http-service-specification
Considerable work has taken place on equinox's http.servlet to implement RFC 189 and the Http Whiteboard draft spec. I believe the issues described here should be cleared up. Could someone be willing to test it? Futher, a test case would be appreciated.
