Issue #5320 - using jetty-websocket-httpclient.xml within webapp

jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/extensions/ExtensionFactory.java

@@ -32,17 +32,26 @@
 @Deprecated
 public abstract class ExtensionFactory implements Iterable<Class<? extends Extension>>
 {
-    private ServiceLoader<Extension> extensionLoader = ServiceLoader.load(Extension.class);
-    private Map<String, Class<? extends Extension>> availableExtensions;
+    private final Map<String, Class<? extends Extension>> availableExtensions;
 
     public ExtensionFactory()
     {
         availableExtensions = new HashMap<>();
-        for (Extension ext : extensionLoader)
+        Iterator<Extension> iterator = ServiceLoader.load(Extension.class).iterator();
+        while (true)
         {
-            if (ext != null)
+            try
             {
-                availableExtensions.put(ext.getName(), ext.getClass());
+                if (!iterator.hasNext())
+                    break;
+
+                Extension ext = iterator.next();
+                if (ext != null)
+                    availableExtensions.put(ext.getName(), ext.getClass());
+            }
+            catch (Throwable ignored)
+            {
+                // Ignored.
             }
         }
     }

jetty-websocket/websocket-client/pom.xml

@@ -24,6 +24,7 @@
       <groupId>org.eclipse.jetty</groupId>
       <artifactId>jetty-xml</artifactId>
       <version>${project.version}</version>
+      <optional>true</optional>
     </dependency>
     <dependency>
       <groupId>org.eclipse.jetty</groupId>

jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/HttpClientProvider.java

@@ -18,33 +18,16 @@
 
 package org.eclipse.jetty.websocket.client;
 
-import java.lang.reflect.Method;
-
 import org.eclipse.jetty.client.HttpClient;
-import org.eclipse.jetty.util.log.Log;
 import org.eclipse.jetty.websocket.common.scopes.WebSocketContainerScope;
 
 public final class HttpClientProvider
 {
     public static HttpClient get(WebSocketContainerScope scope)
     {
-        try
-        {
-            if (Class.forName("org.eclipse.jetty.xml.XmlConfiguration") != null)
-            {
-                Class<?> xmlClazz = Class.forName("org.eclipse.jetty.websocket.client.XmlBasedHttpClientProvider");
-                Method getMethod = xmlClazz.getMethod("get", WebSocketContainerScope.class);
-                Object ret = getMethod.invoke(null, scope);
-                if ((ret != null) && (ret instanceof HttpClient))
-                {
-                    return (HttpClient)ret;
-                }
-            }
-        }
-        catch (Throwable ignore)
-        {
-            Log.getLogger(HttpClientProvider.class).ignore(ignore);
-        }
+        HttpClient httpClient = XmlBasedHttpClientProvider.get(scope);
+        if (httpClient != null)
+            return httpClient;
 
         return DefaultHttpClientProvider.newHttpClient(scope);
     }

jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/XmlBasedHttpClientProvider.java

@@ -22,27 +22,46 @@
 
 import org.eclipse.jetty.client.HttpClient;
 import org.eclipse.jetty.util.log.Log;
+import org.eclipse.jetty.util.log.Logger;
+import org.eclipse.jetty.util.resource.Resource;
 import org.eclipse.jetty.websocket.common.scopes.WebSocketContainerScope;
 import org.eclipse.jetty.xml.XmlConfiguration;
 
 class XmlBasedHttpClientProvider
 {
+    public static final Logger LOG = Log.getLogger(XmlBasedHttpClientProvider.class);
+
     public static HttpClient get(@SuppressWarnings("unused") WebSocketContainerScope scope)
     {
-        URL resource = Thread.currentThread().getContextClassLoader().getResource("jetty-websocket-httpclient.xml");
+        ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader();
+        if (contextClassLoader == null)
+            return null;
+
+        URL resource = contextClassLoader.getResource("jetty-websocket-httpclient.xml");
         if (resource == null)
-        {
             return null;
+
+        try
+        {
+            Thread.currentThread().setContextClassLoader(HttpClient.class.getClassLoader());
+            return newHttpClient(resource);
+        }
+        finally
+        {
+            Thread.currentThread().setContextClassLoader(contextClassLoader);
         }
+    }
 
+    private static HttpClient newHttpClient(URL resource)
+    {
         try
         {
-            XmlConfiguration configuration = new XmlConfiguration(resource);
+            XmlConfiguration configuration = new XmlConfiguration(Resource.newResource(resource));
             return (HttpClient)configuration.configure();
         }
         catch (Throwable t)
         {
-            Log.getLogger(XmlBasedHttpClientProvider.class).warn("Unable to load: " + resource, t);
+            LOG.warn("Failure to load HttpClient from XML {}", resource, t);
         }
 
         return null;

jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/extensions/WebSocketExtensionFactory.java

@@ -19,11 +19,6 @@
 package org.eclipse.jetty.websocket.common.extensions;
 
 import java.io.IOException;
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.Map;
-import java.util.ServiceLoader;
-import java.util.Set;
 import java.util.zip.Deflater;
 
 import org.eclipse.jetty.util.StringUtil;
@@ -42,10 +37,8 @@
 
 public class WebSocketExtensionFactory extends ExtensionFactory implements LifeCycle, Dumpable
 {
-    private ContainerLifeCycle containerLifeCycle;
-    private WebSocketContainerScope container;
-    private ServiceLoader<Extension> extensionLoader = ServiceLoader.load(Extension.class);
-    private Map<String, Class<? extends Extension>> availableExtensions;
+    private final ContainerLifeCycle containerLifeCycle;
+    private final WebSocketContainerScope container;
     private final InflaterPool inflaterPool = new InflaterPool(CompressionPool.INFINITE_CAPACITY, true);
     private final DeflaterPool deflaterPool = new DeflaterPool(CompressionPool.INFINITE_CAPACITY, Deflater.DEFAULT_COMPRESSION, true);
 
@@ -59,42 +52,12 @@ public String toString()
                 return String.format("%s@%x{%s}", WebSocketExtensionFactory.class.getSimpleName(), hashCode(), containerLifeCycle.getState());
             }
         };
-        availableExtensions = new HashMap<>();
-        for (Extension ext : extensionLoader)
-        {
-            if (ext != null)
-                availableExtensions.put(ext.getName(), ext.getClass());
-        }
 
         this.container = container;
         containerLifeCycle.addBean(inflaterPool);
         containerLifeCycle.addBean(deflaterPool);
     }
 
-    @Override
-    public Map<String, Class<? extends Extension>> getAvailableExtensions()
-    {
-        return availableExtensions;
-    }
-
-    @Override
-    public Class<? extends Extension> getExtension(String name)
-    {
-        return availableExtensions.get(name);
-    }
-
-    @Override
-    public Set<String> getExtensionNames()
-    {
-        return availableExtensions.keySet();
-    }
-
-    @Override
-    public boolean isAvailable(String name)
-    {
-        return availableExtensions.containsKey(name);
-    }
-
     @Override
     public Extension newInstance(ExtensionConfig config)
     {
@@ -139,24 +102,6 @@ public Extension newInstance(ExtensionConfig config)
         }
     }
 
-    @Override
-    public void register(String name, Class<? extends Extension> extension)
-    {
-        availableExtensions.put(name, extension);
-    }
-
-    @Override
-    public void unregister(String name)
-    {
-        availableExtensions.remove(name);
-    }
-
-    @Override
-    public Iterator<Class<? extends Extension>> iterator()
-    {
-        return availableExtensions.values().iterator();
-    }
-
     /* --- All of the below ugliness due to not being able to break API compatibility with ExtensionFactory --- */
 
     @Override

tests/test-distribution/pom.xml

@@ -117,6 +117,18 @@
       <version>${project.version}</version>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.eclipse.jetty.websocket</groupId>
+      <artifactId>websocket-api</artifactId>
+      <version>${project.version}</version>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.eclipse.jetty.websocket</groupId>
+      <artifactId>websocket-client</artifactId>
+      <version>${project.version}</version>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>org.eclipse.jetty.tests</groupId>
       <artifactId>test-felix-webapp</artifactId>

tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/AbstractDistributionTest.java

@@ -21,6 +21,7 @@
 import java.util.function.Supplier;
 
 import org.eclipse.jetty.client.HttpClient;
+import org.eclipse.jetty.util.ssl.SslContextFactory;
 import org.junit.jupiter.api.AfterEach;
 
 public class AbstractDistributionTest
@@ -29,7 +30,15 @@ public class AbstractDistributionTest
 
     protected void startHttpClient() throws Exception
     {
-        startHttpClient(HttpClient::new);
+        startHttpClient(false);
+    }
+
+    protected void startHttpClient(boolean secure) throws Exception
+    {
+        if (secure)
+            startHttpClient(() -> new HttpClient(new SslContextFactory.Client(true)));
+        else
+            startHttpClient(HttpClient::new);
     }
 
     protected void startHttpClient(Supplier<HttpClient> supplier) throws Exception

tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java

@@ -19,6 +19,7 @@
 package org.eclipse.jetty.tests.distribution;
 
 import java.io.File;
+import java.net.URI;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -39,6 +40,8 @@
 import org.junit.jupiter.api.condition.DisabledOnOs;
 import org.junit.jupiter.api.condition.JRE;
 import org.junit.jupiter.api.condition.OS;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsString;
@@ -304,4 +307,103 @@ public void testLog4j2ModuleWithSimpleWebAppWithJSP() throws Exception
             IO.delete(jettyBase.toFile());
         }
     }
