servlet icon indicating copy to clipboard operation
servlet copied to clipboard

Fix wording in context root mapping in section 12.2

Open markt-asf opened this issue 5 years ago • 12 comments

The current wording is:

The empty string ("") is a special URL pattern that exactly maps to the application's context root, i.e., requests of the form http://host:port/<context-root>/. In this case the path info is ’ / ’ and the servlet path and context path is empty string (““).

The requirement that this makes the context path the empty string makes no sense. I propose we strike that text to give:

The empty string ("") is a special URL pattern that exactly maps to the application's context root, i.e., requests of the form http://host:port/<context-root>/. In this case the path info is ’ / ’ and the servlet path is empty string (““).

markt-asf avatar Jan 31 '20 14:01 markt-asf

Context: https://bz.apache.org/bugzilla/show_bug.cgi?id=64109

I agree. Context Path should always be the context path of given application / war / org.eclipse.jetty.server.handler.ContextHandler / org.apache.catalina.core.StandardContext.

grgrzybek avatar Jan 31 '20 14:01 grgrzybek

For documentation purpose, here's my summary for test of this special "" mapping rule from Servlet API 4.0, 12.2 "Specification of Mappings" chapter.

For three container implementations, I've created two contexts - one for / context path and one for /c1 context path.

Jetty

ServletContextHandler rootHandler = new ServletContextHandler(chc, "", ServletContextHandler.NO_SESSIONS);
rootHandler.setAllowNullPathInfo(true);

ServletContextHandler otherHandler = new ServletContextHandler(chc, "/c1", ServletContextHandler.NO_SESSIONS);
// with "false", "/c1" request will be redirected (HTTP 302) to "/c1/"
otherHandler.setAllowNullPathInfo(true);

Tomcat

Context rootContext = new StandardContext();
rootContext.setPath("");
rootContext.setMapperContextRootRedirectEnabled(false);

Context otherContext = new StandardContext();
otherContext.setPath("/c1");
// with "true", "/c1" request will be redirected (HTTP 302) to "/c1/"
otherContext.setMapperContextRootRedirectEnabled(false);

Undertow

DeploymentInfo rootContext = Servlets.deployment().setContextPath("/");

DeploymentInfo otherContext = Servlets.deployment().setContextPath("/c1");

