Skip to content

Commit

Permalink
FELIX-6746-websocketservlet-init-NPE
Browse files Browse the repository at this point in the history
- Remove util class, move method to handler
- Add unit test, add test scope dependency on jetty12
  • Loading branch information
paulrutter committed Jan 22, 2025
1 parent c849ca4 commit 7612655
Show file tree
Hide file tree
Showing 6 changed files with 89 additions and 58 deletions.
6 changes: 6 additions & 0 deletions http/base/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -157,5 +157,11 @@
<version>5.7.0</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.eclipse.jetty.ee10.websocket</groupId>
<artifactId>jetty-ee10-websocket-jetty-server</artifactId>
<version>12.0.16</version>
<scope>test</scope>
</dependency>
</dependencies>
</project>
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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)));
}
}

0 comments on commit 7612655

Please sign in to comment.