+
+    @ParameterizedTest
+    @ValueSource(strings = {"http", "https"})
+    public void testWebsocketClientInWebappProvidedByServer(String scheme) throws Exception
+    {
+        Path jettyBase = Files.createTempDirectory("jetty_base");
+        String jettyVersion = System.getProperty("jettyVersion");
+        DistributionTester distribution = DistributionTester.Builder.newInstance()
+            .jettyVersion(jettyVersion)
+            .jettyBase(jettyBase)
+            .mavenLocalRepository(System.getProperty("mavenRepoPath"))
+            .build();
+
+        String[] args1 = {
+            "--create-startd",
+            "--approve-all-licenses",
+            "--add-to-start=resources,server,webapp,deploy,jsp,jmx,servlet,servlets,websocket," + scheme
+        };
+        try (DistributionTester.Run run1 = distribution.start(args1))
+        {
+            assertTrue(run1.awaitFor(5, TimeUnit.SECONDS));
+            assertEquals(0, run1.getExitValue());
+
+            File webApp = distribution.resolveArtifact("org.eclipse.jetty.tests:test-websocket-client-provided-webapp:war:" + jettyVersion);
+            distribution.installWarFile(webApp, "test");
+
+            int port = distribution.freePort();
+            String[] args2 = {
+                "jetty.http.port=" + port,
+                "jetty.ssl.port=" + port,
+                // "jetty.server.dumpAfterStart=true",
+            };
+
+            try (DistributionTester.Run run2 = distribution.start(args2))
+            {
+                assertTrue(run2.awaitConsoleLogsFor("Started @", 10, TimeUnit.SECONDS));
+
+                // We should get the correct configuration from the jetty-websocket-httpclient.xml file.
+                startHttpClient(scheme.equals("https"));
+                URI serverUri = URI.create(scheme + "://localhost:" + port + "/test");
+                ContentResponse response = client.GET(serverUri);
+                assertEquals(HttpStatus.OK_200, response.getStatus());
+                String content = response.getContentAsString();
+                assertThat(content, containsString("WebSocketEcho: success"));
+                assertThat(content, containsString("ConnectTimeout: 4999"));
+            }
+        }
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = {"http", "https"})
+    public void testWebsocketClientInWebapp(String scheme) throws Exception
+    {
+        Path jettyBase = Files.createTempDirectory("jetty_base");
+        String jettyVersion = System.getProperty("jettyVersion");
+        DistributionTester distribution = DistributionTester.Builder.newInstance()
+            .jettyVersion(jettyVersion)
+            .jettyBase(jettyBase)
+            .mavenLocalRepository(System.getProperty("mavenRepoPath"))
+            .build();
+
+        String[] args1 = {
+            "--create-startd",
+            "--approve-all-licenses",
+            "--add-to-start=resources,server,webapp,deploy,jsp,jmx,servlet,servlets,websocket," + scheme
+        };
+        try (DistributionTester.Run run1 = distribution.start(args1))
+        {
+            assertTrue(run1.awaitFor(5, TimeUnit.SECONDS));
+            assertEquals(0, run1.getExitValue());
+
+            File webApp = distribution.resolveArtifact("org.eclipse.jetty.tests:test-websocket-client-webapp:war:" + jettyVersion);
+            distribution.installWarFile(webApp, "test");
+
+            int port = distribution.freePort();
+            String[] args2 = {
+                "jetty.http.port=" + port,
+                "jetty.ssl.port=" + port,
+                // We must hide the websocket classes from the webapp if we are to include websocket client jars in WEB-INF/lib.
+                "jetty.webapp.addServerClasses+=,+org.eclipse.jetty.websocket.",
+                "jetty.webapp.addSystemClasses+=,-org.eclipse.jetty.websocket.",
+                // "jetty.server.dumpAfterStart=true",
+                };
+
+            try (DistributionTester.Run run2 = distribution.start(args2))
+            {
+                assertTrue(run2.awaitConsoleLogsFor("Started @", 10, TimeUnit.SECONDS));
+
+                // We should get the correct configuration from the jetty-websocket-httpclient.xml file.
+                startHttpClient(scheme.equals("https"));
+                URI serverUri = URI.create(scheme + "://localhost:" + port + "/test");
+                ContentResponse response = client.GET(serverUri);
+                assertEquals(HttpStatus.OK_200, response.getStatus());
+                String content = response.getContentAsString();
+                assertThat(content, containsString("WebSocketEcho: success"));
+                assertThat(content, containsString("ConnectTimeout: 4999"));
+            }
+        }
+    }
 }

tests/test-webapps/pom.xml

@@ -43,5 +43,7 @@
     <module>test-cdi-common-webapp</module>
     <module>test-weld-cdi-webapp</module>
     <module>test-owb-cdi-webapp</module>
+    <module>test-websocket-client-webapp</module>
+    <module>test-websocket-client-provided-webapp</module>
   </modules>
 </project>