Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
gissuebot opened this issue Jul 7, 2014 · 9 comments
Closed

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

gissuebot opened this issue Jul 7, 2014 · 9 comments

Comments

@gissuebot
Copy link

@gissuebot gissuebot commented Jul 7, 2014

From hgschmie on August 20, 2009 18:52:13

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: http://code.google.com/p/google-guice/issues/detail?id=418

@gissuebot
Copy link
Author

@gissuebot gissuebot commented Jul 7, 2014

From dhanji on August 20, 2009 18:06:38

(No comment was entered for this change.)

Status: Accepted
Owner: dhanji

@gissuebot
Copy link
Author

@gissuebot gissuebot commented Jul 7, 2014

From hgschmie on November 24, 2009 00:45:45

Attached patch fixes the problem

Attachment: gist
   ISSUE-418.patch

@gissuebot
Copy link
Author

@gissuebot gissuebot commented Jul 7, 2014

From undeconstructed on April 23, 2010 08:34:11

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.

@gissuebot
Copy link
Author

@gissuebot gissuebot commented Jul 7, 2014

From undeconstructed on April 23, 2010 09:15:14

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

@gissuebot
Copy link
Author

@gissuebot gissuebot commented Jul 7, 2014

From sberlin on February 21, 2011 17:42:35

(No comment was entered for this change.)

Labels: Extension-Servlet

@gissuebot
Copy link
Author

@gissuebot gissuebot commented Jul 7, 2014

From henning@schmiedehausen.org on March 21, 2011 19:46:47

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

Attachment: gist
   servlet.patch

@gissuebot
Copy link
Author

@gissuebot gissuebot commented Jul 7, 2014

From sberlin on March 21, 2011 20:16:16

Issue 449 has been merged into this issue.

@gissuebot
Copy link
Author

@gissuebot gissuebot commented Jul 7, 2014

From henning@schmiedehausen.org on March 23, 2011 15:57:25

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

Attachment: gist
   issue418-test.patch

@gissuebot
Copy link
Author

@gissuebot gissuebot commented Jul 7, 2014

From sberlin on March 24, 2011 06:45:18

fixed in r1529 .  thanks for the patches!

Status: Fixed
Owner: sberlin

@gissuebot gissuebot closed this Jul 7, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant