Bring back Jetty <12 flexibility of "current context / context handler"

[grgrzybek]

Jetty version(s)
Jetty 12.0.0.beta1

Enhancement Description
I work on Pax Web which uses Jetty, Tomcat or Undertow in OSGi environment behind OSGi CMPN web specification facades.

Pax Web 8 uses Jetty 9.4 and Pax Web 9 uses Jetty 10. One of the integration aspects is WebSocket handling (using SCIs and Jetty's org.eclipse.jetty.ee10.websocket.servlet.WebSocketUpgradeFilter).

Websockets related code (filters, scis) need to use Jetty API directly so in several places, generic javax|jakarta.servlet.ServletContext is used to obtain Jetty context and related context handler.

In Jetty 10, org.eclipse.jetty.servlet.ServletContextHandler#getServletContextHandler() is:

ContextHandler contextHandler = ContextHandler.getContextHandler(servletContext);
if (contextHandler == null)
    throw new IllegalStateException("No Jetty ContextHandler, " + purpose + " unavailable");

if (!(contextHandler instanceof ServletContextHandler))
    throw new IllegalStateException("No Jetty ServletContextHandler, " + purpose + " unavailable");

return (ServletContextHandler)contextHandler;

where org.eclipse.jetty.server.handler.ContextHandler#getContextHandler() is also using thread local org.eclipse.jetty.server.handler.ContextHandler#__context.
In Jetty 12 this is only:

if (servletContext instanceof ServletContextApi servletContextApi)
    return servletContextApi.getContext().getServletContextHandler();
throw new IllegalStateException("No Jetty ServletContextHandler, " + purpose + " unavailable");

In OSGi environment we have to pass around different implementation of javax|jakarta.servlet.ServletContext to fulfill the contract (like classloader access) and the above code simply doesn't work. With several static methods we can't reimplement them without using shaded versions of Jetty classes (which we don't want to use - except maybe DefaultServlet to unprivate some methods/fields).
I see that the org.eclipse.jetty.server.handler.ContextHandler#__context thread local is still available in Jetty 12 - can it be used again? Or is it going to be removed?

[janbartel]

@grgrzybek are you saying that in jetty-12 the static method ServletContextHandler.getServletContextHandler(ServletContext) is returning you null? Or are you saying you want access to the threadlocal? If the former, that might be a bug we can fix. If the latter, unlikely we want to expose the thread local.

[grgrzybek]

@janbartel ServletContextHandler.getServletContextHandler(ServletContext) is giving me null and org.eclipse.jetty.ee10.servlet.ServletContextHandler#getServletContextHandler(ServletContext, String) is throwing IllegalStateException.

that's because the passed ServletContext is actually Pax Web's org.ops4j.pax.web.service.spi.servlet.OsgiDynamicServletContext which I need to do some OSGi magic...

In Jetty 10 it's not a problem, because org.eclipse.jetty.server.handler.ContextHandler#getContextHandler() is used and it's first calling org.eclipse.jetty.server.handler.ContextHandler#getCurrentContext() (that uses thread local). This is not the case in Jetty 12.

[janbartel]

@grgrzybek could your OsgiDynamicServletContext extend ServletContextApi?

[grgrzybek]

Unfortunately no, as it's common for pax-web-tomcat, pax-web-jetty and pax-web-undertow.
I can try looking for more hooks to hijack this call, but still I think the thread local could be used (unless you plan to remove it) ;)

[grgrzybek]

I see org.eclipse.jetty.ee10.servlet.ServletContextHandler#getCurrentServletContext() method that checks the thread local. Maybe this little change then?

diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java
index acd3a871dbb..32c524d8cb3 100644
--- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java
+++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java
@@ -159,6 +159,9 @@ public class ServletContextHandler extends ContextHandler implements Graceful
     {
         if (servletContext instanceof ServletContextApi servletContextApi)
             return servletContextApi.getContext().getServletContextHandler();
+        ServletContextHandler currentHandler = getCurrentServletContextHandler();
+        if (currentHandler != null)
+            return currentHandler;
         throw new IllegalStateException("No Jetty ServletContextHandler, " + purpose + " unavailable");
     }
 
@@ -166,6 +169,9 @@ public class ServletContextHandler extends ContextHandler implements Graceful
     {
         if (servletContext instanceof ServletContextApi)
             return ((ServletContextApi)servletContext).getContext().getServletContextHandler();
+        ServletContextHandler currentHandler = getCurrentServletContextHandler();
+        if (currentHandler != null)
+            return currentHandler;
         return null;
     }

Or add org.eclipse.jetty.ee10.servlet.ServletContextHandler#getCurrentServletContextHandler() call in:

CdiServletContainerInitializer.onStartup(Set<Class<?>>, ServletContext)  (org.eclipse.jetty.ee10.cdi)
DebugListener.contextInitialized(ServletContextEvent)  (org.eclipse.jetty.ee10.servlet)
getContentTypeFromRequest(String, HttpServletRequest) in MimeTypeContentServlet in GzipHandlerTest  (org.eclipse.jetty.ee10.servlet)
JettyWebSocketFilterTest.testCustomUpgradeFilter()  (org.eclipse.jetty.ee10.websocket.tests)
JettyWebSocketFilterTest.testMultipleWebSocketUpgradeFilter()  (org.eclipse.jetty.ee10.websocket.tests)
JettyWebSocketServlet.init()  (org.eclipse.jetty.ee10.websocket.server)
WebSocketUpgradeFilter.ensureFilter(ServletContext)  (org.eclipse.jetty.ee10.websocket.servlet)
WebSocketUpgradeFilter.getFilter(ServletContext)  (org.eclipse.jetty.ee10.websocket.servlet)
WebSocketUpgradeFilter.init(FilterConfig)  (org.eclipse.jetty.ee10.websocket.servlet)

JakartaWebSocketServerContainer.ensureContainer(ServletContext)  (org.eclipse.jetty.ee10.websocket.jakarta.server)
JakartaWebSocketServletContainerInitializer.onStartup(Set<Class<?>>, ServletContext)  (org.eclipse.jetty.ee10.websocket.jakarta.server.config)
JettyWebSocketServerContainer.ensureContainer(ServletContext)  (org.eclipse.jetty.ee10.websocket.server)
JettyWebSocketServletContainerInitializer.onStartup(Set<Class<?>>, ServletContext)  (org.eclipse.jetty.ee10.websocket.server.config)

[janbartel]

@grgrzybek hope that PR #9933 works for you, as it's been merged.

[grgrzybek]

@janbartel thanks, but ... almost ;) WebSocket related code is using the 2-arg version:

public static ServletContextHandler getServletContextHandler(ServletContext servletContext, String purpose)

and I'd still get an exception.

[janbartel]

@grgrzybek ok, committed PR #9941 hopefully that does the trick.

[grgrzybek]

That should do it! Thanks! I'll confirm on Monday, ok?

[grgrzybek]

Thanks - I confirmed it works fine!n You can close it.