for all the above context I've added a servlet mapped to 4 URI patterns:

  • /p/* (path mapping)
  • *.action (extension mapping)
  • "" (empty string - this special rule from 12.2 chapter of Servlet API spec, meaning "[...] maps to the application's context root")
  • /x (exact mapping)

I didn't use "/" mapping to not hijack other request URIs I wanted to test.

The single servlet just prints 3 things:

  • javax.servlet.http.HttpServletRequest#getContextPath()
  • javax.servlet.http.HttpServletRequest#getServletPath()
  • javax.servlet.http.HttpServletRequest#getPathInfo()

And I send these requests destined for root context:

  • GET /p/anything
  • GET /anything.action
  • GET /
  • GET /x
  • GET /y

and these destined for /c1 context:

  • GET /c1/p/anything
  • GET /c1/anything.action
  • GET /c1
  • GET /c1/
  • GET /c1/x
  • GET /c1/y

Here are summaries (CP=context path, SP=servlet path, PI=path info):

request jetty tomcat undertow
/p/anything cp: ""
sp: /p
pi: /anything
cp: ""
sp: /p
pi: /anything
cp: ""
sp: /p
pi: /anything
/c1/p/anything cp: /c1
sp: /p
pi: /anything
cp: /c1
sp: /p
pi: /anything
cp: /c1
sp: /p
pi: /anything
/anything.action cp: ""
sp: /anything.action
pi: null
cp: ""
sp: /anything.action
pi: null
cp: ""
sp: /anything.action
pi: null
/c1/anything.action cp: /c1
sp: /anything.action
pi: null
cp: /c1
sp: /anything.action
pi: null
cp: /c1
sp: /anything.action
pi: null
:exclamation: /c1
(can't have "" case)
cp: /c1
sp: ""
pi: null
HTTP 404 HTTP 404
:exclamation: / cp: ""
sp: ""
pi: null
cp: ""
sp: ""
pi: /
cp: ""
sp: /
pi: null
:exclamation: /c1/ cp: /c1
sp: ""
pi: null
cp: /c1
sp: ""
pi: /
cp: /c1
sp: /
pi: null
/x cp: ""
sp: /x
pi: null
cp: ""
sp: /x
pi: null
cp: ""
sp: /x
pi: null
/c1/x cp: /c1
sp: /x
pi: null
cp: /c1
sp: /x
pi: null
cp: /c1
sp: /x
pi: null
/y HTTP 404 HTTP 404 HTTP 404
/c1/y HTTP 404 HTTP 404 HTTP 404

Summarizing special "" mapping rule:

  • for GET /, Tomcat does it according to specification. Jetty returns wrong path info, Undertow returns bad servlet path and path info.
  • for GET /c1, Jetty (with setAllowNullPathInfo(true) preventing redirect to /c1/) behaves rationally but against specification, Tomcat (with setMapperContextRootRedirectEnabled(false) preventing redirect to /c1/) and Undertow return 404, which looks like "can't find mapping for "" == mapping for application's context root (as written in specification)".
  • for GET /c1/ (also with Jetty's setAllowNullPathInfo(false) and Tomcat's setMapperContextRootRedirectEnabled(true)), all containers return /c1 as context path (rational, but against spec) and servlet path + path info are inconsistent

I hope this summary will be a good inspiration for improving the specification ;).

grgrzybek avatar Feb 04 '20 10:02 grgrzybek

@grgrzybek Great Work!!!

I hope this summary will be a good inspiration for improving the specification ;). Well at the very least it is inspiration for improving Jetty :)

I think we also want to improve the spec to match jetty for /c1 and tomcat for / and /c1/

gregw avatar Feb 04 '20 17:02 gregw

I hope this summary will be a good inspiration for improving the specification ;).

And the TCK!!!

bshannon avatar Feb 05 '20 02:02 bshannon

I think we also want to improve the spec to match jetty for /c1 and tomcat for / and /c1/

I'd like to see a column for the "reference implementation" as well, before deciding which interpretation is "correct". We may not have a reference implementation for Jakarta EE, but we did for Java EE, and that's what we would've used to resolve ambiguities or conflicts such as this in the Java EE 8 spec, which this spec is supposed to be compatible with.

Hopefully, GlassFIsh and Tomcat behave the same due to their common ancestry.

bshannon avatar Feb 05 '20 02:02 bshannon

I'm not involved in a JCP process, so I should not suggest anything ;). IMO, even if Tomcat (which was RI at some point, right?) handles root context + "" mapping according to spec, it does it with some bad aftertaste.

Also, knowing that:

  • Jetty (with org.eclipse.jetty.server.handler.ContextHandler#setAllowNullPathInfo()),
  • Tomcat (with org.apache.catalina.Context#setMapperContextRootRedirectEnabled())
  • and even Undertow (with proper handler setup)

treat "access root of the context" as something special (security concerns?), maybe the spec should be more clear about it? What does it mean (12.2 Specification of Mappings) "[...] a special URL pattern that exactly maps to the application's context root"? IMO the "context root" is more virtual concept, mapping is always from URI to servlets (or filters), even a "resource" ("default") servlet.

Even "12.1 Use of URL Paths" ends the checklist with default servlet. No way to know what "access context root" means.

What would be the problem specifying that access to "/c1" should always be redirected to "/c1/" and then normal servlet lookup will take place?

grgrzybek avatar Feb 05 '20 06:02 grgrzybek

I tried checking Glassfish, but entire eclipse-ee4j/glassfish history related to Tomcat is one entry - can't tell which version of Tomcat it is. History of javaee/glassfish also looks more like cherry picking from official apache/tomcat repo than something more like alignment with upstream Tomcat version.

grgrzybek avatar Feb 05 '20 07:02 grgrzybek

What would be the problem specifying that access to "/c1" should always be redirected to "/c1/" and then normal servlet lookup will take place?

I think something should be said about handling of such requests. However, while I think that redirection is probably the best behaviour, the fact that both jetty and tomcat have felt the need to make it configurable indicates that there are probably some users out there that see differently. However I'm not sure they are in numbers large enough to warrant official support in the spec to configure non redirect behaviour. So I'd be fine with either a statement that /context SHOULD or MAY be redirected to /context/.

But note that this is really just a special case of the redirection from /context/path to /context/path/ that occurs if relative URIs are to be used - eg. prior serving a welcome file from a static directory. I'm not aware of anything in the spec that indicates that behaviour, but I believe most servers do it. So perhaps we should have a separate issue to come up with wording for that kind of redirect and then make sure that the context root is handled as a edge case of that?

gregw avatar Feb 05 '20 08:02 gregw

I tried checking Glassfish, but entire eclipse-ee4j/glassfish history related to Tomcat is one entry - can't tell which version of Tomcat it is. History of javaee/glassfish also looks more like cherry picking from official apache/tomcat repo than something more like alignment with upstream Tomcat version.

Yes, the history there is ugly. My hope was that someone could actually test it determine how the two behaved.

bshannon avatar Feb 06 '20 23:02 bshannon

Hi,

On Wed, Feb 5, 2020 at 8:30 AM Grzegorz Grzybek [email protected] wrote:

I tried checking Glassfish, but entire eclipse-ee4j/glassfish history https://github.com/eclipse-ee4j/glassfish/commits/master/appserver/web/web-core/src/main/java/org/apache related to Tomcat is one entry - can't tell which version of Tomcat it is. History of javaee/glassfish https://github.com/javaee/glassfish/commits/master/appserver/web/web-core/src/main/java/org/apache also looks more like cherry picking from official apache/tomcat repo than something more like alignment with upstream Tomcat version.

It's technically not "Tomcat", but Catalina and Jasper mostly. Coyote from Tomcat has been replaced by Grizzly.

That said, indeed, it's a proper mess. At some point the classes making up Catalina were obviously copied into GlassFish, and then later on only very sporadically fixes were imported. If you look at say FormAuthenticator.java and compare the Tomcat and GlassFish versions, you see they mostly match, but there are small changes everywhere. Some things like the Servlet Container Profile of JSR 196 has been implemented independently by both, etc.

Kind regards, Arjan Tijms

arjantijms avatar Feb 07 '20 00:02 arjantijms

It's technically not "Tomcat", but Catalina and Jasper mostly. Coyote from Tomcat has been replaced by Grizzly.

RIght. Then I don't think I should add 4th column to the above. the mapper (virtual host mapping, context mapping, servlet mapping) is done at Catalina level (and not Coyote/Grizzly) afaik. So it SHOULD behave as Tomcat. But you never know ;) And it'd really be good to know what Catalina version is used in GlassFish...

grgrzybek avatar Feb 07 '20 06:02 grgrzybek

maybe would be good to add such test in the TCK? I cannot find any atm (or I missed it?)

olamy avatar Feb 16 '20 20:02 olamy

Fixed via #497

markt-asf avatar Jan 27 '23 12:01 markt-asf