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: r1346510 - in /tomcat/trunk: java/org/apache/catalina/core/StandardContext.java java/org/apache/tomcat/util/http/mapper/Mapper.java test/org/apache/tomcat/util/http/mapper/TestMapperContextRoot.java
Date Tue, 05 Jun 2012 19:57:25 GMT
2012/6/5  <markt@apache.org>:
> Author: markt
> Date: Tue Jun  5 18:17:58 2012
> New Revision: 1346510
>
> URL: http://svn.apache.org/viewvc?rev=1346510&view=rev
> Log:
> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=53356
> Add support for an explicit mapping of a servlet to the context root
>
> Added:
>    tomcat/trunk/test/org/apache/tomcat/util/http/mapper/TestMapperContextRoot.java
> Modified:
>    tomcat/trunk/java/org/apache/catalina/core/StandardContext.java
>    tomcat/trunk/java/org/apache/tomcat/util/http/mapper/Mapper.java
>

> @@ -1026,8 +1032,16 @@ public final class Mapper {
>         int pos = find(wrappers, path);
>         if ((pos != -1) && (path.equals(wrappers[pos].name))) {
>             mappingData.requestPath.setString(wrappers[pos].name);
> -            mappingData.wrapperPath.setString(wrappers[pos].name);
>             mappingData.wrapper = wrappers[pos].object;
> +            if (path.equals("/")) {
> +                // Special handling for Context Root mapped servlet
> +                mappingData.pathInfo.setString("/");
> +                mappingData.wrapperPath.recycle();
> +                // This seems wrong but it is what the spec says...
> +                mappingData.contextPath.recycle();
> +            } else {
> +                mappingData.wrapperPath.setString(wrappers[pos].name);
> +            }
>         }
>     }

> --- tomcat/trunk/test/org/apache/tomcat/util/http/mapper/TestMapperContextRoot.java (added)
> +++ tomcat/trunk/test/org/apache/tomcat/util/http/mapper/TestMapperContextRoot.java Tue
Jun  5 18:17:58 2012
> @@ -0,0 +1,79 @@
> +    private static class Bug53356Servlet extends HttpServlet {
> +
> +        private static final long serialVersionUID = 1L;
> +
> +        @Override
> +        protected void doGet(HttpServletRequest req, HttpServletResponse resp)
> +                throws ServletException, IOException {
> +            // Confirm behaviour as per Servler 12.2
> +            boolean pass = "/".equals(req.getPathInfo());
> +            if (pass) {
> +                pass = (req.getServletPath() == null);
> +            }
> +            if (pass) {
> +                pass = (req.getContextPath() == null);
> +            }
> +

Looking into Servlet spec 3.0 rev.a chapter 12.2.

It says that contextPath and servletPath are empty string (""),  but
the code above expects nulls.

Regarding contextPath being "":
It is surely strange, as

1. it contradicts other places like 3.5  (definition of "Context Path"
and famous equation of requestURI = contextPath + servletPath +
pathInfo ).

2. There is also ServletContext.getContextPath() that provides the
correct information.

3. Though looking at Javadoc [3], it says that
HttpServletRequest.getContextPath() may be different with
ServletContext.getContextPath(), but it does not mention this special
servlet mapping as a case.

[3] http://docs.oracle.com/javaee/6/api/javax/servlet/http/HttpServletRequest.html#getContextPath%28%29

I wonder if it was ever raised with the EG.
It is worth mentioning the chapter number (12.2) in the comment in Mapper.java

I suspect that this feature is not covered by official tests, because
otherwise we would have had to deal with it earlier.

So maybe we should respect ch.3.5 here, instead of 12.2?

> +            resp.setContentType("text/plain");
> +            if (pass) {
> +                resp.getWriter().write("OK");
> +            } else {
> +                resp.getWriter().write("FAIL");
> +            }
> +        }
> +    }
> +}
>

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