Skip to content

Commit

Permalink
Issue #6287 - fix classloading for WebSocketClient in webapp
Browse files Browse the repository at this point in the history
Signed-off-by: Lachlan Roberts <[email protected]>
  • Loading branch information
lachlan-roberts committed May 26, 2021
1 parent 5a5c011 commit 816b326
Show file tree
Hide file tree
Showing 6 changed files with 69 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,12 @@ public WebSocketClient getClient()
return client;
}

@Override
public ClassLoader getClassLoader()
{
return client.getClassLoader();
}

public EndpointMetadata getClientEndpointMetadata(Class<?> endpoint, EndpointConfig config)
{
synchronized (endpointClientMetadataCache)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
//
// ========================================================================
// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others.
// ========================================================================
// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others.
// ------------------------------------------------------------------------
// All rights reserved. This program and the accompanying materials
// are made available under the terms of the Eclipse Public License v1.0
// and Apache License v2.0 which accompanies this distribution.
//
// This program and the accompanying materials are made available under the
// terms of the Eclipse Public License v. 2.0 which is available at
// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
// which is available at https://www.apache.org/licenses/LICENSE-2.0.
// The Eclipse Public License is available at
// http://www.eclipse.org/legal/epl-v10.html
//
// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
// ========================================================================
// The Apache License v2.0 is available at
// http://www.opensource.org/licenses/apache2.0.php
//
// You may elect to redistribute this code under either of these licenses.
// ========================================================================
//

package org.eclipse.jetty.websocket.jsr356.server;
Expand All @@ -35,6 +40,8 @@
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.io.ByteBufferPool;
import org.eclipse.jetty.util.component.ContainerLifeCycle;
import org.eclipse.jetty.webapp.WebAppContext;
import org.eclipse.jetty.websocket.client.WebSocketClient;
import org.eclipse.jetty.websocket.common.WebSocketSession;
import org.eclipse.jetty.websocket.jsr356.ClientContainer;
import org.eclipse.jetty.xml.XmlConfiguration;
Expand All @@ -48,7 +55,7 @@

public class JavaxClientClassLoaderTest
{
private WebAppTester server;
private final WebAppTester server = new WebAppTester();
private HttpClient httpClient;

@FunctionalInterface
Expand All @@ -59,7 +66,6 @@ interface ThrowingRunnable

public void start(ThrowingRunnable configuration) throws Exception
{
server = new WebAppTester();
configuration.run();
server.start();
httpClient = new HttpClient();
Expand Down Expand Up @@ -129,13 +135,18 @@ public void onMessage(Session session, String message) throws Exception

public WebAppTester.WebApp createWebSocketWebapp(String contextName) throws Exception
{
WebAppTester.WebApp app = server.createWebApp(contextName);
WebAppTester.WebApp app = this.server.createWebApp(contextName);

// We must hide the websocket classes from the webapp if we are to include websocket client jars in WEB-INF/lib.
WebAppContext context = app.getWebAppContext();
context.getServerClasspathPattern().include("org.eclipse.jetty.websocket.");
context.getSystemClasspathPattern().exclude("org.eclipse.jetty.websocket.");

// Copy over the individual jars required for Javax WebSocket.
app.createWebInf();
app.copyLib(WebSocketContainer.class, "websocket-javax-api.jar");
app.copyLib(ServerContainer.class, "websocket-javax-server.jar");
app.copyLib(ClientContainer.class, "websocket-javax-client.jar");
app.copyLib(WebSocketClient.class, "websocket-jetty-client.jar");
app.copyLib(WebSocketSession.class, "websocket-common.jar");
app.copyLib(ContainerLifeCycle.class, "jetty-util.jar");
app.copyLib(Response.class, "jetty-client.jar");
Expand All @@ -158,7 +169,6 @@ public void websocketProvidedByServer() throws Exception
app1.deploy();

WebAppTester.WebApp app2 = server.createWebApp("/echo");
app2.getWebAppContext().setAttribute("org.eclipse.jetty.websocket.jsr356", Boolean.TRUE);
app2.createWebInf();
app2.copyClass(EchoSocket.class);
app2.deploy();
Expand Down Expand Up @@ -189,7 +199,6 @@ public void websocketProvidedByWebApp() throws Exception

// Do not exclude JavaxWebSocketConfiguration for this webapp (we need the websocket server classes).
WebAppTester.WebApp app2 = server.createWebApp("/echo");
app2.getWebAppContext().setAttribute("org.eclipse.jetty.websocket.jsr356", Boolean.TRUE);
app2.createWebInf();
app2.copyClass(EchoSocket.class);
app2.deploy();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
//
// ========================================================================
// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others.
// ========================================================================
// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others.
// ------------------------------------------------------------------------
// All rights reserved. This program and the accompanying materials
// are made available under the terms of the Eclipse Public License v1.0
// and Apache License v2.0 which accompanies this distribution.
//
// This program and the accompanying materials are made available under the
// terms of the Eclipse Public License v. 2.0 which is available at
// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
// which is available at https://www.apache.org/licenses/LICENSE-2.0.
// The Eclipse Public License is available at
// http://www.eclipse.org/legal/epl-v10.html
//
// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
// ========================================================================
// The Apache License v2.0 is available at
// http://www.opensource.org/licenses/apache2.0.php
//
// You may elect to redistribute this code under either of these licenses.
// ========================================================================
//

package org.eclipse.jetty.websocket.jsr356.server;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ public class WebSocketClient extends ContainerLifeCycle implements WebSocketCont
private final Supplier<DecoratedObjectFactory> objectFactorySupplier;

// WebSocket Specifics
private final ClassLoader classloader;
private final WebSocketPolicy policy;
private final WebSocketExtensionFactory extensionRegistry;
private final SessionTracker sessionTracker = new SessionTracker();
Expand Down Expand Up @@ -112,6 +113,7 @@ public WebSocketClient(HttpClient httpClient)
*/
public WebSocketClient(HttpClient httpClient, DecoratedObjectFactory decoratedObjectFactory)
{
this.classloader = Thread.currentThread().getContextClassLoader();
this.httpClient = Objects.requireNonNull(httpClient, "HttpClient");

addBean(httpClient);
Expand Down Expand Up @@ -262,6 +264,7 @@ public WebSocketClient(final WebSocketContainerScope scope, EventDriverFactory e
*/
public WebSocketClient(final WebSocketContainerScope scope, EventDriverFactory eventDriverFactory, SessionFactory sessionFactory, HttpClient httpClient)
{
this.classloader = Thread.currentThread().getContextClassLoader();
this.httpClient = httpClient == null ? HttpClientProvider.get(scope) : httpClient;
addBean(this.httpClient);

Expand Down Expand Up @@ -386,6 +389,12 @@ public Future<Session> connect(Object websocket, URI toUri, ClientUpgradeRequest
return wsReq.sendAsync();
}

@Override
public ClassLoader getClassLoader()
{
return classloader;
}

public void setEventDriverFactory(EventDriverFactory eventDriverFactory)
{
this.eventDriverFactory = eventDriverFactory;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,12 @@ public class WebSocketSession extends ContainerLifeCycle implements Session, Rem
private final WebSocketPolicy policy;
private final AtomicBoolean onCloseCalled = new AtomicBoolean(false);
private final RemoteEndpointFactory remoteEndpointFactory;
private ClassLoader classLoader;
private final ClassLoader classLoader;
private ExtensionFactory extensionFactory;
private String protocolVersion;
private Map<String, String[]> parameterMap = new HashMap<>();
private final Map<String, String[]> parameterMap = new HashMap<>();
private RemoteEndpoint remote;
private IncomingFrames incomingHandler;
private final IncomingFrames incomingHandler;
private OutgoingFrames outgoingHandler;
private UpgradeRequest upgradeRequest;
private UpgradeResponse upgradeResponse;
Expand All @@ -98,7 +98,7 @@ public WebSocketSession(WebSocketContainerScope containerScope, URI requestURI,
Objects.requireNonNull(containerScope, "Container Scope cannot be null");
Objects.requireNonNull(requestURI, "Request URI cannot be null");

this.classLoader = Thread.currentThread().getContextClassLoader();
this.classLoader = containerScope.getClassLoader();
this.containerScope = containerScope;
this.requestURI = requestURI;
this.websocket = websocket;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,20 @@ public interface WebSocketContainerScope
*/
SslContextFactory getSslContextFactory();

/**
* <p>The ClassLoader used to load classes for the WebSocketSession.</p>
* <p>By default this will be the ContextClassLoader at the time this method is called. However this will be overridden
* by the WebSocketClient to use the ContextClassLoader at the time it was created, this is because the
* client uses its own {@link org.eclipse.jetty.util.thread.ThreadPool} so the WebSocketSessions may be created when
* the ContextClassLoader is not set.</p>
*
* @return the classloader.
*/
default ClassLoader getClassLoader()
{
return Thread.currentThread().getContextClassLoader();
}

/**
* Test for if the container has been started.
*
Expand Down

0 comments on commit 816b326

Please sign in to comment.