From f23094ca7dd4772b9425b57a267efa23db992f0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20R=C3=BCtter?= Date: Mon, 30 Dec 2024 10:44:18 +0100 Subject: [PATCH 01/14] FELIX-6746 JettyWebSocketServlet: Error during calling init() - Add reproducer in integration test --- .../jetty/it/JettySpecificWebsocketIT.java | 46 +++++++++++++++++-- 1 file changed, 43 insertions(+), 3 deletions(-) diff --git a/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/JettySpecificWebsocketIT.java b/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/JettySpecificWebsocketIT.java index 63503f6e2b..097dc83fe3 100644 --- a/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/JettySpecificWebsocketIT.java +++ b/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/JettySpecificWebsocketIT.java @@ -20,10 +20,12 @@ import static org.junit.Assert.assertNotNull; import static org.ops4j.pax.exam.CoreOptions.mavenBundle; import static org.ops4j.pax.exam.cm.ConfigurationAdminOptions.newConfiguration; +import static org.osgi.service.servlet.whiteboard.HttpWhiteboardConstants.HTTP_WHITEBOARD_SERVLET_PATTERN; import java.io.IOException; import java.net.URI; import java.time.Duration; +import java.util.Dictionary; import java.util.Hashtable; import java.util.Map; import java.util.concurrent.CompletableFuture; @@ -39,6 +41,8 @@ import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.client.transport.HttpClientTransportOverHTTP; import org.eclipse.jetty.ee10.websocket.server.JettyWebSocketServerContainer; +import org.eclipse.jetty.ee10.websocket.server.JettyWebSocketServlet; +import org.eclipse.jetty.ee10.websocket.server.JettyWebSocketServletFactory; import org.eclipse.jetty.websocket.api.Callback; import org.eclipse.jetty.websocket.api.Session; import org.eclipse.jetty.websocket.api.annotations.OnWebSocketMessage; @@ -54,7 +58,6 @@ import org.ops4j.pax.exam.spi.reactors.PerClass; import org.osgi.framework.BundleContext; import org.osgi.service.http.HttpService; -import org.osgi.service.servlet.whiteboard.HttpWhiteboardConstants; /** * @@ -100,12 +103,11 @@ protected Option felixHttpConfig(int httpPort) { .asOption(); } - @Test public void testWebSocketConversation() throws Exception { assertNotNull(bundleContext); bundleContext.registerService(Servlet.class, new MyWebSocketInitServlet(), new Hashtable<>(Map.of( - HttpWhiteboardConstants.HTTP_WHITEBOARD_SERVLET_PATTERN, "/mywebsocket1" + HTTP_WHITEBOARD_SERVLET_PATTERN, "/mywebsocket1" ))); HttpClientTransportOverHTTP transport = new HttpClientTransportOverHTTP(); @@ -134,6 +136,44 @@ public void testWebSocketConversation() throws Exception { assertEquals("Hello WebSocket", clientWebSocket.getLastMessage()); } + @Test + public void testWebSocketServletWhiteboard() throws Exception { + final JettyWebSocketServlet webSocketServlet2 = new JettyWebSocketServlet() { + @Override + protected void configure(JettyWebSocketServletFactory jettyWebSocketServletFactory) { + jettyWebSocketServletFactory.register(MyServerWebSocket.class); + } + }; + final Dictionary webSocketServletProps2 = new Hashtable<>(); + webSocketServletProps2.put(HTTP_WHITEBOARD_SERVLET_PATTERN, "/websocketservlet/*"); + bundleContext.registerService(Servlet.class, webSocketServlet2, webSocketServletProps2); + + HttpClientTransportOverHTTP transport = new HttpClientTransportOverHTTP(); + HttpClient httpClient = new org.eclipse.jetty.client.HttpClient(transport); + WebSocketClient webSocketClient = new WebSocketClient(httpClient); + webSocketClient.start(); + + Object value = bundleContext.getServiceReference(HttpService.class).getProperty("org.osgi.service.http.port"); + int httpPort = Integer.parseInt((String)value); + URI destUri = new URI(String.format("ws://localhost:%d/websocketservlet/test", httpPort)); + + MyClientWebSocket clientWebSocket = new MyClientWebSocket(); + ClientUpgradeRequest request = new ClientUpgradeRequest(); + CompletableFuture future = webSocketClient.connect(clientWebSocket, destUri, request); + Session session = future.get(); + assertNotNull(session); + + // send a message from the client to the server + clientWebSocket.sendMessage("Hello WebSocket"); + + // wait for the async response from the server + Awaitility.await("waitForResponse") + .atMost(Duration.ofSeconds(30)) + .pollDelay(Duration.ofMillis(200)) + .until(() -> clientWebSocket.getLastMessage() != null); + assertEquals("Hello WebSocket", clientWebSocket.getLastMessage()); + } + /** * A servlet that declares the websocket during init */ From 0de03ea42d170aa982cafdccaf9b059596313e96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20R=C3=BCtter?= Date: Mon, 30 Dec 2024 11:36:52 +0100 Subject: [PATCH 02/14] FELIX-6746 JettyWebSocketServlet: Error during calling init() - Call init lazy in case of servlets that extend JettyWebSocketServlet - This fixes the IT as well --- .../base/internal/handler/ServletHandler.java | 54 +++++++++++++++---- .../jetty/it/JettySpecificWebsocketIT.java | 10 ++-- 2 files changed, 49 insertions(+), 15 deletions(-) diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java index 5a9c97a0ac..1291c7a8a0 100644 --- a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java +++ b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java @@ -42,6 +42,9 @@ public abstract class ServletHandler implements Comparable private static final String JAVA_SERVLET_TEMP_DIR_PROP = "jakarta.servlet.content.tempdir"; + // The Jetty class used for Jetty WebSocket servlets + private static final String JETTY_WEB_SOCKET_SERVLET_CLASS = "JettyWebSocketServlet"; + private final long contextServiceId; private final ServletInfo servletInfo; @@ -52,6 +55,8 @@ public abstract class ServletHandler implements Comparable protected volatile int useCount; + private boolean isInitialized = false; + private final MultipartConfig mpConfig; public ServletHandler(final long contextServiceId, @@ -125,6 +130,16 @@ public void handle(final ServletRequest req, final ServletResponse res) final Servlet local = this.servlet; if ( local != null ) { + if (!isInitialized){ + // Lazy init if needed + try { + servlet.init(new ServletConfigImpl(getName(), getContext(), getServletInfo().getInitParameters())); + isInitialized = true; + } catch (final Exception e) { + SystemLogger.LOGGER.error(SystemLogger.formatMessage(this.getServletInfo().getServiceReference(), + "Error calling init() lazy on servlet ".concat(this.servletInfo.getClassName(this.servlet))), + e);} + } local.service(req, res); } else @@ -173,21 +188,40 @@ public int init() return DTOConstants.FAILURE_REASON_SERVICE_NOT_GETTABLE; } - try - { - servlet.init(new ServletConfigImpl(getName(), getContext(), getServletInfo().getInitParameters())); - } - catch (final Exception e) - { - SystemLogger.LOGGER.error(SystemLogger.formatMessage(this.getServletInfo().getServiceReference(), - "Error during calling init() on servlet ".concat(this.servletInfo.getClassName(this.servlet))), - e); - return DTOConstants.FAILURE_REASON_EXCEPTION_ON_INIT; + if (!isJettyWebSocketServlet()) { + try { + servlet.init(new ServletConfigImpl(getName(), getContext(), getServletInfo().getInitParameters())); + isInitialized = true; + } catch (final Exception e) { + SystemLogger.LOGGER.error(SystemLogger.formatMessage(this.getServletInfo().getServiceReference(), + "Error during calling init() on servlet ".concat(this.servletInfo.getClassName(this.servlet))), + e); + return DTOConstants.FAILURE_REASON_EXCEPTION_ON_INIT; + } } this.useCount++; return -1; } + /** + * Check if the servlet is a JettyWebSocketServlet. + * JettyWebSocket classes are handled differently due to FELIX-6746 + * @return true if the servlet is a JettyWebSocketServlet, false otherwise + */ + private boolean isJettyWebSocketServlet() { + final Class superClass = servlet.getClass().getSuperclass(); + SystemLogger.LOGGER.debug("Checking if the servlet is a JettyWebSocketServlet: '" + superClass.getSimpleName() + "'"); + + // Now check if the servlet class extends 'JettyWebSocketServlet' + boolean isJettyWebSocketServlet = superClass.getSimpleName().endsWith(JETTY_WEB_SOCKET_SERVLET_CLASS); + if (!isJettyWebSocketServlet && servlet instanceof org.apache.felix.http.javaxwrappers.ServletWrapper) { + // In case the servlet is wrapped, we need to check the wrapped servlet + isJettyWebSocketServlet = ((org.apache.felix.http.javaxwrappers.ServletWrapper) servlet) + .getServlet().getClass().getSuperclass().getSimpleName().endsWith(JETTY_WEB_SOCKET_SERVLET_CLASS); + } + return isJettyWebSocketServlet; + } + public boolean destroy() { diff --git a/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/JettySpecificWebsocketIT.java b/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/JettySpecificWebsocketIT.java index 097dc83fe3..7f883370c5 100644 --- a/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/JettySpecificWebsocketIT.java +++ b/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/JettySpecificWebsocketIT.java @@ -138,15 +138,15 @@ public void testWebSocketConversation() throws Exception { @Test public void testWebSocketServletWhiteboard() throws Exception { - final JettyWebSocketServlet webSocketServlet2 = new JettyWebSocketServlet() { + final JettyWebSocketServlet webSocketServlet = new JettyWebSocketServlet() { @Override protected void configure(JettyWebSocketServletFactory jettyWebSocketServletFactory) { jettyWebSocketServletFactory.register(MyServerWebSocket.class); } }; - final Dictionary webSocketServletProps2 = new Hashtable<>(); - webSocketServletProps2.put(HTTP_WHITEBOARD_SERVLET_PATTERN, "/websocketservlet/*"); - bundleContext.registerService(Servlet.class, webSocketServlet2, webSocketServletProps2); + final Dictionary props = new Hashtable<>(); + props.put(HTTP_WHITEBOARD_SERVLET_PATTERN, "/websocketservlet"); + bundleContext.registerService(Servlet.class, webSocketServlet, props); HttpClientTransportOverHTTP transport = new HttpClientTransportOverHTTP(); HttpClient httpClient = new org.eclipse.jetty.client.HttpClient(transport); @@ -155,7 +155,7 @@ protected void configure(JettyWebSocketServletFactory jettyWebSocketServletFacto Object value = bundleContext.getServiceReference(HttpService.class).getProperty("org.osgi.service.http.port"); int httpPort = Integer.parseInt((String)value); - URI destUri = new URI(String.format("ws://localhost:%d/websocketservlet/test", httpPort)); + URI destUri = new URI(String.format("ws://localhost:%d/websocketservlet", httpPort)); MyClientWebSocket clientWebSocket = new MyClientWebSocket(); ClientUpgradeRequest request = new ClientUpgradeRequest(); From 1a94da05a2adacf0789ad36d63938265e1f3c158 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20R=C3=BCtter?= Date: Mon, 30 Dec 2024 12:20:29 +0100 Subject: [PATCH 03/14] FELIX-6746 JettyWebSocketServlet: Error during calling init() - Extract test methods - Remove ServletWrapper support, as it doesn't work properly yet --- .../base/internal/handler/ServletHandler.java | 8 +---- .../jetty/it/JettySpecificWebsocketIT.java | 33 ++++--------------- http/samples/whiteboard/pom.xml | 2 +- 3 files changed, 9 insertions(+), 34 deletions(-) diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java index 1291c7a8a0..e1c8b38431 100644 --- a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java +++ b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java @@ -213,13 +213,7 @@ private boolean isJettyWebSocketServlet() { SystemLogger.LOGGER.debug("Checking if the servlet is a JettyWebSocketServlet: '" + superClass.getSimpleName() + "'"); // Now check if the servlet class extends 'JettyWebSocketServlet' - boolean isJettyWebSocketServlet = superClass.getSimpleName().endsWith(JETTY_WEB_SOCKET_SERVLET_CLASS); - if (!isJettyWebSocketServlet && servlet instanceof org.apache.felix.http.javaxwrappers.ServletWrapper) { - // In case the servlet is wrapped, we need to check the wrapped servlet - isJettyWebSocketServlet = ((org.apache.felix.http.javaxwrappers.ServletWrapper) servlet) - .getServlet().getClass().getSuperclass().getSimpleName().endsWith(JETTY_WEB_SOCKET_SERVLET_CLASS); - } - return isJettyWebSocketServlet; + return superClass.getSimpleName().endsWith(JETTY_WEB_SOCKET_SERVLET_CLASS); } diff --git a/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/JettySpecificWebsocketIT.java b/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/JettySpecificWebsocketIT.java index 7f883370c5..78d7a8b235 100644 --- a/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/JettySpecificWebsocketIT.java +++ b/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/JettySpecificWebsocketIT.java @@ -110,30 +110,7 @@ public void testWebSocketConversation() throws Exception { HTTP_WHITEBOARD_SERVLET_PATTERN, "/mywebsocket1" ))); - HttpClientTransportOverHTTP transport = new HttpClientTransportOverHTTP(); - HttpClient httpClient = new org.eclipse.jetty.client.HttpClient(transport); - WebSocketClient webSocketClient = new WebSocketClient(httpClient); - webSocketClient.start(); - - Object value = bundleContext.getServiceReference(HttpService.class).getProperty("org.osgi.service.http.port"); - int httpPort = Integer.parseInt((String)value); - URI destUri = new URI(String.format("ws://localhost:%d/mywebsocket1", httpPort)); - - MyClientWebSocket clientWebSocket = new MyClientWebSocket(); - ClientUpgradeRequest request = new ClientUpgradeRequest(); - CompletableFuture future = webSocketClient.connect(clientWebSocket, destUri, request); - Session session = future.get(); - assertNotNull(session); - - // send a message from the client to the server - clientWebSocket.sendMessage("Hello WebSocket"); - - // wait for the async response from the server - Awaitility.await("waitForResponse") - .atMost(Duration.ofSeconds(30)) - .pollDelay(Duration.ofMillis(200)) - .until(() -> clientWebSocket.getLastMessage() != null); - assertEquals("Hello WebSocket", clientWebSocket.getLastMessage()); + assertWebSocketResponse("mywebsocket1"); } @Test @@ -145,9 +122,13 @@ protected void configure(JettyWebSocketServletFactory jettyWebSocketServletFacto } }; final Dictionary props = new Hashtable<>(); - props.put(HTTP_WHITEBOARD_SERVLET_PATTERN, "/websocketservlet"); + props.put(HTTP_WHITEBOARD_SERVLET_PATTERN, "/websocketservletwhiteboard"); bundleContext.registerService(Servlet.class, webSocketServlet, props); + assertWebSocketResponse("websocketservletwhiteboard"); + } + + private void assertWebSocketResponse(String servletPath) throws Exception { HttpClientTransportOverHTTP transport = new HttpClientTransportOverHTTP(); HttpClient httpClient = new org.eclipse.jetty.client.HttpClient(transport); WebSocketClient webSocketClient = new WebSocketClient(httpClient); @@ -155,7 +136,7 @@ protected void configure(JettyWebSocketServletFactory jettyWebSocketServletFacto Object value = bundleContext.getServiceReference(HttpService.class).getProperty("org.osgi.service.http.port"); int httpPort = Integer.parseInt((String)value); - URI destUri = new URI(String.format("ws://localhost:%d/websocketservlet", httpPort)); + URI destUri = new URI(String.format("ws://localhost:%d/" + servletPath, httpPort)); MyClientWebSocket clientWebSocket = new MyClientWebSocket(); ClientUpgradeRequest request = new ClientUpgradeRequest(); diff --git a/http/samples/whiteboard/pom.xml b/http/samples/whiteboard/pom.xml index 3cb20fa90b..aa08f324e0 100644 --- a/http/samples/whiteboard/pom.xml +++ b/http/samples/whiteboard/pom.xml @@ -96,7 +96,7 @@ org.apache.felix org.apache.felix.http.jetty12 - 1.0.5-SNAPSHOT + 1.0.20-SNAPSHOT provided From 6044d22b63059ecd92248a07d1c883084e7fb4a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20R=C3=BCtter?= Date: Mon, 30 Dec 2024 16:24:01 +0100 Subject: [PATCH 04/14] FELIX-6746 JettyWebSocketServlet: Error during calling init() - Make solution thread-safe - Fix destroy logic - Minor test improvements --- .../base/internal/handler/ServletHandler.java | 75 ++++++++++++------- .../jetty/it/JettySpecificWebsocketIT.java | 2 +- 2 files changed, 49 insertions(+), 28 deletions(-) diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java index e1c8b38431..32daf78855 100644 --- a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java +++ b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java @@ -18,6 +18,8 @@ import java.io.File; import java.io.IOException; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.atomic.AtomicBoolean; import org.apache.felix.http.base.internal.context.ExtServletContext; import org.apache.felix.http.base.internal.dispatch.MultipartConfig; @@ -55,7 +57,8 @@ public abstract class ServletHandler implements Comparable protected volatile int useCount; - private boolean isInitialized = false; + private final AtomicBoolean lazyFirstInitCall = new AtomicBoolean(true); + private final CountDownLatch initBarrier = new CountDownLatch(1); private final MultipartConfig mpConfig; @@ -130,16 +133,9 @@ public void handle(final ServletRequest req, final ServletResponse res) final Servlet local = this.servlet; if ( local != null ) { - if (!isInitialized){ - // Lazy init if needed - try { - servlet.init(new ServletConfigImpl(getName(), getContext(), getServletInfo().getInitParameters())); - isInitialized = true; - } catch (final Exception e) { - SystemLogger.LOGGER.error(SystemLogger.formatMessage(this.getServletInfo().getServiceReference(), - "Error calling init() lazy on servlet ".concat(this.servletInfo.getClassName(this.servlet))), - e);} - } + // Lazy init if needed, thread-safe + lazyInit(); + local.service(req, res); } else @@ -191,32 +187,19 @@ public int init() if (!isJettyWebSocketServlet()) { try { servlet.init(new ServletConfigImpl(getName(), getContext(), getServletInfo().getInitParameters())); - isInitialized = true; } catch (final Exception e) { SystemLogger.LOGGER.error(SystemLogger.formatMessage(this.getServletInfo().getServiceReference(), "Error during calling init() on servlet ".concat(this.servletInfo.getClassName(this.servlet))), e); return DTOConstants.FAILURE_REASON_EXCEPTION_ON_INIT; + } finally { + initBarrier.countDown(); } } this.useCount++; return -1; } - /** - * Check if the servlet is a JettyWebSocketServlet. - * JettyWebSocket classes are handled differently due to FELIX-6746 - * @return true if the servlet is a JettyWebSocketServlet, false otherwise - */ - private boolean isJettyWebSocketServlet() { - final Class superClass = servlet.getClass().getSuperclass(); - SystemLogger.LOGGER.debug("Checking if the servlet is a JettyWebSocketServlet: '" + superClass.getSimpleName() + "'"); - - // Now check if the servlet class extends 'JettyWebSocketServlet' - return superClass.getSimpleName().endsWith(JETTY_WEB_SOCKET_SERVLET_CLASS); - } - - public boolean destroy() { if (this.servlet == null) @@ -229,7 +212,9 @@ public boolean destroy() { try { - servlet.destroy(); + if (!isJettyWebSocketServlet() || !lazyFirstInitCall.get()) { + servlet.destroy(); + } } catch ( final Exception ignore ) { @@ -281,4 +266,40 @@ public Bundle getMultipartSecurityContext() { return null; } + + /** + * Check if the servlet is a JettyWebSocketServlet. + * JettyWebSocket classes are handled differently due to FELIX-6746 + * @return true if the servlet is a JettyWebSocketServlet, false otherwise + */ + private boolean isJettyWebSocketServlet() { + final Class superClass = servlet.getClass().getSuperclass(); + SystemLogger.LOGGER.debug("Checking if the servlet is a JettyWebSocketServlet: '" + superClass.getSimpleName() + "'"); + + // Now check if the servlet class extends 'JettyWebSocketServlet' + return superClass.getSimpleName().endsWith(JETTY_WEB_SOCKET_SERVLET_CLASS); + } + + /* + * Lazy initialization of the servlet. + * Will only be called once for each servlet instance and is thread-safe. + */ + private void lazyInit() { + if (lazyFirstInitCall.compareAndSet(true, false)) { + try { + servlet.init(new ServletConfigImpl(getName(), getContext(), getServletInfo().getInitParameters())); + } catch (final Exception e) { + SystemLogger.LOGGER.error(SystemLogger.formatMessage(this.getServletInfo().getServiceReference(), + "Error calling init() lazy on servlet ".concat(this.servletInfo.getClassName(this.servlet))), e); + } finally { + initBarrier.countDown(); + } + } else { + try { + initBarrier.await(); + } catch (final InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + } } diff --git a/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/JettySpecificWebsocketIT.java b/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/JettySpecificWebsocketIT.java index 78d7a8b235..5b891a3134 100644 --- a/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/JettySpecificWebsocketIT.java +++ b/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/JettySpecificWebsocketIT.java @@ -136,7 +136,7 @@ private void assertWebSocketResponse(String servletPath) throws Exception { Object value = bundleContext.getServiceReference(HttpService.class).getProperty("org.osgi.service.http.port"); int httpPort = Integer.parseInt((String)value); - URI destUri = new URI(String.format("ws://localhost:%d/" + servletPath, httpPort)); + URI destUri = new URI(String.format("ws://localhost:%d/%s", httpPort, servletPath)); MyClientWebSocket clientWebSocket = new MyClientWebSocket(); ClientUpgradeRequest request = new ClientUpgradeRequest(); From 93f2d2045bebf5937c9b11afcd94370d560c1804 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20R=C3=BCtter?= Date: Mon, 30 Dec 2024 16:41:20 +0100 Subject: [PATCH 05/14] FELIX-6746 JettyWebSocketServlet: Error during calling init() - Add support for HttpService / ServletWrappers - Add IT to guard it --- .../base/internal/handler/ServletHandler.java | 22 ++++++++++++++----- .../jetty/it/JettySpecificWebsocketIT.java | 18 +++++++++++++++ 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java index 32daf78855..60945ee8aa 100644 --- a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java +++ b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java @@ -184,7 +184,7 @@ public int init() return DTOConstants.FAILURE_REASON_SERVICE_NOT_GETTABLE; } - if (!isJettyWebSocketServlet()) { + if (!isJettyWebSocketServlet(servlet)) { try { servlet.init(new ServletConfigImpl(getName(), getContext(), getServletInfo().getInitParameters())); } catch (final Exception e) { @@ -212,7 +212,7 @@ public boolean destroy() { try { - if (!isJettyWebSocketServlet() || !lazyFirstInitCall.get()) { + if (!isJettyWebSocketServlet(servlet) || !lazyFirstInitCall.get()) { servlet.destroy(); } } @@ -269,15 +269,27 @@ public Bundle getMultipartSecurityContext() /** * Check if the servlet is a JettyWebSocketServlet. - * JettyWebSocket classes are handled differently due to FELIX-6746 + * JettyWebSocket classes are handled differently due to FELIX-6746. + * @param servlet the servlet to check * @return true if the servlet is a JettyWebSocketServlet, false otherwise */ - private boolean isJettyWebSocketServlet() { + private static boolean isJettyWebSocketServlet(Object servlet) { final Class superClass = servlet.getClass().getSuperclass(); SystemLogger.LOGGER.debug("Checking if the servlet is a JettyWebSocketServlet: '" + superClass.getSimpleName() + "'"); // Now check if the servlet class extends 'JettyWebSocketServlet' - return superClass.getSimpleName().endsWith(JETTY_WEB_SOCKET_SERVLET_CLASS); + boolean isJettyWebSocketServlet = superClass.getSimpleName().endsWith(JETTY_WEB_SOCKET_SERVLET_CLASS); + if (!isJettyWebSocketServlet) { + // Recurse through the wrapped servlets, in case of double-wrapping + if (servlet instanceof org.apache.felix.http.jakartawrappers.ServletWrapper) { + final javax.servlet.Servlet wrappedServlet = ((org.apache.felix.http.jakartawrappers.ServletWrapper) servlet).getServlet(); + return isJettyWebSocketServlet(wrappedServlet); + } else if (servlet instanceof org.apache.felix.http.javaxwrappers.ServletWrapper) { + final jakarta.servlet.Servlet wrappedServlet = ((org.apache.felix.http.javaxwrappers.ServletWrapper) servlet).getServlet(); + return isJettyWebSocketServlet(wrappedServlet); + } + } + return isJettyWebSocketServlet; } /* diff --git a/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/JettySpecificWebsocketIT.java b/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/JettySpecificWebsocketIT.java index 5b891a3134..7302ccf2f0 100644 --- a/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/JettySpecificWebsocketIT.java +++ b/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/JettySpecificWebsocketIT.java @@ -37,6 +37,7 @@ import jakarta.servlet.ServletException; import jakarta.servlet.http.HttpServlet; +import org.apache.felix.http.javaxwrappers.ServletWrapper; import org.awaitility.Awaitility; import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.client.transport.HttpClientTransportOverHTTP; @@ -69,6 +70,9 @@ public class JettySpecificWebsocketIT extends AbstractJettyTestSupport { @Inject protected BundleContext bundleContext; + @Inject + protected HttpService httpService; + @Override protected Option[] additionalOptions() throws IOException { String jettyVersion = System.getProperty("jetty.version", JETTY_VERSION); @@ -128,6 +132,20 @@ protected void configure(JettyWebSocketServletFactory jettyWebSocketServletFacto assertWebSocketResponse("websocketservletwhiteboard"); } + @Test + public void testWebSocketServletHttpService() throws Exception { + final JettyWebSocketServlet webSocketServlet = new JettyWebSocketServlet() { + @Override + protected void configure(JettyWebSocketServletFactory jettyWebSocketServletFactory) { + jettyWebSocketServletFactory.register(MyServerWebSocket.class); + } + }; + + httpService.registerServlet("/websocketservlethttpservice", new ServletWrapper(webSocketServlet), null, null); + + assertWebSocketResponse("websocketservletwhiteboard"); + } + private void assertWebSocketResponse(String servletPath) throws Exception { HttpClientTransportOverHTTP transport = new HttpClientTransportOverHTTP(); HttpClient httpClient = new org.eclipse.jetty.client.HttpClient(transport); From bb22e52046a7ee4f673bf9c67edee35836bdaea5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20R=C3=BCtter?= Date: Mon, 30 Dec 2024 16:45:42 +0100 Subject: [PATCH 06/14] FELIX-6746 JettyWebSocketServlet: Error during calling init() - Update whiteboard examples; the FelixJettyWebSocketServlet is no longer needed with these fixes --- .../FelixJettyWebSocketServlet.java | 72 ------------------- .../TestWebSocketServletAlternative.java | 5 +- 2 files changed, 3 insertions(+), 74 deletions(-) delete mode 100644 http/samples/whiteboard/src/main/java/org/apache/felix/http/samples/whiteboard/FelixJettyWebSocketServlet.java diff --git a/http/samples/whiteboard/src/main/java/org/apache/felix/http/samples/whiteboard/FelixJettyWebSocketServlet.java b/http/samples/whiteboard/src/main/java/org/apache/felix/http/samples/whiteboard/FelixJettyWebSocketServlet.java deleted file mode 100644 index 55af0f1863..0000000000 --- a/http/samples/whiteboard/src/main/java/org/apache/felix/http/samples/whiteboard/FelixJettyWebSocketServlet.java +++ /dev/null @@ -1,72 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.felix.http.samples.whiteboard; - -import java.io.IOException; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.atomic.AtomicBoolean; - -import jakarta.servlet.ServletException; -import jakarta.servlet.ServletRequest; -import jakarta.servlet.ServletResponse; - -import org.eclipse.jetty.ee10.websocket.server.JettyWebSocketServlet; - -/** - * Abstract class that hides all Jetty Websocket specifics and provides a way for the developer to focus on the actual WebSocket implementation. - */ -public abstract class FelixJettyWebSocketServlet extends JettyWebSocketServlet { - private final AtomicBoolean myFirstInitCall = new AtomicBoolean(true); - private final CountDownLatch myInitBarrier = new CountDownLatch(1); - - public final void init() { - // nothing, see delayed init below in service method - // this is a workaround as stated in https://issues.apache.org/jira/browse/FELIX-5310 - } - - @Override - public void service(final ServletRequest req, final ServletResponse res) throws ServletException, IOException { - if (myFirstInitCall.compareAndSet(true, false)) { - try { - super.init(); - } finally { - myInitBarrier.countDown(); - } - } else { - try { - myInitBarrier.await(); - } catch (final InterruptedException e) { - throw new ServletException("Timed out waiting for initialisation", e); - } - } - - super.service(req, res); - } - - /** - * Cleanup method. - */ - @Override - public final void destroy() { - // only call destroy when the servlet has been initialized - if (!myFirstInitCall.get()) { - // This is required because WebSocketServlet needs to have it's destroy() method called as well - // Causes NPE otherwise when calling an WS endpoint - super.destroy(); - } - } -} diff --git a/http/samples/whiteboard/src/main/java/org/apache/felix/http/samples/whiteboard/TestWebSocketServletAlternative.java b/http/samples/whiteboard/src/main/java/org/apache/felix/http/samples/whiteboard/TestWebSocketServletAlternative.java index e916a9a3ff..133fc1b4fc 100644 --- a/http/samples/whiteboard/src/main/java/org/apache/felix/http/samples/whiteboard/TestWebSocketServletAlternative.java +++ b/http/samples/whiteboard/src/main/java/org/apache/felix/http/samples/whiteboard/TestWebSocketServletAlternative.java @@ -19,6 +19,7 @@ import jakarta.servlet.ServletConfig; import jakarta.servlet.ServletException; +import org.eclipse.jetty.ee10.websocket.server.JettyWebSocketServlet; import org.eclipse.jetty.ee10.websocket.server.JettyWebSocketServletFactory; import org.eclipse.jetty.websocket.api.Callback; import org.eclipse.jetty.websocket.api.Session; @@ -30,10 +31,10 @@ /** * Example of a WebSocket servlet that uses the Jetty WebSocket API, and is registered by extending JettyWebSocketServlet. - * It does respect the path this servlet is registered to, but requires a further workaround. See FelixJettyWebSocketServlet. + * It does respect the path this servlet is registered to. * Requires setting `org.apache.felix.jetty.websocket.enable=true`. */ -public class TestWebSocketServletAlternative extends FelixJettyWebSocketServlet { +public class TestWebSocketServletAlternative extends JettyWebSocketServlet { private final String name; public TestWebSocketServletAlternative(String name) { From 37254fd6a4ac9c4542eaedd39417d68c6bc1a18a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20R=C3=BCtter?= Date: Mon, 30 Dec 2024 17:18:54 +0100 Subject: [PATCH 07/14] FELIX-6746 JettyWebSocketServlet: Error during calling init() - Disable HTTP service test, as it fails with "java.lang.IllegalStateException: JettyServerFrameHandlerFactory not found" --- .../felix/http/jetty/it/JettySpecificWebsocketIT.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/JettySpecificWebsocketIT.java b/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/JettySpecificWebsocketIT.java index 7302ccf2f0..3ea4f57024 100644 --- a/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/JettySpecificWebsocketIT.java +++ b/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/JettySpecificWebsocketIT.java @@ -132,7 +132,9 @@ protected void configure(JettyWebSocketServletFactory jettyWebSocketServletFacto assertWebSocketResponse("websocketservletwhiteboard"); } - @Test + // @Test + // This test is disabled because it fails with a NPE in the JettyWebSocketServletFactory + // java.lang.IllegalStateException: JettyServerFrameHandlerFactory not found public void testWebSocketServletHttpService() throws Exception { final JettyWebSocketServlet webSocketServlet = new JettyWebSocketServlet() { @Override @@ -143,7 +145,7 @@ protected void configure(JettyWebSocketServletFactory jettyWebSocketServletFacto httpService.registerServlet("/websocketservlethttpservice", new ServletWrapper(webSocketServlet), null, null); - assertWebSocketResponse("websocketservletwhiteboard"); + assertWebSocketResponse("websocketservlethttpservice"); } private void assertWebSocketResponse(String servletPath) throws Exception { From 95e594c024a2584f3fc081ba853901112969bde1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20R=C3=BCtter?= Date: Mon, 30 Dec 2024 19:06:10 +0100 Subject: [PATCH 08/14] FELIX-6746 JettyWebSocketServlet: Error during calling init() - Fix passing along shared websocket attributes to the HttpContext in SharedHttpServiceImpl, when registerServlet is called - Enable test again --- .../base/internal/HttpServiceController.java | 1 + .../internal/service/HttpServiceFactory.java | 12 ++++++++++ .../service/SharedHttpServiceImpl.java | 24 +++++++++++++++++++ .../whiteboard/WhiteboardManager.java | 4 ++-- .../jetty/it/JettySpecificWebsocketIT.java | 4 +--- 5 files changed, 40 insertions(+), 5 deletions(-) diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/HttpServiceController.java b/http/base/src/main/java/org/apache/felix/http/base/internal/HttpServiceController.java index 96439f5b1a..e8658faa84 100644 --- a/http/base/src/main/java/org/apache/felix/http/base/internal/HttpServiceController.java +++ b/http/base/src/main/java/org/apache/felix/http/base/internal/HttpServiceController.java @@ -145,6 +145,7 @@ public void register(@NotNull final ServletContext containerContext, @NotNull fi */ public void setAttributeSharedServletContext(String key, Object value) { this.whiteboardManager.setAttributeSharedServletContext(key, value); + this.httpServiceFactory.setAttributeSharedServletContext(key, value); } /** diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/service/HttpServiceFactory.java b/http/base/src/main/java/org/apache/felix/http/base/internal/service/HttpServiceFactory.java index 7ff37e0309..c5f7b1d8a3 100644 --- a/http/base/src/main/java/org/apache/felix/http/base/internal/service/HttpServiceFactory.java +++ b/http/base/src/main/java/org/apache/felix/http/base/internal/service/HttpServiceFactory.java @@ -16,10 +16,14 @@ */ package org.apache.felix.http.base.internal.service; +import java.util.HashMap; import java.util.Hashtable; +import java.util.Map; +import org.apache.felix.http.base.internal.logger.SystemLogger; import org.apache.felix.http.base.internal.registry.HandlerRegistry; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.osgi.framework.Bundle; import org.osgi.framework.BundleContext; import org.osgi.framework.Constants; @@ -76,6 +80,7 @@ public final class HttpServiceFactory private final HandlerRegistry handlerRegistry; private volatile SharedHttpServiceImpl sharedHttpService; + private volatile Map attributesForSharedContext = new HashMap<>(); public HttpServiceFactory(final BundleContext bundleContext, final HandlerRegistry handlerRegistry) @@ -101,6 +106,7 @@ public void start(final ServletContext context, this.context = context; this.sharedHttpService = new SharedHttpServiceImpl(handlerRegistry); + this.sharedHttpService.setSharedContextAttributes(attributesForSharedContext); this.active = true; this.httpServiceReg = bundleContext.registerService(HttpService.class, this, this.httpServiceProps); @@ -120,6 +126,7 @@ public void stop() this.sharedHttpService = null; this.httpServiceProps.clear(); + this.attributesForSharedContext.clear(); } @Override @@ -160,6 +167,11 @@ public long getHttpServiceServiceId() return (Long) this.httpServiceReg.getReference().getProperty(Constants.SERVICE_ID); } + public void setAttributeSharedServletContext(String key, Object value) { + SystemLogger.LOGGER.info("HttpServiceFactory: Storing attribute for shared servlet context. Key '{}', value: '{}'", key, value); + this.attributesForSharedContext.put(key, value); + } + private boolean getBoolean(final String property) { String prop = this.bundleContext.getProperty(property); diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/service/SharedHttpServiceImpl.java b/http/base/src/main/java/org/apache/felix/http/base/internal/service/SharedHttpServiceImpl.java index d96c656375..b2e9001d43 100644 --- a/http/base/src/main/java/org/apache/felix/http/base/internal/service/SharedHttpServiceImpl.java +++ b/http/base/src/main/java/org/apache/felix/http/base/internal/service/SharedHttpServiceImpl.java @@ -23,6 +23,7 @@ import org.apache.felix.http.base.internal.context.ExtServletContext; import org.apache.felix.http.base.internal.handler.HttpServiceServletHandler; import org.apache.felix.http.base.internal.handler.ServletHandler; +import org.apache.felix.http.base.internal.logger.SystemLogger; import org.apache.felix.http.base.internal.registry.HandlerRegistry; import org.apache.felix.http.base.internal.runtime.ServletInfo; import org.apache.felix.http.jakartawrappers.ServletWrapper; @@ -38,6 +39,7 @@ public final class SharedHttpServiceImpl private final HandlerRegistry handlerRegistry; private final Map aliasMap = new HashMap<>(); + private Map attributesForSharedContext; /** * Create a new implementation @@ -69,6 +71,9 @@ public void registerServlet(@NotNull final String alias, { final ServletHandler handler = new HttpServiceServletHandler(httpContext, servletInfo, servlet); + // Track properties from shared context + setAttributes(httpContext); + synchronized (this.aliasMap) { if (this.aliasMap.containsKey(alias)) @@ -155,4 +160,23 @@ public void unregisterServlet(final javax.servlet.Servlet servlet) { return this.handlerRegistry; } + + public void setSharedContextAttributes(Map attributesForSharedContext) { + this.attributesForSharedContext = attributesForSharedContext; + } + + /** + * Set the stored attributes on the servlet context. + * @param context the servlet context + */ + private void setAttributes(ExtServletContext context) { + if (context != null) { + attributesForSharedContext.forEach((key, value) -> { + if (key != null && value != null) { + SystemLogger.LOGGER.info("SharedHttpServiceImpl: Shared context found, setting stored attribute key: '{}', value: '{}'", key, value); + context.setAttribute(key, value); + } + }); + } + } } diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/whiteboard/WhiteboardManager.java b/http/base/src/main/java/org/apache/felix/http/base/internal/whiteboard/WhiteboardManager.java index cb48cb9a53..cecabb0775 100644 --- a/http/base/src/main/java/org/apache/felix/http/base/internal/whiteboard/WhiteboardManager.java +++ b/http/base/src/main/java/org/apache/felix/http/base/internal/whiteboard/WhiteboardManager.java @@ -416,7 +416,7 @@ private void setAttributes(@Nullable ServletContext context) { if (context != null) { attributesForSharedContext.forEach((key, value) -> { if (key != null && value != null) { - SystemLogger.LOGGER.info("Shared context found, setting stored attribute key: '{}', value: '{}'", key, value); + SystemLogger.LOGGER.info("WhiteboardManager: Shared context found, setting stored attribute key: '{}', value: '{}'", key, value); context.setAttribute(key, value); } }); @@ -981,7 +981,7 @@ private void updateRuntimeChangeCount() * @param value attribute value */ public void setAttributeSharedServletContext(String key, Object value) { - SystemLogger.LOGGER.info("Storing attribute for shared servlet context. Key '{}', value: '{}'", key, value); + SystemLogger.LOGGER.info("WhiteboardManager: Storing attribute for shared servlet context. Key '{}', value: '{}'", key, value); this.attributesForSharedContext.put(key, value); } } diff --git a/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/JettySpecificWebsocketIT.java b/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/JettySpecificWebsocketIT.java index 3ea4f57024..27b33bc9d0 100644 --- a/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/JettySpecificWebsocketIT.java +++ b/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/JettySpecificWebsocketIT.java @@ -132,9 +132,7 @@ protected void configure(JettyWebSocketServletFactory jettyWebSocketServletFacto assertWebSocketResponse("websocketservletwhiteboard"); } - // @Test - // This test is disabled because it fails with a NPE in the JettyWebSocketServletFactory - // java.lang.IllegalStateException: JettyServerFrameHandlerFactory not found + @Test public void testWebSocketServletHttpService() throws Exception { final JettyWebSocketServlet webSocketServlet = new JettyWebSocketServlet() { @Override From 24edc751a3f429bfb9c998e34514b888c43aecff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20R=C3=BCtter?= Date: Mon, 30 Dec 2024 19:11:50 +0100 Subject: [PATCH 09/14] FELIX-6746 JettyWebSocketServlet: Error during calling init() - Add NPE check --- .../felix/http/base/internal/service/SharedHttpServiceImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/service/SharedHttpServiceImpl.java b/http/base/src/main/java/org/apache/felix/http/base/internal/service/SharedHttpServiceImpl.java index b2e9001d43..ea54f58b7a 100644 --- a/http/base/src/main/java/org/apache/felix/http/base/internal/service/SharedHttpServiceImpl.java +++ b/http/base/src/main/java/org/apache/felix/http/base/internal/service/SharedHttpServiceImpl.java @@ -170,7 +170,7 @@ public void setSharedContextAttributes(Map attributesForSharedCo * @param context the servlet context */ private void setAttributes(ExtServletContext context) { - if (context != null) { + if (context != null && attributesForSharedContext != null) { attributesForSharedContext.forEach((key, value) -> { if (key != null && value != null) { SystemLogger.LOGGER.info("SharedHttpServiceImpl: Shared context found, setting stored attribute key: '{}', value: '{}'", key, value); From a05cdb0b874eeda78439785f3f8b6582f3b8f2b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20R=C3=BCtter?= Date: Fri, 3 Jan 2025 16:38:24 +0100 Subject: [PATCH 10/14] Update README.md --- http/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/http/README.md b/http/README.md index 38585ff5d8..77f1cacc5f 100644 --- a/http/README.md +++ b/http/README.md @@ -442,6 +442,7 @@ properties can be used (some legacy property names still exist but are not docum | `org.apache.felix.jetty.websocket.enable` | Enables Jetty websocket support. Default is false. | | `org.apache.felix.http.jetty.threadpool.max` | The maximum number of threads in the Jetty thread pool. Default is unlimited. Works for both platform threads and virtual threads (Jetty 12 only). | | `org.apache.felix.http.jetty.virtualthreads.enable` | Enables using virtual threads in Jetty 12 (JDK 21 required). Default is false. When enabled, `org.apache.felix.http.jetty.threadpool.max` is used for a bounded virtual thread pool. | + ### Multiple Servers It is possible to configure several Http Services, each running on a different port. The first service can be configured as outlined above using the service PID for `"org.apache.felix.http"`. Additional servers can be configured through OSGi factory configurations using `"org.apache.felix.http"` as the factory PID. The properties for the configuration are outlined as above. From bb16933c80c2feae13e63b780d3192b161e5627c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20R=C3=BCtter?= Date: Tue, 21 Jan 2025 23:02:51 +0100 Subject: [PATCH 11/14] FELIX-6746-websocketservlet-init-NPE - Move logic into subclass --- .../handler/HttpServiceServletHandler.java | 2 +- .../HttpServiceWebSocketServletHandler.java | 65 ++++++++++++++ .../base/internal/handler/ServletHandler.java | 81 +++-------------- .../internal/handler/WebSocketHandler.java | 87 +++++++++++++++++++ .../handler/WhiteboardServletHandler.java | 2 +- .../WhiteboardWebSocketServletHandler.java | 81 +++++++++++++++++ .../service/SharedHttpServiceImpl.java | 19 +++- .../base/internal/util/WebSocketUtil.java | 54 ++++++++++++ .../whiteboard/WhiteboardManager.java | 36 ++++++-- 9 files changed, 346 insertions(+), 81 deletions(-) create mode 100644 http/base/src/main/java/org/apache/felix/http/base/internal/handler/HttpServiceWebSocketServletHandler.java create mode 100644 http/base/src/main/java/org/apache/felix/http/base/internal/handler/WebSocketHandler.java create mode 100644 http/base/src/main/java/org/apache/felix/http/base/internal/handler/WhiteboardWebSocketServletHandler.java create mode 100644 http/base/src/main/java/org/apache/felix/http/base/internal/util/WebSocketUtil.java diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/HttpServiceServletHandler.java b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/HttpServiceServletHandler.java index 255ad55551..d84a0e120e 100644 --- a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/HttpServiceServletHandler.java +++ b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/HttpServiceServletHandler.java @@ -26,7 +26,7 @@ /** * Servlet handler for servlets registered through the http service. */ -public final class HttpServiceServletHandler extends ServletHandler +public class HttpServiceServletHandler extends ServletHandler { /** * New handler diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/HttpServiceWebSocketServletHandler.java b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/HttpServiceWebSocketServletHandler.java new file mode 100644 index 0000000000..8d99962538 --- /dev/null +++ b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/HttpServiceWebSocketServletHandler.java @@ -0,0 +1,65 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.felix.http.base.internal.handler; + +import java.io.IOException; + +import jakarta.servlet.ServletException; +import jakarta.servlet.ServletRequest; +import jakarta.servlet.ServletResponse; + +import org.apache.felix.http.base.internal.context.ExtServletContext; +import org.apache.felix.http.base.internal.runtime.ServletInfo; + +/** + * Servlet handler for servlets extending JettyWebSocketServlet registered through the http service. + */ +public final class HttpServiceWebSocketServletHandler extends HttpServiceServletHandler +{ + private final WebSocketHandler webSocketHandler; + + public HttpServiceWebSocketServletHandler(final ExtServletContext context, + final ServletInfo servletInfo, + final javax.servlet.Servlet servlet) + { + super(context, servletInfo, servlet); + this.webSocketHandler = new WebSocketHandler(this); + } + + @Override + public int init() { + if (webSocketHandler.shouldInit()) { + return super.init(); + } + // do nothing, delay init until first service call + return -1; + } + + @Override + public void handle(ServletRequest req, ServletResponse res) throws ServletException, IOException { + this.webSocketHandler.lazyInit(); + super.handle(req, res); + } + + @Override + public boolean destroy() { + if (webSocketHandler.shouldDestroy()) { + return super.destroy(); + } + return false; + } +} diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java index 60945ee8aa..b961bd9a86 100644 --- a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java +++ b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java @@ -44,9 +44,6 @@ public abstract class ServletHandler implements Comparable private static final String JAVA_SERVLET_TEMP_DIR_PROP = "jakarta.servlet.content.tempdir"; - // The Jetty class used for Jetty WebSocket servlets - private static final String JETTY_WEB_SOCKET_SERVLET_CLASS = "JettyWebSocketServlet"; - private final long contextServiceId; private final ServletInfo servletInfo; @@ -57,9 +54,6 @@ public abstract class ServletHandler implements Comparable protected volatile int useCount; - private final AtomicBoolean lazyFirstInitCall = new AtomicBoolean(true); - private final CountDownLatch initBarrier = new CountDownLatch(1); - private final MultipartConfig mpConfig; public ServletHandler(final long contextServiceId, @@ -133,9 +127,6 @@ public void handle(final ServletRequest req, final ServletResponse res) final Servlet local = this.servlet; if ( local != null ) { - // Lazy init if needed, thread-safe - lazyInit(); - local.service(req, res); } else @@ -184,17 +175,15 @@ public int init() return DTOConstants.FAILURE_REASON_SERVICE_NOT_GETTABLE; } - if (!isJettyWebSocketServlet(servlet)) { - try { - servlet.init(new ServletConfigImpl(getName(), getContext(), getServletInfo().getInitParameters())); - } catch (final Exception e) { - SystemLogger.LOGGER.error(SystemLogger.formatMessage(this.getServletInfo().getServiceReference(), - "Error during calling init() on servlet ".concat(this.servletInfo.getClassName(this.servlet))), - e); - return DTOConstants.FAILURE_REASON_EXCEPTION_ON_INIT; - } finally { - initBarrier.countDown(); - } + try + { + servlet.init(new ServletConfigImpl(getName(), getContext(), getServletInfo().getInitParameters())); + } + catch (final Exception e) { + SystemLogger.LOGGER.error(SystemLogger.formatMessage(this.getServletInfo().getServiceReference(), + "Error during calling init() on servlet ".concat(this.servletInfo.getClassName(this.servlet))), + e); + return DTOConstants.FAILURE_REASON_EXCEPTION_ON_INIT; } this.useCount++; return -1; @@ -212,9 +201,7 @@ public boolean destroy() { try { - if (!isJettyWebSocketServlet(servlet) || !lazyFirstInitCall.get()) { - servlet.destroy(); - } + servlet.destroy(); } catch ( final Exception ignore ) { @@ -266,52 +253,4 @@ public Bundle getMultipartSecurityContext() { return null; } - - /** - * Check if the servlet is a JettyWebSocketServlet. - * JettyWebSocket classes are handled differently due to FELIX-6746. - * @param servlet the servlet to check - * @return true if the servlet is a JettyWebSocketServlet, false otherwise - */ - private static boolean isJettyWebSocketServlet(Object servlet) { - final Class superClass = servlet.getClass().getSuperclass(); - SystemLogger.LOGGER.debug("Checking if the servlet is a JettyWebSocketServlet: '" + superClass.getSimpleName() + "'"); - - // Now check if the servlet class extends 'JettyWebSocketServlet' - boolean isJettyWebSocketServlet = superClass.getSimpleName().endsWith(JETTY_WEB_SOCKET_SERVLET_CLASS); - if (!isJettyWebSocketServlet) { - // Recurse through the wrapped servlets, in case of double-wrapping - if (servlet instanceof org.apache.felix.http.jakartawrappers.ServletWrapper) { - final javax.servlet.Servlet wrappedServlet = ((org.apache.felix.http.jakartawrappers.ServletWrapper) servlet).getServlet(); - return isJettyWebSocketServlet(wrappedServlet); - } else if (servlet instanceof org.apache.felix.http.javaxwrappers.ServletWrapper) { - final jakarta.servlet.Servlet wrappedServlet = ((org.apache.felix.http.javaxwrappers.ServletWrapper) servlet).getServlet(); - return isJettyWebSocketServlet(wrappedServlet); - } - } - return isJettyWebSocketServlet; - } - - /* - * Lazy initialization of the servlet. - * Will only be called once for each servlet instance and is thread-safe. - */ - private void lazyInit() { - if (lazyFirstInitCall.compareAndSet(true, false)) { - try { - servlet.init(new ServletConfigImpl(getName(), getContext(), getServletInfo().getInitParameters())); - } catch (final Exception e) { - SystemLogger.LOGGER.error(SystemLogger.formatMessage(this.getServletInfo().getServiceReference(), - "Error calling init() lazy on servlet ".concat(this.servletInfo.getClassName(this.servlet))), e); - } finally { - initBarrier.countDown(); - } - } else { - try { - initBarrier.await(); - } catch (final InterruptedException e) { - Thread.currentThread().interrupt(); - } - } - } } diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/WebSocketHandler.java b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/WebSocketHandler.java new file mode 100644 index 0000000000..02481208d6 --- /dev/null +++ b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/WebSocketHandler.java @@ -0,0 +1,87 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.felix.http.base.internal.handler; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.atomic.AtomicBoolean; + +import org.apache.felix.http.base.internal.logger.SystemLogger; + +/** + * Class that handles initialization for servlets extending JettyWebSocketServlet. + */ +public final class WebSocketHandler { + private final AtomicBoolean lazyFirstInitCall = new AtomicBoolean(true); + private final CountDownLatch initBarrier = new CountDownLatch(1); + private final ServletHandler servletHandler; + + public WebSocketHandler(ServletHandler servletHandler) { + this.servletHandler = servletHandler; + } + + /* + * Lazy initialization of the servlet. + * Will only be called once for each servlet instance and is thread-safe. + */ + public void lazyInit() { + if (lazyFirstInitCall.compareAndSet(true, false)) { + try { + this.servletHandler.init(); + } catch (final Exception e) { + SystemLogger.LOGGER.error(SystemLogger.formatMessage( + this.servletHandler.getServletInfo().getServiceReference(), + "Error calling init() lazy on servlet ".concat( + this.servletHandler.getServletInfo().getClassName(this.servletHandler.getServlet()))), e); + } finally { + initBarrier.countDown(); + } + } else { + // already initialized, await the first initialization + try { + initBarrier.await(); + } catch (final InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + } + + /** + * Returns true if the servlet should be initialized, false otherwise. + * @return true if the servlet should be initialized, false otherwise + */ + public boolean shouldInit() { + return !lazyFirstInitCall.get() && initBarrier.getCount() > 0; + } + + /** + * Returns true if the servlet was initialized earlier, false otherwise. + * @return true if the servlet should be destroyed, false otherwise + */ + public boolean shouldDestroy() { + if (!lazyFirstInitCall.get()){ + try { + initBarrier.await(); + } catch (final InterruptedException e) { + Thread.currentThread().interrupt(); + } + return true; + } + return false; + } +} diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/WhiteboardServletHandler.java b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/WhiteboardServletHandler.java index 5fae620ab5..8427da5535 100644 --- a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/WhiteboardServletHandler.java +++ b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/WhiteboardServletHandler.java @@ -29,7 +29,7 @@ /** * Servlet handler for servlets registered through the http whiteboard. */ -public final class WhiteboardServletHandler extends ServletHandler +public class WhiteboardServletHandler extends ServletHandler { private final BundleContext bundleContext; diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/WhiteboardWebSocketServletHandler.java b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/WhiteboardWebSocketServletHandler.java new file mode 100644 index 0000000000..d35536f5ff --- /dev/null +++ b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/WhiteboardWebSocketServletHandler.java @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.felix.http.base.internal.handler; + +import java.io.FilePermission; +import java.io.IOException; + +import jakarta.servlet.Servlet; +import jakarta.servlet.ServletException; +import jakarta.servlet.ServletRequest; +import jakarta.servlet.ServletResponse; + +import org.apache.felix.http.base.internal.context.ExtServletContext; +import org.apache.felix.http.base.internal.logger.SystemLogger; +import org.apache.felix.http.base.internal.runtime.ServletInfo; +import org.osgi.framework.Bundle; +import org.osgi.framework.BundleContext; +import org.osgi.service.servlet.runtime.dto.DTOConstants; + +/** + * Servlet handler for servlets extending JettyWebSocketServlet registered through the http whiteboard. + */ +public final class WhiteboardWebSocketServletHandler extends WhiteboardServletHandler +{ + private final WebSocketHandler webSocketHandler; + + public WhiteboardWebSocketServletHandler(final long contextServiceId, + final ExtServletContext context, + final ServletInfo servletInfo, + final BundleContext contextBundleContext, + final Bundle registeringBundle, + final Bundle httpWhiteboardBundle, + final Object servlet) + { + super(contextServiceId, context, servletInfo, contextBundleContext, registeringBundle, httpWhiteboardBundle); + this.webSocketHandler = new WebSocketHandler(this); + this.setServlet((Servlet) servlet); + } + + @Override + public int init() { + SystemLogger.LOGGER.error("WhiteboardWebSocketServletHandler.init()"); + if (webSocketHandler.shouldInit()) { + SystemLogger.LOGGER.error("WhiteboardWebSocketServletHandler.init - called()"); + return super.init(); + } + // do nothing, delay init until first service call + return -1; + } + + @Override + public void handle(ServletRequest req, ServletResponse res) throws ServletException, IOException { + SystemLogger.LOGGER.error("WhiteboardWebSocketServletHandler.handle()"); + this.webSocketHandler.lazyInit(); + super.handle(req, res); + } + + @Override + public boolean destroy() { + SystemLogger.LOGGER.error("WhiteboardWebSocketServletHandler.destroy()"); + if (webSocketHandler.shouldDestroy()) { + SystemLogger.LOGGER.error("WhiteboardWebSocketServletHandler.destroy() - called"); + return super.destroy(); + } + return false; + } +} diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/service/SharedHttpServiceImpl.java b/http/base/src/main/java/org/apache/felix/http/base/internal/service/SharedHttpServiceImpl.java index ea54f58b7a..7f7d967eae 100644 --- a/http/base/src/main/java/org/apache/felix/http/base/internal/service/SharedHttpServiceImpl.java +++ b/http/base/src/main/java/org/apache/felix/http/base/internal/service/SharedHttpServiceImpl.java @@ -16,16 +16,20 @@ */ package org.apache.felix.http.base.internal.service; +import static org.apache.felix.http.base.internal.util.WebSocketUtil.isJettyWebSocketServlet; + import java.util.HashMap; import java.util.Iterator; import java.util.Map; import org.apache.felix.http.base.internal.context.ExtServletContext; import org.apache.felix.http.base.internal.handler.HttpServiceServletHandler; +import org.apache.felix.http.base.internal.handler.HttpServiceWebSocketServletHandler; import org.apache.felix.http.base.internal.handler.ServletHandler; import org.apache.felix.http.base.internal.logger.SystemLogger; import org.apache.felix.http.base.internal.registry.HandlerRegistry; import org.apache.felix.http.base.internal.runtime.ServletInfo; +import org.apache.felix.http.base.internal.util.WebSocketUtil; import org.apache.felix.http.jakartawrappers.ServletWrapper; import org.jetbrains.annotations.NotNull; import org.osgi.service.http.NamespaceException; @@ -69,7 +73,7 @@ public void registerServlet(@NotNull final String alias, @NotNull final javax.servlet.Servlet servlet, @NotNull final ServletInfo servletInfo) throws javax.servlet.ServletException, NamespaceException { - final ServletHandler handler = new HttpServiceServletHandler(httpContext, servletInfo, servlet); + final ServletHandler handler = getServletHandler(httpContext, servlet, servletInfo); // Track properties from shared context setAttributes(httpContext); @@ -86,6 +90,19 @@ public void registerServlet(@NotNull final String alias, } } + @NotNull + private static HttpServiceServletHandler getServletHandler( + @NotNull ExtServletContext httpContext, + @NotNull javax.servlet.Servlet servlet, + @NotNull ServletInfo servletInfo) + { + if (isJettyWebSocketServlet(servlet)) + { + return new HttpServiceWebSocketServletHandler(httpContext, servletInfo, servlet); + } + return new HttpServiceServletHandler(httpContext, servletInfo, servlet); + } + /** * Unregister a servlet * @param alias The alias diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/util/WebSocketUtil.java b/http/base/src/main/java/org/apache/felix/http/base/internal/util/WebSocketUtil.java new file mode 100644 index 0000000000..b0b077fb0c --- /dev/null +++ b/http/base/src/main/java/org/apache/felix/http/base/internal/util/WebSocketUtil.java @@ -0,0 +1,54 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.felix.http.base.internal.util; + +import org.apache.felix.http.base.internal.logger.SystemLogger; + +/** + * Utility methods for WebSocket handling + */ +public abstract class WebSocketUtil { + // The Jetty class used for Jetty WebSocket servlets + private static final String JETTY_WEB_SOCKET_SERVLET_CLASS = "JettyWebSocketServlet"; + + /** + * Check if the servlet is a JettyWebSocketServlet. + * JettyWebSocket classes are handled differently due to FELIX-6746. + * @param servlet the servlet to check + * @return true if the servlet is a JettyWebSocketServlet, false otherwise + */ + public static boolean isJettyWebSocketServlet(Object servlet) { + final Class superClass = servlet.getClass().getSuperclass(); + SystemLogger.LOGGER.debug("Checking if the servlet is a JettyWebSocketServlet: '" + superClass.getSimpleName() + "'"); + + // Now check if the servlet class extends 'JettyWebSocketServlet' + boolean isJettyWebSocketServlet = superClass.getSimpleName().endsWith(JETTY_WEB_SOCKET_SERVLET_CLASS); + if (!isJettyWebSocketServlet) { + // Recurse through the wrapped servlets, in case of double-wrapping + if (servlet instanceof org.apache.felix.http.jakartawrappers.ServletWrapper) { + final javax.servlet.Servlet wrappedServlet = ((org.apache.felix.http.jakartawrappers.ServletWrapper) servlet).getServlet(); + return isJettyWebSocketServlet(wrappedServlet); + } else if (servlet instanceof org.apache.felix.http.javaxwrappers.ServletWrapper) { + final jakarta.servlet.Servlet wrappedServlet = ((org.apache.felix.http.javaxwrappers.ServletWrapper) servlet).getServlet(); + return isJettyWebSocketServlet(wrappedServlet); + } + } + return isJettyWebSocketServlet; + } +} diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/whiteboard/WhiteboardManager.java b/http/base/src/main/java/org/apache/felix/http/base/internal/whiteboard/WhiteboardManager.java index cecabb0775..e5a65658d4 100644 --- a/http/base/src/main/java/org/apache/felix/http/base/internal/whiteboard/WhiteboardManager.java +++ b/http/base/src/main/java/org/apache/felix/http/base/internal/whiteboard/WhiteboardManager.java @@ -16,6 +16,7 @@ */ package org.apache.felix.http.base.internal.whiteboard; +import static org.apache.felix.http.base.internal.util.WebSocketUtil.isJettyWebSocketServlet; import static org.osgi.service.servlet.runtime.dto.DTOConstants.FAILURE_REASON_NO_SERVLET_CONTEXT_MATCHING; import static org.osgi.service.servlet.runtime.dto.DTOConstants.FAILURE_REASON_SHADOWED_BY_OTHER_SERVICE; import static org.osgi.service.servlet.runtime.dto.DTOConstants.FAILURE_REASON_UNKNOWN; @@ -40,6 +41,7 @@ import org.apache.felix.http.base.internal.handler.PreprocessorHandler; import org.apache.felix.http.base.internal.handler.ServletHandler; import org.apache.felix.http.base.internal.handler.WhiteboardServletHandler; +import org.apache.felix.http.base.internal.handler.WhiteboardWebSocketServletHandler; import org.apache.felix.http.base.internal.logger.SystemLogger; import org.apache.felix.http.base.internal.registry.EventListenerRegistry; import org.apache.felix.http.base.internal.registry.HandlerRegistry; @@ -58,6 +60,7 @@ import org.apache.felix.http.base.internal.runtime.dto.ServletContextDTOBuilder; import org.apache.felix.http.base.internal.service.HttpServiceFactory; import org.apache.felix.http.base.internal.service.HttpServiceRuntimeImpl; +import org.apache.felix.http.base.internal.util.WebSocketUtil; import org.apache.felix.http.base.internal.whiteboard.tracker.FilterTracker; import org.apache.felix.http.base.internal.whiteboard.tracker.JavaxFilterTracker; import org.apache.felix.http.base.internal.whiteboard.tracker.JavaxListenersTracker; @@ -726,13 +729,7 @@ private void registerWhiteboardService(final WhiteboardContextHandler handler, f } else { - final ServletHandler servletHandler = new WhiteboardServletHandler( - handler.getContextInfo().getServiceId(), - servletContext, - (ServletInfo)info, - handler.getBundleContext(), - info.getServiceReference().getBundle(), - this.httpBundleContext.getBundle()); + final ServletHandler servletHandler = getServletHandler(handler, info, servletContext); handler.getRegistry().registerServlet(servletHandler); } } @@ -805,6 +802,31 @@ else if ( info instanceof ListenerInfo ) } } + @NotNull + private WhiteboardServletHandler getServletHandler(WhiteboardContextHandler handler, + WhiteboardServiceInfo info, + ExtServletContext servletContext) + { + Object servlet = info.getService(handler.getBundleContext()); + if (isJettyWebSocketServlet(servlet)) + { + return new WhiteboardWebSocketServletHandler( + handler.getContextInfo().getServiceId(), + servletContext, + (ServletInfo) info, + handler.getBundleContext(), + info.getServiceReference().getBundle(), + this.httpBundleContext.getBundle(), servlet); + } + return new WhiteboardServletHandler( + handler.getContextInfo().getServiceId(), + servletContext, + (ServletInfo) info, + handler.getBundleContext(), + info.getServiceReference().getBundle(), + this.httpBundleContext.getBundle()); + } + /** * Unregister whiteboard service from the http service * @param handler Context handler From 7e5f719b343a63d9b315b3f2919f6d5a415a21af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20R=C3=BCtter?= Date: Tue, 21 Jan 2025 23:08:07 +0100 Subject: [PATCH 12/14] FELIX-6746-websocketservlet-init-NPE - Imports --- .../http/base/internal/handler/ServletHandler.java | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java index b961bd9a86..fb456f208a 100644 --- a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java +++ b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java @@ -18,8 +18,11 @@ import java.io.File; import java.io.IOException; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.atomic.AtomicBoolean; + +import jakarta.servlet.Servlet; +import jakarta.servlet.ServletException; +import jakarta.servlet.ServletRequest; +import jakarta.servlet.ServletResponse; import org.apache.felix.http.base.internal.context.ExtServletContext; import org.apache.felix.http.base.internal.dispatch.MultipartConfig; @@ -29,11 +32,6 @@ import org.osgi.framework.Bundle; import org.osgi.service.servlet.runtime.dto.DTOConstants; -import jakarta.servlet.Servlet; -import jakarta.servlet.ServletException; -import jakarta.servlet.ServletRequest; -import jakarta.servlet.ServletResponse; - /** * The servlet handler handles the initialization and destruction of * a servlet. From c849ca47be62105cc4c2dd13a67ea8495aa1087b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20R=C3=BCtter?= Date: Tue, 21 Jan 2025 23:09:51 +0100 Subject: [PATCH 13/14] FELIX-6746-websocketservlet-init-NPE - Minor changes --- .../http/base/internal/handler/ServletHandler.java | 13 +++++++------ .../handler/WhiteboardWebSocketServletHandler.java | 5 ----- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java index fb456f208a..4254ce6a93 100644 --- a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java +++ b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java @@ -19,11 +19,6 @@ import java.io.File; import java.io.IOException; -import jakarta.servlet.Servlet; -import jakarta.servlet.ServletException; -import jakarta.servlet.ServletRequest; -import jakarta.servlet.ServletResponse; - import org.apache.felix.http.base.internal.context.ExtServletContext; import org.apache.felix.http.base.internal.dispatch.MultipartConfig; import org.apache.felix.http.base.internal.logger.SystemLogger; @@ -32,6 +27,11 @@ import org.osgi.framework.Bundle; import org.osgi.service.servlet.runtime.dto.DTOConstants; +import jakarta.servlet.Servlet; +import jakarta.servlet.ServletException; +import jakarta.servlet.ServletRequest; +import jakarta.servlet.ServletResponse; + /** * The servlet handler handles the initialization and destruction of * a servlet. @@ -177,7 +177,8 @@ public int init() { servlet.init(new ServletConfigImpl(getName(), getContext(), getServletInfo().getInitParameters())); } - catch (final Exception e) { + catch (final Exception e) + { SystemLogger.LOGGER.error(SystemLogger.formatMessage(this.getServletInfo().getServiceReference(), "Error during calling init() on servlet ".concat(this.servletInfo.getClassName(this.servlet))), e); diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/WhiteboardWebSocketServletHandler.java b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/WhiteboardWebSocketServletHandler.java index d35536f5ff..dba9de191e 100644 --- a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/WhiteboardWebSocketServletHandler.java +++ b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/WhiteboardWebSocketServletHandler.java @@ -53,9 +53,7 @@ public WhiteboardWebSocketServletHandler(final long contextServiceId, @Override public int init() { - SystemLogger.LOGGER.error("WhiteboardWebSocketServletHandler.init()"); if (webSocketHandler.shouldInit()) { - SystemLogger.LOGGER.error("WhiteboardWebSocketServletHandler.init - called()"); return super.init(); } // do nothing, delay init until first service call @@ -64,16 +62,13 @@ public int init() { @Override public void handle(ServletRequest req, ServletResponse res) throws ServletException, IOException { - SystemLogger.LOGGER.error("WhiteboardWebSocketServletHandler.handle()"); this.webSocketHandler.lazyInit(); super.handle(req, res); } @Override public boolean destroy() { - SystemLogger.LOGGER.error("WhiteboardWebSocketServletHandler.destroy()"); if (webSocketHandler.shouldDestroy()) { - SystemLogger.LOGGER.error("WhiteboardWebSocketServletHandler.destroy() - called"); return super.destroy(); } return false; From 7612655dd7f6b1bcf7c68175b6bbd64f667fb40f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20R=C3=BCtter?= Date: Wed, 22 Jan 2025 08:38:58 +0100 Subject: [PATCH 14/14] FELIX-6746-websocketservlet-init-NPE - Remove util class, move method to handler - Add unit test, add test scope dependency on jetty12 --- http/base/pom.xml | 6 +++ .../internal/handler/WebSocketHandler.java | 28 ++++++++++ .../service/SharedHttpServiceImpl.java | 3 +- .../base/internal/util/WebSocketUtil.java | 54 ------------------- .../whiteboard/WhiteboardManager.java | 4 +- .../handler/WebSocketHandlerTest.java | 52 ++++++++++++++++++ 6 files changed, 89 insertions(+), 58 deletions(-) delete mode 100644 http/base/src/main/java/org/apache/felix/http/base/internal/util/WebSocketUtil.java create mode 100644 http/base/src/test/java/org/apache/felix/http/base/internal/handler/WebSocketHandlerTest.java diff --git a/http/base/pom.xml b/http/base/pom.xml index bd6eeea198..1ae70a29af 100644 --- a/http/base/pom.xml +++ b/http/base/pom.xml @@ -157,5 +157,11 @@ 5.7.0 test + + org.eclipse.jetty.ee10.websocket + jetty-ee10-websocket-jetty-server + 12.0.16 + test + diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/WebSocketHandler.java b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/WebSocketHandler.java index 02481208d6..89b58d7c3f 100644 --- a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/WebSocketHandler.java +++ b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/WebSocketHandler.java @@ -27,6 +27,9 @@ * Class that handles initialization for servlets extending JettyWebSocketServlet. */ public final class WebSocketHandler { + // The Jetty class used for Jetty WebSocket servlets + private static final String JETTY_WEB_SOCKET_SERVLET_CLASS = "JettyWebSocketServlet"; + private final AtomicBoolean lazyFirstInitCall = new AtomicBoolean(true); private final CountDownLatch initBarrier = new CountDownLatch(1); private final ServletHandler servletHandler; @@ -84,4 +87,29 @@ public boolean shouldDestroy() { } return false; } + + /** + * Check if the servlet is a JettyWebSocketServlet. + * JettyWebSocket classes are handled differently due to FELIX-6746. + * @param servlet the servlet to check + * @return true if the servlet is a JettyWebSocketServlet, false otherwise + */ + public static boolean isJettyWebSocketServlet(Object servlet) { + final Class superClass = servlet.getClass().getSuperclass(); + SystemLogger.LOGGER.debug("Checking if the servlet is a JettyWebSocketServlet: '" + superClass.getSimpleName() + "'"); + + // Now check if the servlet class extends 'JettyWebSocketServlet' + boolean isJettyWebSocketServlet = superClass.getSimpleName().endsWith(JETTY_WEB_SOCKET_SERVLET_CLASS); + if (!isJettyWebSocketServlet) { + // Recurse through the wrapped servlets, in case of double-wrapping + if (servlet instanceof org.apache.felix.http.jakartawrappers.ServletWrapper) { + final javax.servlet.Servlet wrappedServlet = ((org.apache.felix.http.jakartawrappers.ServletWrapper) servlet).getServlet(); + return isJettyWebSocketServlet(wrappedServlet); + } else if (servlet instanceof org.apache.felix.http.javaxwrappers.ServletWrapper) { + final jakarta.servlet.Servlet wrappedServlet = ((org.apache.felix.http.javaxwrappers.ServletWrapper) servlet).getServlet(); + return isJettyWebSocketServlet(wrappedServlet); + } + } + return isJettyWebSocketServlet; + } } diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/service/SharedHttpServiceImpl.java b/http/base/src/main/java/org/apache/felix/http/base/internal/service/SharedHttpServiceImpl.java index 7f7d967eae..cb7ed32ead 100644 --- a/http/base/src/main/java/org/apache/felix/http/base/internal/service/SharedHttpServiceImpl.java +++ b/http/base/src/main/java/org/apache/felix/http/base/internal/service/SharedHttpServiceImpl.java @@ -16,7 +16,7 @@ */ package org.apache.felix.http.base.internal.service; -import static org.apache.felix.http.base.internal.util.WebSocketUtil.isJettyWebSocketServlet; +import static org.apache.felix.http.base.internal.handler.WebSocketHandler.isJettyWebSocketServlet; import java.util.HashMap; import java.util.Iterator; @@ -29,7 +29,6 @@ import org.apache.felix.http.base.internal.logger.SystemLogger; import org.apache.felix.http.base.internal.registry.HandlerRegistry; import org.apache.felix.http.base.internal.runtime.ServletInfo; -import org.apache.felix.http.base.internal.util.WebSocketUtil; import org.apache.felix.http.jakartawrappers.ServletWrapper; import org.jetbrains.annotations.NotNull; import org.osgi.service.http.NamespaceException; diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/util/WebSocketUtil.java b/http/base/src/main/java/org/apache/felix/http/base/internal/util/WebSocketUtil.java deleted file mode 100644 index b0b077fb0c..0000000000 --- a/http/base/src/main/java/org/apache/felix/http/base/internal/util/WebSocketUtil.java +++ /dev/null @@ -1,54 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.felix.http.base.internal.util; - -import org.apache.felix.http.base.internal.logger.SystemLogger; - -/** - * Utility methods for WebSocket handling - */ -public abstract class WebSocketUtil { - // The Jetty class used for Jetty WebSocket servlets - private static final String JETTY_WEB_SOCKET_SERVLET_CLASS = "JettyWebSocketServlet"; - - /** - * Check if the servlet is a JettyWebSocketServlet. - * JettyWebSocket classes are handled differently due to FELIX-6746. - * @param servlet the servlet to check - * @return true if the servlet is a JettyWebSocketServlet, false otherwise - */ - public static boolean isJettyWebSocketServlet(Object servlet) { - final Class superClass = servlet.getClass().getSuperclass(); - SystemLogger.LOGGER.debug("Checking if the servlet is a JettyWebSocketServlet: '" + superClass.getSimpleName() + "'"); - - // Now check if the servlet class extends 'JettyWebSocketServlet' - boolean isJettyWebSocketServlet = superClass.getSimpleName().endsWith(JETTY_WEB_SOCKET_SERVLET_CLASS); - if (!isJettyWebSocketServlet) { - // Recurse through the wrapped servlets, in case of double-wrapping - if (servlet instanceof org.apache.felix.http.jakartawrappers.ServletWrapper) { - final javax.servlet.Servlet wrappedServlet = ((org.apache.felix.http.jakartawrappers.ServletWrapper) servlet).getServlet(); - return isJettyWebSocketServlet(wrappedServlet); - } else if (servlet instanceof org.apache.felix.http.javaxwrappers.ServletWrapper) { - final jakarta.servlet.Servlet wrappedServlet = ((org.apache.felix.http.javaxwrappers.ServletWrapper) servlet).getServlet(); - return isJettyWebSocketServlet(wrappedServlet); - } - } - return isJettyWebSocketServlet; - } -} diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/whiteboard/WhiteboardManager.java b/http/base/src/main/java/org/apache/felix/http/base/internal/whiteboard/WhiteboardManager.java index e5a65658d4..7f1cb5f4b6 100644 --- a/http/base/src/main/java/org/apache/felix/http/base/internal/whiteboard/WhiteboardManager.java +++ b/http/base/src/main/java/org/apache/felix/http/base/internal/whiteboard/WhiteboardManager.java @@ -16,7 +16,7 @@ */ package org.apache.felix.http.base.internal.whiteboard; -import static org.apache.felix.http.base.internal.util.WebSocketUtil.isJettyWebSocketServlet; +import static org.apache.felix.http.base.internal.handler.WebSocketHandler.isJettyWebSocketServlet; import static org.osgi.service.servlet.runtime.dto.DTOConstants.FAILURE_REASON_NO_SERVLET_CONTEXT_MATCHING; import static org.osgi.service.servlet.runtime.dto.DTOConstants.FAILURE_REASON_SHADOWED_BY_OTHER_SERVICE; import static org.osgi.service.servlet.runtime.dto.DTOConstants.FAILURE_REASON_UNKNOWN; @@ -40,6 +40,7 @@ import org.apache.felix.http.base.internal.handler.ListenerHandler; import org.apache.felix.http.base.internal.handler.PreprocessorHandler; import org.apache.felix.http.base.internal.handler.ServletHandler; +import org.apache.felix.http.base.internal.handler.WebSocketHandler; import org.apache.felix.http.base.internal.handler.WhiteboardServletHandler; import org.apache.felix.http.base.internal.handler.WhiteboardWebSocketServletHandler; import org.apache.felix.http.base.internal.logger.SystemLogger; @@ -60,7 +61,6 @@ import org.apache.felix.http.base.internal.runtime.dto.ServletContextDTOBuilder; import org.apache.felix.http.base.internal.service.HttpServiceFactory; import org.apache.felix.http.base.internal.service.HttpServiceRuntimeImpl; -import org.apache.felix.http.base.internal.util.WebSocketUtil; import org.apache.felix.http.base.internal.whiteboard.tracker.FilterTracker; import org.apache.felix.http.base.internal.whiteboard.tracker.JavaxFilterTracker; import org.apache.felix.http.base.internal.whiteboard.tracker.JavaxListenersTracker; diff --git a/http/base/src/test/java/org/apache/felix/http/base/internal/handler/WebSocketHandlerTest.java b/http/base/src/test/java/org/apache/felix/http/base/internal/handler/WebSocketHandlerTest.java new file mode 100644 index 0000000000..e635a50f1d --- /dev/null +++ b/http/base/src/test/java/org/apache/felix/http/base/internal/handler/WebSocketHandlerTest.java @@ -0,0 +1,52 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.felix.http.base.internal.handler; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; + +import org.apache.felix.http.javaxwrappers.ServletWrapper; +import org.eclipse.jetty.ee10.websocket.server.JettyWebSocketServlet; +import org.junit.Before; +import org.junit.Test; + +public class WebSocketHandlerTest { + private javax.servlet.Servlet javaxServlet; + private jakarta.servlet.Servlet jakartaServlet; + private JettyWebSocketServlet jakartaWebSocketServlet; + + @Before + public void setUp() + { + this.javaxServlet = mock(javax.servlet.Servlet.class); + this.jakartaServlet = mock(jakarta.servlet.Servlet.class); + this.jakartaWebSocketServlet = mock(JettyWebSocketServlet.class); + } + + @Test + public void isJettyWebSocketServlet(){ + assertFalse(WebSocketHandler.isJettyWebSocketServlet(this.javaxServlet)); + assertFalse(WebSocketHandler.isJettyWebSocketServlet(this.jakartaServlet)); + + // See test scope dependency in pom.xml + assertTrue(WebSocketHandler.isJettyWebSocketServlet(this.jakartaWebSocketServlet)); + + // Also works with the wrapper + assertTrue(WebSocketHandler.isJettyWebSocketServlet(new ServletWrapper(this.jakartaWebSocketServlet))); + } +}