FilterDefinition should wrap the servlet request like ServletDefinition

[GoogleCodeExporter]

Perhaps that design suggestion is incorrect.

Here is the scenario:

filter("/dir/*).through(MyFilter.class);
serve("/dir/*).with(MyServlet.class);

MyFilter.doFilter(request, response) {
  request.getServletPath() == "/dir/com/google/whatever.java"
  request.getPathInfo() == null
}

MyServlet.doGet(request, response) {
  request.getServletPath() == "/dir"
  request.getPathInfo() == "/com/google/whatever.java"
}

The filter does not get the same wrapped request that the servlet gets which 
makes it difficult for me to correctly process request info.  Can the filter 
also receive a Request that is wrapped correctly like the servlet appears to 
receive?

Original issue reported on code.google.com by James.Mo...@gmail.com on 23 May 2014 at 3:50

[GoogleCodeExporter]

I'd love to be able to use Guice, but this limitation is a killer.  I use 
filters frequently for security control and I can't do anything useful with a 
filter instantiated through Guice because it doesn't know it's url mount point.

The fix looks pretty straight-forward since it would mostly involve porting the 
working code from the ServletDefinition to the FilterDefinition.  I have made 
an attempt at this, but I couldn't figure out how to setup Eclipse for hacking 
on guice-servlet.

Original comment by James.Mo...@gmail.com on 29 May 2014 at 11:55

[GoogleCodeExporter]

Can you not check the request.getURI() for the same details?

Or do you need it to be done per servlet?

Original comment by dha...@gmail.com on 12 Jun 2014 at 1:38

[GoogleCodeExporter]

I tried that... but (using my above example) the result is:

Request URI: /context/dir/com/google/whatever.java

I have probably 10 filters each with different paths.  Properly identifying 
pathinfo is a critical step.  I could make assumptions and hack on the returned 
urls, but that is not a good solution.

guice-servlet has the correct servlet path info internally, but the wrapped 
request isn't constructed correctly for filters. The wrapped request is correct 
(AFAIK) for servlets [1].

I think I now have a working dev environment. I can attempt a patch (bringing 
some of the ServletDefinition design to the FilterDefinition), if that would be 
welcome.

[1]: 
https://code.google.com/p/google-guice/source/browse/extensions/servlet/src/com/
google/inject/servlet/ServletDefinition.java#194

Original comment by James.Mo...@gmail.com on 12 Jun 2014 at 12:57

[GoogleCodeExporter]

Yup, patches with regression/unit tests are very welcome.

Original comment by sberlin on 12 Jun 2014 at 1:52

[GoogleCodeExporter]

I've attached a patch which wraps the servlet request before filter execution 
using code hijacked from ServletDefinition.  I'm not sure if this is 100% 
correct (i.e. REQUEST_DISPATCHER_REQUEST attribute) but this change provides my 
filters with the correct path info.

This change required a tweak to a unit test to retrieve the inner (I hesitate 
to say original) request for identity comparison.

Unfortunately, this patch also includes a regression (doh!) in:

    FilterDispatchIntegrationTest.testDispatchFilterPipelineWithRegexMatching

I haven't figured out why this is the case yet.  When debugging the test, 
successful test conditions are met - but it fails at what seems to be an 
unusual place - no doubt due to EasyMock proxy generation which I am unfamiliar 
with.  There is a subtle detail here which is escaping me.

Original comment by James.Mo...@gmail.com on 12 Jun 2014 at 3:52

Attachments:

• fix_issue_807.patch