servlet icon indicating copy to clipboard operation
servlet copied to clipboard

Clarify Behaviour of HttpServletRequest.getServletPath()

Open Pandrex247 opened this issue 5 years ago • 12 comments

The Javadoc states that this method should return the path of the servlet being called, as specified in the request URL.

GlassFish & WildFly give differing behaviour on this line in the JavaEE7 Samples repo.

Namely, what is the expected behaviour of the getServletPath() method when called from a HttpServletRequest injected into a RequestScoped Bean, which has been injected into a servlet that has been forwarded to?

GlassFish returns the original servlet, WildFly returns the forwarded-to servlet. The sample indicates that it should be the forwarded-to servlet, but this seems like it contradicts the Javadoc and/or simply depends on when the Bean is instantiated.

Relevant classes from sample:

Pandrex247 avatar Sep 19 '19 10:09 Pandrex247

Hi Andrew!

To the best of my knowledge this is simply not well defined. It was discussed before if I'm not mistaken what request the injected one corresponds to anyway, as requests can be wrapped as well.

Is it the the first request as it enters the system? Is it the request at the point of invocation (e.g. in a Filter, the current one as passed in by the previous Filter), the request as it arrived in the original Servlet, or indeed, the one as it arrives in the forwarded Servlet.

Back then I proposed adding some qualifiers to make this cleared, but no action was taken upon it.

CC @markt-asf @stuartwdouglas

arjantijms avatar Sep 19 '19 11:09 arjantijms

p.s. part of the issue is that injection of the HttpServletRequest is defined in CDI, but it's actually Servlet that owns the type. Just as JTA, which is otherwise not dependent on CDI, delivers @Transactional etc, I guess Servlet should deliver the producers for HttpServletRequest and other Servlet types.

arjantijms avatar Sep 19 '19 11:09 arjantijms

I need to first make clear that I am not at all familiar with the CDI spec.

Based purely on my understanding of the Servlet spec my expectation is that when myBean.getServletPath() is called within ForwardedServlet.doGet() the result is the same as if HttpServletRequest.getServletPath() had been called on the HttpServletRequest object passed to ForwardedServlet.doGet().

To put it another way, my expectation is that CDI passes the request to the bean by reference, not by value. I've just quickly scanned through the CDI spec and don;t see anything to support the idea of passing by value, only by reference.

To get to the original question, I'd expect the the path of the forwarded servlet to be returned.

markt-asf avatar Sep 19 '19 13:09 markt-asf

I think that in this regard the servlet spec is pretty clear. From RequestDispatcher.forward:

For a RequestDispatcher obtained via getRequestDispatcher(), the ServletRequest object has its path elements and parameters adjusted to match the path of the target resource.

I think this is clear that during a request forward, the instance of the request object is altered to retur nn the forwarded servlet path. Any component acting on that request as a result of that forward (or at least within the scope of that forward) should see that servlet path.

So I'm not so sure this is an issue for the Servlt spec and more of a CDI spec issue. So I think we should close this issue here?

gregw avatar Sep 22 '19 22:09 gregw

Best would probably be for the Servlet spec to ship a CDI build-in Bean with qualifiers that specifies which HttpServletRequest is obtained at which points.

Such precise specifications would be completely out of scope in the CDI spec itself, but would be suitable for the Servlet spec. The "old" unqualified version can then stay at CDI for unfortunate, but historical reasons.

arjantijms avatar Sep 22 '19 23:09 arjantijms

@arjantijms I'm really not keen on servlets having a CDI dependency. Surely it is whoever specifies javax.enterprise.context.RequestScoped annotation that should clarify this and provide any built-in default beans.

gregw avatar Sep 22 '19 23:09 gregw

I'm also not a fan of adding a CDI dependency to the Servlet spec.

Reading through the CDI spec again my reading of section 5.4 is that the expected behaviour here is well-defined and that it is consistent with the behaviour both Greg and I expect (i.e. the WildFly behaviour).

