Inconsistent handling of empty "path info" between Jetty 10 and 12 #9906

grgrzybek · 2023-06-13T11:23:52Z

Jetty version(s)
12.0.0.beta1

Java version/vendor (use: java -version)

$ java -version
java version "11.0.19" 2023-04-18 LTS
Java(TM) SE Runtime Environment 18.9 (build 11.0.19+9-LTS-224)
Java HotSpot(TM) 64-Bit Server VM 18.9 (build 11.0.19+9-LTS-224, mixed mode)

OS type/version

$ uname -srv
Linux 6.3.6-200.fc38.x86_64 #1 SMP PREEMPT_DYNAMIC Mon Jun  5 15:45:04 UTC 2023

Description
When trying to use ServletContextHandler with default servlet only, there's different behavior between Jetty 10 and 12.

How to reproduce?
I'm working on Pax Web (OSGi CMPN implementation of web related specification based on Jetty, Tomcat and Undertow). When trying to move to jakarta namespace (JakartaEE 10 / Servlet API 6), I now check if everything works consistently.

One of the "showcase tests" (where I try to provide canonical embedded examples) is (Jetty 10): https://github.com/ops4j/org.ops4j.pax.web/blob/web-9.0.9/pax-web-jetty/src/test/java/org/ops4j/pax/web/service/jetty/internal/EmbeddedJettyTest.java#L356-L443

Roughly speaking there's:

org.eclipse.jetty.server.Server + org.eclipse.jetty.server.ServerConnector
org.eclipse.jetty.server.handler.ContextHandlerCollection set as Server's handler
two org.eclipse.jetty.ee10.servlet.ServletContextHandler instances added as /c1 and /c2 to the above chc
the ServletContextHandler has been configured with ServletContextHandler#setAllowNullPathInfo(true) and ContextHandler#setAllowNullPathInContext(true)
each ServletContextHandler has one ServletHolder with default servlet mapped to /
I call GET /c1 and GET /c2

In Jetty 10.0.15, I'm at this stage:

"qtp1336777650-22@2859" prio=5 tid=0x16 nid=NA runnable
  java.lang.Thread.State: RUNNABLE
	  at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1282)
	  at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:129)
	  at org.eclipse.jetty.server.handler.ContextHandlerCollection.handle(ContextHandlerCollection.java:192)
	  at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:122)
	  at org.eclipse.jetty.server.Server.handle(Server.java:563)
	  at org.eclipse.jetty.server.HttpChannel.lambda$handle$0(HttpChannel.java:505)
	  at org.eclipse.jetty.server.HttpChannel$$Lambda$192.1150165308.dispatch(Unknown Source:-1)
	  at org.eclipse.jetty.server.HttpChannel.dispatch(HttpChannel.java:762)
	  at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:497)
	  at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:282)
	  at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:314)
	  at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:100)
	  at org.eclipse.jetty.io.SelectableChannelEndPoint$1.run(SelectableChannelEndPoint.java:53)
	  at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:969)
	  at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.doRunJob(QueuedThreadPool.java:1194)
	  at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1149)
	  at java.lang.Thread.run(Thread.java:834)

target parameter is /c1 and this branch is invoked:

else
{
    target = URIUtil.SLASH;
    pathInContext = null;
}

Then in org.eclipse.jetty.servlet.ServletHandler#getMatchedServlet() target is / and proper mapping is taken using:

if (target.startsWith("/"))
{
    if (_servletPathMap == null)
        return null;
    return _servletPathMap.getMatched(target);
}

However in Jetty 12.0.0.beta1, there's different code being called here:

"qtp366803687-22@3399" prio=5 tid=0x16 nid=NA runnable
  java.lang.Thread.State: RUNNABLE
	  at org.eclipse.jetty.server.Context.getPathInContext(Context.java:129)
	  at org.eclipse.jetty.server.handler.ContextHandler$ScopedContext.getPathInContext(ContextHandler.java:1313)
	  at org.eclipse.jetty.ee10.servlet.ServletContextHandler.wrapRequest(ServletContextHandler.java:1146)
	  at org.eclipse.jetty.ee10.servlet.ServletContextHandler.wrapRequest(ServletContextHandler.java:123)
	  at org.eclipse.jetty.server.handler.ContextHandler.handle(ContextHandler.java:805)
	  at org.eclipse.jetty.server.handler.ContextHandlerCollection.handle(ContextHandlerCollection.java:179)
	  at org.eclipse.jetty.server.Handler$Wrapper.handle(Handler.java:611)
	  at org.eclipse.jetty.server.Server.handle(Server.java:175)
	  at org.eclipse.jetty.server.internal.HttpChannelState$HandlerInvoker.run(HttpChannelState.java:553)
	  at org.eclipse.jetty.server.internal.HttpConnection.onFillable(HttpConnection.java:480)
	  at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:322)
	  at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:100)
	  at org.eclipse.jetty.io.SelectableChannelEndPoint$1.run(SelectableChannelEndPoint.java:53)
	  at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:969)
	  at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.doRunJob(QueuedThreadPool.java:1194)
	  at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1149)
	  at java.lang.Thread.run(Thread.java:833)

and it's this branch (encodedContextPath == encodedPath == /c1):

if (encodedPath.length() == encodedContextPath.length())
    return "";

this time org.eclipse.jetty.ee10.servlet.ServletHandler#getMatchedServlet() is being passed "" value and while _servletPathMap is correct:

_servletPathMap = {org.eclipse.jetty.http.pathmap.PathMappings@4295}  size = 1
 {org.eclipse.jetty.http.pathmap.ServletPathSpec@4466} "ServletPathSpec@9457dcb7{DEFAULT,/}" -> {org.eclipse.jetty.ee10.servlet.ServletHandler$MappedServlet@4467} "MappedServlet6faad6a5{/->default-servlet==org.ops4j.pax.web.service.jetty.internal.EmbeddedJettyTest$5@2c16bab9{jsp=null,order=-1,inst=true,async=true,src=EMBEDDED:<null>,STARTED}}"
  key: org.eclipse.jetty.http.pathmap.ServletPathSpec  = {org.eclipse.jetty.http.pathmap.ServletPathSpec@4466} "ServletPathSpec@9457dcb7{DEFAULT,/}"
  value: org.eclipse.jetty.ee10.servlet.ServletHandler$MappedServlet  = {org.eclipse.jetty.ee10.servlet.ServletHandler$MappedServlet@4467} "MappedServlet6faad6a5{/->default-servlet==org.ops4j.pax.web.service.jetty.internal.EmbeddedJettyTest$5@2c16bab9{jsp=null,order=-1,inst=true,async=true,src=EMBEDDED:<null>,STARTED}}"

this fragment is used instead:

MappedServlet holder = _servletNameMap.get(target);

and null is returned.

I know that Jetty 12 is still being worked on, but I hope you don't mind reporting such differences ;)

The text was updated successfully, but these errors were encountered:

janbartel · 2023-06-13T13:14:02Z

@grgrzybek we're very happy to have people looking at jetty12! I'll look into this for you. Can you confirm you are using ee10 in jetty-12, as I notice your issue in PaxWeb is entitled ee9?

grgrzybek · 2023-06-13T13:30:13Z

@janbartel I've renamed the Pax Web issue (I'll skip JakartaEE 9 / Servlet API 5 and go directly to 10 / 6).

And yes - this is definitely ee10 (I see Tomcat 10.1 and Undertow 2.3 are based on Servlet API 6). I created this Pax Web issue when I was starting to consider javax -> jakarta migration, but there was no Jetty 12 back then.

