Servlet forwards from servlets with pathinfo mishandled

[GoogleCodeExporter]

This is related to bug 455. Following advice there, to overcome that bug I 
built from trunk, which partly solved the problem with servlet forwards to JSPs.

The remaining difficulty is that servlets invoked with additional path info are 
mishandled. It I have a servlet at "/rss/*" and I invoke it as "/rss/news" 
(with /news being pathinfo), then try to forward to "news.jsp", it dies trying 
to find "rsss.jsp". Apparently the base part of the servlet path is being 
overlaid on the beginning of the forwarding string.

If I change the servlet to handle only "/rss", it works fine -- but I can't use 
pathinfo.

Original issue reported on code.google.com by cdbe...@gmail.com on 13 Jul 2010 at 5:56

[GoogleCodeExporter]

Even if a fix might take a while, a suggested workaround for this would be most 
appreciated. I'd very much like to be able to use pathinfo with my 
guice-enabled servlets that forward to JSPs.

Original comment by cdbe...@gmail.com on 13 Jul 2010 at 8:28

[GoogleCodeExporter]

In your forwarding servlet you could alter the path info with an 
HttpServletRequestWrapper to return the correct one for the JSP.

Original comment by dha...@gmail.com on 13 Jul 2010 at 11:05

[GoogleCodeExporter]

That doesn't seem to work. The real issue is that the name of the forwarding 
guice-controlled servlet (/rss) is getting written over the name of the forward 
target (/news.jsp). I don't know how to diagnose or correct this. Any ideas? Is 
anyone out there using a guice servlet handling /foo/* with a forward to a jsp 
successfully?

Original comment by cdbe...@gmail.com on 15 Jul 2010 at 4:43

[GoogleCodeExporter]

Anyone have a fix for this? Or a workaround? Or an ETA for either? I've had to 
fall back to using a non-injected servlet for any case where I need to forward 
to JSP, which sucks pretty bad.

Original comment by cdbe...@gmail.com on 27 Jul 2010 at 11:58

[GoogleCodeExporter]

Can you submit a patch for us? that will speed things up, in the meantime I'll 
try and look at it in a week or so, but I'm swamped of late =(

Original comment by dha...@gmail.com on 28 Jul 2010 at 5:03

[GoogleCodeExporter]

If you're able to write a test for this, cdberry, preferably in the same spirit 
as the existing servlet tests, it will make writing a fix easier.  (Or if you 
submit a patch with a fix, too.)

Original comment by sberlin on 1 Aug 2010 at 7:59

[GoogleCodeExporter]

Issue 562 has been merged into this issue.

Original comment by limpbizkit on 23 Oct 2010 at 11:44

[GoogleCodeExporter]

Issue 549 has been merged into this issue.

Original comment by limpbizkit on 23 Oct 2010 at 11:45

[GoogleCodeExporter]

I'd like to help with creating a testcase and a patch, but I'm struggling with 
getting an archetypal Guice environment which includes dependencies, source and 
tests through Maven or anything else.

Is there a royal road to creating a Eclipse project which can run Guice tests, 
so I can add mine (inside or outside Eclipse).

Is there still a blocker with no Guice source being available in Maven?

Original comment by google....@cefn.com on 6 Dec 2010 at 3:19

[GoogleCodeExporter]

Just install m2eclipse and checkout and import Guice trunk as an existing Maven 
project(s) - tests can be run from Eclipse as normal by right-clicking on the 
test and selecting RunAs->JUnitTest.

Original comment by mccu...@gmail.com on 6 Dec 2010 at 3:33

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

Hi, 
Sorry for the noise. I have been bitten by this issue too - both 455 and 522 - 
so I took a serious look at it in the last couple of days and I believe that I 
have identified the problem and a possible solution (in the attached patch).

The root of the problem is that Guice's handling of include() and forward() 
doesn't duplicate everything that a servlet container would do - restoring the 
query, path, attributes, etc. 

One way to handle it properly is to let the servlet container perform the 
forward() or include() and get control into Guice again after that. Also, the 
request must be unwrapped to take care of the memoization problem.

In addition to the attached patch, a second filter for include and forward must 
be installed in web.xml (see the description in the patch).

The attached patch is against Guice-2.0 Stable. It seems to resolve the issues 
for me and seems to work OK after some minimal testing. 

More importantly, I do feel that it is doing the right thing conceptually. Hope 
this helps, and any comments are welcome.

Original comment by tmi...@gmail.com on 19 Dec 2010 at 10:26

Attachments:

• 0001-guice-servlet-2.0-patched-fix-Guice-issues-455-and-5.patch

[GoogleCodeExporter]

I run into this bug so often just when experimenting with test cases that I'm 
surprised people can use Guice in a production environment at all!

I can't even get Stripes quickstart to work, and I think I've traced it to 
Guice forwarding. Here's the original issue I raised on the Stripes user list. 
From what I could see from the stack Stripes' DispatcherServlet was doing 
everything it should do, and then the forward back to the URL was being botched 
by Guice. 
http://permalink.gmane.org/gmane.comp.java.stripes.user/12594

I attempted to set up a testing environment, (thanks for the info), and started 
trying to author a test case but I struggled so much with the 'correct' model 
of the name/value pairs being thrown around at each stage in the Servlet 
processing that I found it very hard to author mocks which adequately recreate 
the behaviour of my (Jetty) servlet environment.

@tmikov Glad an expert who can follow what all these attributes should do was 
able to look into it. Thanks for all your efforts. I hope the detail you've 
provided means a patch makes it into guice-servlets trunk for the rest of us. 

At this stage I don't have a web.xml at all, though looks like I'll have to go 
back to it to incorporate your patch, or possibly drop guice-servlets to avoid 
maintaining two different ways to wire things. When Guice dies on such a 
mainstream operation as a forward, it's hard to keep the faith.

Original comment by google....@cefn.com on 9 Jan 2011 at 10:32

[GoogleCodeExporter]

The good news is I have been using Guice-stable-2.0 with my patch in a 
production environment for the last two weeks and I haven't experienced any 
problems so far. So I am feeling pretty good about it.

I think I can port it to trunk, though I'd prefer to get some feedback on the 
approach itself from the developers. It is possible that I am missing some 
important detail.

Original comment by tmi...@gmail.com on 9 Jan 2011 at 10:58

[GoogleCodeExporter]

Assuming the patch is accepted, is there a way to wire in your Forwarding 
filter using Guice bindings without web.xml? This is one of the holy grails of 
Guice for me; avoiding type-unsafe XML files lying about. 

I can't see anything equivalent in Guice-servlets to the <dispatcher/> element 
in the <filter-mapping> of a web.xml which would permit you to bind differently 
for FORWARD, INCLUDE, REQUEST, ERROR

Perhaps the Forwarding state could be checked in the ForwardingFilter itself, 
assuming the host servlet engine is strict on setting the correct attributes in 
the request.

javax.servlet.forward.* and javax.servlet.include.* seem to be the 'namespaces' 
to check for...
http://java.boot.by/wcd-guide/ch03s05.html

In this way the ForwardingFilter could be wired to handle all requests, check 
request attributes and only intervene when necessary, whilst at the same time 
an efficiently configured server can bring it in to the chain only when 
forwarding or including is actually happening. 

Original comment by google....@cefn.com on 10 Jan 2011 at 1:08

[GoogleCodeExporter]

@tmikov I think this may fix issue 580 too since I think the source of the 
problem is the same (Guice's incomplete manipulation of the Request's 
attributes) and the bug should be fixed by handing off to the Servlet engine's 
more consistent handling.

Original comment by google....@cefn.com on 10 Jan 2011 at 3:46

[GoogleCodeExporter]

Original comment by sberlin on 22 Feb 2011 at 1:44

• Added labels: Extension-Servlet

[GoogleCodeExporter]

With Guice 3.0, I have the same kind of problem (same source) when using 
forwards with the Apache Click framework; I just had to replace all forwards by 
redirects to make it work, that's not very good for performance!

Is there a plan to fix this issue for Guice 3.1, is it fixed in the trunk?

Original comment by jfpoilp...@gmail.com on 31 May 2012 at 7:06

[GoogleCodeExporter]

I wouldn't be optimistic for any attention. My guess is that this framework is 
no longer actively maintained, or that it is not used by the maintainers to 
configure servlets in the way mainstream java developers tend to use them.

I don't know how else to explain the fact this huge bug has lurked for so many 
years, even after a patch was produced. Perhaps it's a latent bug which is only 
triggered for some special configurations somehow, else how can anyone use 
Guice-Servlets at all?

Sadly I gave up on using Guice-Servlets for this reason, despite its excellent 
architecture and huge promise, but only after days spent debugging.

Original comment by c...@cefn.com on 31 May 2012 at 7:51

[GoogleCodeExporter]

What is the news about this issue (+ bug 647) ? Would we have a chance to use 
guice in a webapp project, if ever ?

Original comment by dupon...@gmail.com on 31 Dec 2012 at 9:11

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

Any word if this patch was ever taken? Based on these threads, there doesn't 
seem to be much activity on this project.

Original comment by raya...@gmail.com on 30 Apr 2014 at 6:12

[GoogleCodeExporter]

The patch was not taken.  Please consider adding another patch that includes 
tests and is slightly simpler (e.g -- separate refactoring from fixing bugs).

Original comment by sberlin on 30 Apr 2014 at 6:51

[GoogleCodeExporter]

I submitted a patch for this critical design error four (!!!) years ago with 
absolutely no comments or feedback from whoever is responsible for this 
project. Sadly, things have moved on in the meantime. You would be crazy to 
waste your time with Guice these days...

Original comment by tmi...@gmail.com on 30 Apr 2014 at 7:13

[GoogleCodeExporter]

>>You would be crazy to waste your time with Guice these days...

what would be best alternative these days then?

Original comment by hus...@gmail.com on 1 May 2014 at 11:34

[GoogleCodeExporter]

After reading this thread, I searched and found the Google Java Libs team 
recommending Dagger from 4square as well as Guice.  
http://www.reddit.com/r/java/comments/1y9e6t/ama_were_the_google_team_behind_gua
va_dagger

Original comment by kl...@quixey.com on 1 May 2014 at 4:48

[GoogleCodeExporter]

Dagger's support for servlets does not seem as extensive as Guice. Though this 
bug is annoying, I've decided to use includes instead of forwards (i.e. 
requestDispatcher.include) as a workaround.

Original comment by p.legaul...@gmail.com on 6 Jul 2014 at 5:26