Guice 2 SNAPSHOT: migrating from web.xml to ServletModule/GuiceFilter

[tchan]

Hi;

I'm playing around with the guice-20090205 snapshot build and am
having problems getting jsps to work when using the GuiceFilter
approach to map servlet requests.

I have a servlet that forwards to a jsp like this:

req.getRequestDispatcher( "/WEB-INF/jsp/test.jsp" ).forward( req,
res );

If I hook up this servlet in web.xml using a servlet mapping, it works
as expected. When I take out the servlet mapping, put in a /* filter
mapping for GuiceFilter and add a listener definition for my own
implementation of GuiceServletContextListener, my servlet container
will fail to find the referenced jsp and return a 404 instead.

Before with web.xml:
<servlet-mapping>
<servlet-name>TestServlet</servlet-name>
<url-pattern>/test</url-pattern>
</servlet-mapping>

After with ServletModule in my context listener class:
@Override protected void configureServlets()
{
bind( TestServlet.class ).asEagerSingleton();
serve( "/test" ).with( TestServlet.class );
}

My servlet code does get called and I can see that the call to
getRequestDispatcher() goes into ManagedFilterPipeline.withDispatcher
() in an attempt to find a Guice controlled servlet before delegating
upwards to my servlet container. And this is the same code path using
either the web.xml or ServletModule approach, I just can't figure out
why the latter would fail to find/load the jsp file.

Has anyone else seen this or have been able to get jsps working with
Guice? I am using Tomcat 6.0.18 on OSX with jre 1.6 if it matters.

[Dhanji R. Prasanna]

yea this is a known problem, I just have not had any time to address it =(

If you would like to contribute a patch we would be thrilled! Otherwise I hope to get to this problem soon.

Dhanji.

[tchan]

I don`t mind taking a stab at ii if you can point me in the right
direction. I tried debugging through Tomcat`s pipeline but that was
too much for me (and I really couldn`t see a difference between the
web.xml and GuiceFilter scenarios).

[tchan]

All right, I took a stab at it and managed to find the problem but I
don't understand it well enough to fix it.

In Tomcat, the forward to the referenced jsp will eventually be
serviced by the Jasper jsp servlet. When the target jsp is defined in
web.xml, it can just grab its path from that servlet definition entry
otherwise it has to be a request from RequestDispatcher. So if it's
an include, then there's a request attribute with the path already
otherwise for a forward, Jasper will call request.getServletPath() and
append getPathInfo() on it as needed. This is where the problem is.

The request object here is a Guice wrapper object, specifically the
anonymous HttpServletRequestWrapper class inside
ServletDefinition.doService(). Its getServlePath() defers to
computePath() to figure out the servlet path and it *seems* to return
the servlet mapping of the original servlet rather than the servlet
path of the jsp forward. computePath() does defer to its super
(Tomcat) request object if it can't compute it itself and that wrapped
request object returns the right servlet path.

So in short, the request wrapper in ServletDefinition has a bug that
trips jsp forwards in Tomcat. But I don't understand why it does what
it tries to do so I'll have to leave it to Dhanji to take a look at
it.

[Dhanji R. Prasanna]

Yea you're right that that is basically the problem. I'll look into this when I have some time later in the month. Any other volunteers?

Dhanji.

[Alen Vrecko]

I gave it a quick go (trunk build) and it to works with jsp out of the
box but it is not perfect(calling include works but calling forward
crashes).

IIRC There are 2 different ways of getting the RequestDispatcher

o) From ServletContext in that case you specify the whole path
context.getRequestDispatcher("/fooServlet/whatever.html") it begins
with / i.e. the root
o) From ServletRequest in that case you specify only the part that
comes after the servlet since the request knows for which servlet it
belongs e.g. request.getRequestDispatcher("whatever.html"). Note the "/
fooServlet/" is figured out from the request.

You're doing req.getRequestDispatcher( "/WEB-INF/jsp/test.jsp"). I
could be wrong but you are doing the request way your request is
actually for /<servlet>//WEB-INF/jsp/test.jsp beside I think that
request for WEB-INF/* should return 404 in any case. WEB-INF is
private property.

You can try @Inject ServletContext ctx; and doing
ctx.getRequestDispatcher("/jsp/test.jsp") or properly use request way.

I noticed

serve( "/test" ).with( TestServlet.class );

If you try /test in the browser it will return 404 since the browser
actually requests /test/ notice the trailing /. Try

serve( "/test/" ).with( TestServlet.class )

Dhanji maybe "/test" and "/test/" should be equivalent in serve
definition? At least I noticed that trailing / makes a big difference.

Anyway. Oddly enough

requestDispatcher = either from context (long path) or request (short
path)

requestDispatcher.forward(httpServletRequest, httpServletResponse); //
this produces SIOOBE WTF?
requestDispatcher.include(httpServletRequest, httpServletResponse); //
works browser returns the correct response

Cheers,
Alen

[tchan]

Hey, thanks for the reply.

> I gave it a quick go (trunk build) and it to works with jsp out of the
> box but it is not perfect(calling include works but calling forward
> crashes).

I am just using the snapshot build.

> You're doing req.getRequestDispatcher( "/WEB-INF/jsp/test.jsp"). I
> could be wrong but you are doing the request way your request is
> actually for /<servlet>//WEB-INF/jsp/test.jsp beside I think that
> request for WEB-INF/* should return 404 in any case. WEB-INF is
> private property.

From my understanding, if I am using the dispatcher from
ServletRequest, forwards and includes will be relative to the servlet
path unless preceded by a slash. In that case, it's relative to
context root, i.e. WebContent. At least this is how it works in
Tomcat and WebSphere.

I tried using the RequestDispatcher from ServletContext without Guice
(straight web.xml mapping) on Tomcat and it looks like it actually
wants all paths to start with a slash, it too serves relative to
context root.

WEB-INF is special in that it can't be access directly from the client
side but you're allowed to access it internally via a request
dispatcher.

> You can try @Inject ServletContext ctx; and doing
> ctx.getRequestDispatcher("/jsp/test.jsp") or properly use request way.

I experimented with getServletContext().getRequestDispatcher() with
the Guice servlet adapter and it too doesn't work with forwards but
it's fine with include. So the request dispatcher doesn't seem to
make a difference and stepping through the debug showed it was the
same problem with the servlet path reported by the server request
wrapper.

> I noticed
>
> serve( "/test" ).with( TestServlet.class );
>
> If you try /test in the browser it will return 404 since the browser
> actually requests /test/ notice the trailing /. Try
>
> serve( "/test/" ).with( TestServlet.class )

I don't think that's the case. When I tried /test in my browser, it
did make a request for /test on the servlet side, sans final slash.

[Alen Vrecko]

> Hey, thanks for the reply.

Hi. You to. I'll learn from this.

> I am just using the snapshot build.

I don't think there is any difference with regards to servlets.

> From my understanding, if I am using the dispatcher from
> ServletRequest, forwards and includes will be relative to the servlet
> path unless preceded by a slash.  In that case, it's relative to
> context root, i.e. WebContent.  At least this is how it works in
> Tomcat and WebSphere.

You're right. In the servlet spec I found this part a bit confusing it
only explicitly mentions the relative paths. Thanks for clarifying
this.

> I tried using the RequestDispatcher from ServletContext without Guice
> (straight web.xml mapping) on Tomcat and it looks like it actually
> wants all paths to start with a slash, it too serves relative to
> context root.

On the other hand this is explicitly mentioned in the spec.
RequestDispatcher for ServletContext must start with a slash. I
thought Context and Request dispatcher are exclusive one is absolute
the other relative. But the latter is only complementary. Great.

>
> WEB-INF is special in that it can't be access directly from the client
> side but you're allowed to access it internally via a request
> dispatcher.

Right. I've always thought there is somebody with a shotgun protecting
WEB-INF. I was wrong;)

> I experimented with getServletContext().getRequestDispatcher() with
> the Guice servlet adapter and it too doesn't work with forwards but
> it's fine with include.
>  So the request dispatcher doesn't seem to
> make a difference and stepping through the debug showed it was the
> same problem with the servlet path reported by the server request
> wrapper.
>

Got it! Squash! Like there is a saying Premature Optimization is the
root of all evil or something like that.

What happens is that the Wrapper caches the paths but when you forward
the paths in the original request are changed but the wrapper returns
the old cached path!

Quick fix is just to get rid of the Memoizer in ServletDefinition
lines 203+. Hope that helps.

Will take a closer look at this wrapper soon and maybe make a patch or
something.

> I don't think that's the case.  When I tried /test in my browser, it
> did make a request for /test on the servlet side, sans final slash.

Never mind. What was I thinking? Nothing! Today it works as expected.
Must been some Heisenbug in the browser or something.

[tchan]

Well, as a workaround, I've resorted to using this when I need to do a
forward:

protected static ServletRequest getUnderlyingRequest
( HttpServletRequest request )
{
ServletRequest innerRequest = request;
while ( innerRequest instanceof HttpServletRequestWrapper )
{
innerRequest = ( ( HttpServletRequestWrapper )
innerRequest ).getRequest();
}
return innerRequest;
}

It's pure hack but I can live with it until it gets fixed in Guice.
And I believe the WEB-INF jsp trick is only valid for recent versions
of the spec, I only learned of this in the past couple of years myself
but it's great for keeping your jsps private.