tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Konstantin Kolinko <knst.koli...@gmail.com>
Subject Re: svn commit: r1004393 - /tomcat/trunk/java/org/apache/catalina/servlets/DefaultServlet.java
Date Wed, 06 Oct 2010 00:11:15 GMT
2010/10/5  <timw@apache.org>:
> Author: timw
> Date: Mon Oct  4 20:19:09 2010
> New Revision: 1004393
>
> URL: http://svn.apache.org/viewvc?rev=1004393&view=rev
> Log:
> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=50026
> Always calculate path of resource to be served relative to the context root.
> This invokes the standard protection of WEB-INF and META-INF directories.
> This is a breaking change for the unofficial use of DefaultServlet to remount the webapp
base under a new path.
>
> Modified:
>    tomcat/trunk/java/org/apache/catalina/servlets/DefaultServlet.java
>
> Modified: tomcat/trunk/java/org/apache/catalina/servlets/DefaultServlet.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/servlets/DefaultServlet.java?rev=1004393&r1=1004392&r2=1004393&view=diff
>

(...)

> @@ -303,6 +338,11 @@ public class DefaultServlet
>      * @param request The servlet request we are processing
>      */
>     protected String getRelativePath(HttpServletRequest request) {
> +        // IMPORTANT: DefaultServlet can be mapped to '/' or '/path/*' but always
> +        // serves resources from the web app root with context rooted paths.
> +        // i.e. it can not be used to mount the web app root under a sub-path
> +        // This method must construct a complete context rooted path, although
> +        // subclasses can change this behaviour.
>
>         // Are we being processed by a RequestDispatcher.include()?
>         if (request.getAttribute(Globals.INCLUDE_REQUEST_URI_ATTR) != null) {
> @@ -319,7 +359,11 @@ public class DefaultServlet
>         // No, extract the desired path directly from the request
>         String result = request.getPathInfo();
>         if (result == null) {
> +            // Mapped to '/'
>             result = request.getServletPath();
> +        } else {
> +            // Mapped to '/path/*' so get entire path under context
> +            result = request.getServletPath() + result;
>         }
>         if ((result == null) || (result.equals(""))) {
>             result = "/";
>

1. In DefaultServlet#getRelativePath(..) there is
 "if (request.getAttribute(Globals.INCLUDE_REQUEST_URI_ATTR) != null)"
branch several lines above that.  I suspect that it needs the same
changes. (As it does the same things with the paths, though takes
them from attributes).

2. The comments
> +            // Mapped to '/'
> +            // Mapped to '/path/*' so get entire path under context
look wrong to me. (It does not matter how the servlet is mapped, but
how it is requested)
The code is correct though.
I think those two wrong comments can be removed, as there is already a
long comment at the start of the method that explains the things.

Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message