vipx / google-guice

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

ServletDefinition#getPathInfo() logic is broken - triggers Index out of bounds when servlets forward #580

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
I am forwarding from one servlet to another which triggers an exception in 
Guice code.

The request comes in as http://localhost:8080/about, which is forwarded to the 
FreemarkerServlet by 
request.getRequestDispatcher("/about.tpl").forward(request, response);
...the pathInfo logic gets confused and tries to retrieve a large section of a 
zero-length string. 

Can anyone tell me how I'm supposed to forward requests as this is definitely 
broken? Perhaps it's the servlet engine which is broken, but the error turns up 
in Guice.

Copy-pasting the stack information from a live eclipse debugging session gives 
you these values...
String "getServletPath()"= "/about.test"    
String "getRequestURI()"= "/about.ftl"  
String "getContextPath()"= ""   

...so clearly the following code in ServletDefinition will throw an array out 
of bounds.

final int servletPathLength = getServletPath().length();
pathInfo = 
getRequestURI().substring(getContextPath().length()).replaceAll("[/]{2,}", 
"/").substring(servletPathLength);

The project is organised as a maven project, managed in eclipse, and is 
downloadable from  github if more information about configuration is needed. 
Servlet config is mostly in this directory...

https://github.com/cefn/Spring-JPA-MVC-Postgres/tree/1407d2d000aa2f9e964f23406a3
aee355de14c00/src/main/java/com/cefn/filesystem/servlet

I'm hosting GuiceServlets 3.0-20100927 from the maven groupid 
com.mycila.com.google.inject.extensions on Jetty 8.0.0M2. 

Original issue reported on code.google.com by google....@cefn.com on 14 Dec 2010 at 7:37

GoogleCodeExporter commented 9 years ago
Can you submit a testcase describing the broken behavior please? Then we can 
fix it up.

Original comment by dha...@gmail.com on 18 Dec 2010 at 10:41

GoogleCodeExporter commented 9 years ago
I've drawn out the individual components from the repo I linked to. The forward 
never completes, so I don't think it matters what 'about.ftl' is bound to.

These lines should only take 30 seconds to copy paste into a working 
Guice-trunk test environment, but I haven't successfully set one up yet.

>>>>>>>>>>>>

In ServletContextAdaptor.ServletModule$1#configureServlets()...

bind(AboutServlet.class).in(Singleton.class);
serve("/about.test").with(AboutServlet.class);

In AboutServlet#service(HttpServletRequest, HttpServletResponse)...

operations.forward(request, response, "/about.ftl");

...which cashes out in BasicHttpServlet.BasicOperations#forward...

public void forward(HttpServletRequest request, HttpServletResponse response, 
String path) throws IOException, ServletException{
    request.getRequestDispatcher(path).forward(request, response);
}

<<<<<<<<<<<<<<<<<

I just noticed mccullis' guidance on Guice test cases against my question how 
to write test cases...
https://code.google.com/p/google-guice/issues/detail?id=522#c9
...and I'll try and schedule the time to work through this next week so I can 
commit a proper failing test case if copy pasting those lines doesn't recreate.

Original comment by google....@cefn.com on 19 Dec 2010 at 12:16

GoogleCodeExporter commented 9 years ago
Forgot to say you would need to visit http://localhost:8080/about.test in order 
to recreate the exception, not http://localhost:8080/about as per my original 
errored description. That's why proper test cases are better!

Original comment by google....@cefn.com on 19 Dec 2010 at 12:20

GoogleCodeExporter commented 9 years ago
I checked out the trunk build and tried to write a testcase. 

I had a hard time rcreating exactly the logic of Jetty through Mock objects as 
I simply don't know what this logic is. The logic which steers things is 
principally about setting named Attributes in the request and I can't find the 
right order to recreate the routing and trigger the fail.

Nevertheless it seems to be an established problem. I found another example of 
it...
http://groups.google.com/group/google-guice/browse_thread/thread/9e360e237d8ad6e
b?pli=1

Original comment by c...@cefn.com on 21 Dec 2010 at 1:01

GoogleCodeExporter commented 9 years ago

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