frjaeger220 / google-guice

Automatically exported from code.google.com/p/google-guice
Apache License 2.0
0 stars 0 forks source link

Filters match on a servlet path, not the actual request path #418

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
In a Jersey based application (where all requests are handled by the Guice
based jersey filter, we use a single servlet as a "catchall". So we have

serve("*").with(JerseyContainer.class); 

in the code. There are multiple endpoints mapped onto this servlet using
Jersey Resources. Some requests should now be processed by an additional
filter. So I added

filter("/special/*").through(MyFilter.class);

but MyFilter never gets triggered. The reason for this is that in line 126
of FilterDefinition, in doFilter() is a

final String path = ((HttpServletRequest) servletRequest).getServletPath();

This should be

final String path = ((HttpServletRequest) servletRequest).getRequestURI();

Original issue reported on code.google.com by hgsch...@gmail.com on 20 Aug 2009 at 10:52

GoogleCodeExporter commented 9 years ago

Original comment by dha...@gmail.com on 21 Aug 2009 at 1:06

GoogleCodeExporter commented 9 years ago
Attached patch fixes the problem

Original comment by hgsch...@gmail.com on 24 Nov 2009 at 8:45

Attachments:

GoogleCodeExporter commented 9 years ago
It seems like there is something similar in ServletDefinition, where there is 
the line:

boolean serve = shouldServe(((HttpServletRequest) 
servletRequest).getServletPath());

In Jetty at least this doesn't ever fire, because the servlet path is always 
"".  To
my mind that isn't too strange, since not servlet has been selected - this is 
in the
code for selecting a servlet. But by that logic, I can't see how this code 
could ever
work.

I'll try to rebuild using getRequestURI and see how that looks.

Original comment by undecons...@gmail.com on 23 Apr 2010 at 3:34

GoogleCodeExporter commented 9 years ago
Oh, there is already ISSUE-449 for this. The change does make everything work, 
but
that is probably already confirmed. Sorry for the noise.

Original comment by undecons...@gmail.com on 23 Apr 2010 at 4:15

GoogleCodeExporter commented 9 years ago

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

GoogleCodeExporter commented 9 years ago
This is the updated patch against guice 3.0-rc3 / trunk that fixes both issues 
418 and 449.

Original comment by henn...@schmiedehausen.org on 22 Mar 2011 at 2:46

Attachments:

GoogleCodeExporter commented 9 years ago
Issue 449 has been merged into this issue.

Original comment by sberlin on 22 Mar 2011 at 3:16

GoogleCodeExporter commented 9 years ago
This is an unit test that tests filtering of requests to the guicefilter. all 
tests pass with the patch applied. the failing tests on the default show nicely 
what assumptions are wrong. Tested with Apache Tomcat 6.0.32

Original comment by henn...@schmiedehausen.org on 23 Mar 2011 at 10:57

Attachments:

GoogleCodeExporter commented 9 years ago
fixed in r1529.  thanks for the patches!

Original comment by sberlin on 24 Mar 2011 at 1:45