I agree with Greg that, if the view is that the behaviour is not well-defined, then that is an issue for the CDI spec and that this issue should be closed.

markt-asf avatar Sep 23 '19 08:09 markt-asf

I'm also not a fan of adding a CDI dependency to the Servlet spec.

It's not the rest of Servlet depending on it, if that's what you mean. JTA for instance also ships a CDI build-in bean, but JTA itself is still the (very) low level API that doesn't itself depend on CDI in any way.

CDI itself should not be defining Servlet behaviour, it has been a historical mistake to put the HttpServletRequest build-in bean in CDI.

It's essentially more of a method to describe how a CDI product gets hold of the current HttpServletRequest. If Servlet is used in a product that doesn't want or can't integrate with CDI, then this method is simply not used.

Specifically, Tomcat, for instance, can just not ship this one class, or, make this class available in a separate jar that people who wish to use CDI on Tomcat can include. Or, it can even be implemented by, say, Weld and not by Tomcat at all.

It's a little like the integration of making @Inject inside Servlets work.

arjantijms avatar Sep 23 '19 10:09 arjantijms

Surely it is whoever specifies javax.enterprise.context.RequestScoped annotation that should clarify this and provide any built-in default beans.

This has very little to almost nothing to do with the javax.enterprise.context.RequestScoped annotation, which is defined in a general way and doesn't just represent an http servlet request.

A scope is something completely different from a build-in bean.

arjantijms avatar Sep 23 '19 10:09 arjantijms

I think CDIs exact place/role in the EE ecosystem still needs to be resolved - and that wont happen within the servlet spec nor without architectural vision and guidance.

If CDI becomes a foundation technology in future EE releases, then I can see Servlets leaning on it a lot more, but for now adhocery is just going to make a better solution more difficult in future.

gregw avatar Sep 23 '19 23:09 gregw

I think CDIs exact place/role in the EE ecosystem still needs to be resolved

I'm not sure why you think it needs to be resolved tbh. The way forward seems pretty clear.

Jakarta Faces has been re-basing on CDI, and so will Jakarta REST (JAX-RS). JTA has already provided the example in Java EE 7. Jakarta Security has been build from the ground up based on CDI. The derived MicroProfile specs are all CDI based.

I can see Servlets leaning on it a lot more, but for now adhocery is just going to make a better solution more difficult in future.

In this case, Servlets don't have to lean on CDI at all. The only thing that's being asked is that the build-in bean (which is, again, different from a scope), is provided by Servlet.

This is a standalone piece of integration code where the rest of the Servlet APIs have no knowledge about. It's to be implemented by Servlet containers that want to support CDI, and it delivers the HttpServletRequest. That's all.

It's not adhocery, and there's not going to be any better solution, as this is pretty much the only way to obtain an HttpServletRequest in CDI. Many other specs already use this exact same method, so how can it be adhocery then?

arjantijms avatar Sep 25 '19 12:09 arjantijms

@arjantijms some EE projects re-basing on CDI while others wait for architectural vision/guidance is exactly what I mean by adhocery.

I do think EE should indeed be based on DI - but currently there are lots of issues - is CDI a container provided service? Do applications provide their own CDI and it has to integrate with containers? If the container is assembled with CDI, how does that interact with the application CDI?

Servlets have addressed the problem by ignoring it. I don't think this is a good thing to do and one of my key objectives for a future Servlet release is to make it properly CDI aware and to resolve the questions above. So yes, it would be good to have standard way that CDI can get hold of a request, but we also need a standard way it can get Listeners, Filters, Serlvets etc. and I think adding a partial solution will be of little help.

Eitherway, I think perhaps we should let the code do the talking. By all means prepare a PR with such a Bean for request access. We can then see if that approach can be extended to Listeners, Filters, Serlvets etc. and we can consider when such a PR can be merged and hopefully spare us all from multiple CDI integrations.

gregw avatar Sep 25 '19 23:09 gregw

The discussion diverged from the original question, which I believe has been answered.

markt-asf avatar Mar 22 '24 16:03 markt-asf