I hope I can help with the compliance (as with #9119).

janbartel · 2023-06-13T13:33:33Z

@grgrzybek as a temporary work around, if you want to get further in your coding while I reproduce and dig into this, you could try using the urls /c1/ and /c2/ - ie add a trailing slash. I bet that works.

grgrzybek · 2023-06-13T14:23:09Z

@grgrzybek as a temporary work around, if you want to get further in your coding while I reproduce and dig into this, you could try using the urls /c1/ and /c2/ - ie add a trailing slash. I bet that works.

Yes - that's how it worked for me ;)

grgrzybek · 2023-06-13T14:25:45Z

BTW, I got confused with:

org.eclipse.jetty.ee10.servlet.ServletContextHandler#setAllowNullPathInfo()
org.eclipse.jetty.server.handler.ContextHandler#setAllowNullPathInContext()

The first one doesn't seem to be used. While it was used by org.eclipse.jetty.server.handler.ContextHandler#checkContext() In Jetty 11.

janbartel · 2023-06-13T14:30:30Z

@grgrzybek yep, we shouldn't have 2 methods like that, fix will be coming. Following up on difference between path info as "" or forcing it to be "/" like we did in jetty-10/11 and probably jetty9 as well.

janbartel · 2023-06-13T15:49:21Z

@grgrzybek I've raised PR #9907. You might like to try out that branch and see if that fixes your problem. Note what I said in the PR, that we will not force the path on the server side to be / instead of "". We talked it over and decided that trying to fix that in jetty-12 would insert the cost of a few to many if else statements into the critical path of handling a request and we didn't feel it was worth it. Especially as urls like /ctx should strictly be handled by a redirect, and when accepting and processing them without a redirect we've gone beyond the bounds of the servlet spec.

So hopefully this fix will allow you to still get your content served in the way you have been used to, but leaving the path as "" instead of forcing it to be /. If you really need the path to be /, then you could use the RewriteHandler to change /ctx to /ctx/ before it ever hit the ServletContextHandler and avoid the whole issue.

grgrzybek · 2023-06-13T16:08:17Z

Thanks - over next days/weeks, I'll try to work more on Pax Web migration from javax to jakarta. I'll use this branch of Jetty instead of just what's in Maven Central (beta1 at this moment).

I remember that (from Pax Web point of view) handling these weird URIs that use context path only without trailing slash was a bit tricky and all runtimes (Jetty, Tomcat, Undertow) have non-standard ways to handle it.

Tomcat: org.apache.catalina.core.StandardContext#setMapperContextRootRedirectEnabled()
Jetty: org.eclipse.jetty.server.handler.ContextHandler#setAllowNullPathInfo()
Undertow: special tweak in overriden org.ops4j.pax.web.service.undertow.internal.web.DefaultServlet#doGet() (extension of io.undertow.servlet.handlers.DefaultServlet)

Fortunately, I chose (in Pax Web) to always redirect /context to /context/, so I support your comment about mandatory redirect ;)

* Issue #9906 Empty path info * Update jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ServletContextHandlerTest.java Co-authored-by: Olivier Lamy <[email protected]> * Update jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/ServletContextHandlerTest.java Co-authored-by: Olivier Lamy <[email protected]> --------- Co-authored-by: Olivier Lamy <[email protected]>

grgrzybek · 2023-06-26T07:57:45Z

@janbartel

we will not force the path on the server side to be / instead of "".

I agree. I won't expect "/c1" request to be handled by default servlet anymore. Please close this issue then.

grgrzybek added the Bug For general bugs on Jetty side label Jun 13, 2023

grgrzybek mentioned this issue Jun 13, 2023

Prepare Pax Web version using JakartaEE 10+ API ops4j/org.ops4j.pax.web#1802

Open

janbartel self-assigned this Jun 13, 2023

janbartel added a commit that referenced this issue Jun 13, 2023

Issue #9906 Empty path info

eab758f

janbartel mentioned this issue Jun 13, 2023

Issue #9906 Empty path info #9907

Merged

janbartel closed this as completed Jun 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Inconsistent handling of empty "path info" between Jetty 10 and 12 #9906

Inconsistent handling of empty "path info" between Jetty 10 and 12 #9906

grgrzybek commented Jun 13, 2023

janbartel commented Jun 13, 2023

grgrzybek commented Jun 13, 2023

janbartel commented Jun 13, 2023

grgrzybek commented Jun 13, 2023

grgrzybek commented Jun 13, 2023

janbartel commented Jun 13, 2023

janbartel commented Jun 13, 2023

grgrzybek commented Jun 13, 2023

grgrzybek commented Jun 26, 2023

Inconsistent handling of empty "path info" between Jetty 10 and 12 #9906

Inconsistent handling of empty "path info" between Jetty 10 and 12 #9906

Comments

grgrzybek commented Jun 13, 2023

janbartel commented Jun 13, 2023

grgrzybek commented Jun 13, 2023

janbartel commented Jun 13, 2023

grgrzybek commented Jun 13, 2023

grgrzybek commented Jun 13, 2023

janbartel commented Jun 13, 2023

janbartel commented Jun 13, 2023

grgrzybek commented Jun 13, 2023

grgrzybek commented Jun 26, 2023