Issue #5320 - also test WebSocketClient on server from WEB-INF/lib

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

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

@@ -32,17 +32,26 @@ import java.util.Set;
 @Deprecated
 public abstract class ExtensionFactory implements Iterable<Class<? extends Extension>>
 {
-    private ServiceLoader<Extension> extensionLoader = ServiceLoader.load(Extension.class);
+    private final Map<String, Class<? extends Extension>> availableExtensions;
-    private 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-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 @@ import org.eclipse.jetty.websocket.common.scopes.WebSocketContainerScope;
 
 public class WebSocketExtensionFactory extends ExtensionFactory implements LifeCycle, Dumpable
 {
-    private ContainerLifeCycle containerLifeCycle;
+    private final ContainerLifeCycle containerLifeCycle;
-    private WebSocketContainerScope container;
+    private final WebSocketContainerScope container;
-    private ServiceLoader<Extension> extensionLoader = ServiceLoader.load(Extension.class);
-    private Map<String, Class<? extends Extension>> availableExtensions;
     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 class WebSocketExtensionFactory extends ExtensionFactory implements LifeC
                 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 class WebSocketExtensionFactory extends ExtensionFactory implements LifeC
         }
     }
 
-    @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/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java

@@ -23,7 +23,6 @@ import java.net.URI;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 
 import org.eclipse.jetty.client.HttpClient;
@@ -33,12 +32,9 @@ import org.eclipse.jetty.http2.client.HTTP2Client;
 import org.eclipse.jetty.http2.client.http.HttpClientTransportOverHTTP2;
 import org.eclipse.jetty.unixsocket.UnixSocketConnector;
 import org.eclipse.jetty.unixsocket.client.HttpClientTransportOverUnixSockets;
-import org.eclipse.jetty.util.BlockingArrayQueue;
 import org.eclipse.jetty.util.IO;
 import org.eclipse.jetty.util.StringUtil;
 import org.eclipse.jetty.util.ssl.SslContextFactory;
-import org.eclipse.jetty.websocket.api.Session;
-import org.eclipse.jetty.websocket.api.WebSocketListener;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.condition.DisabledOnJre;
 import org.junit.jupiter.api.condition.JRE;
@@ -309,6 +305,56 @@ public class DistributionTests extends AbstractDistributionTest
         }
     }
 
+    @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"));
+
+                // We cannot test the HttpClient timeout because it is a server class not exposed to the webapp.
+                // assertThat(content, containsString("ConnectTimeout: 4999"));
+            }
+        }
+    }
+
     @ParameterizedTest
     @ValueSource(strings = {"http", "https"})
     public void testWebsocketClientInWebapp(String scheme) throws Exception
@@ -338,14 +384,10 @@ public class DistributionTests extends AbstractDistributionTest
             String[] args2 = {
                 "jetty.http.port=" + port,
                 "jetty.ssl.port=" + port,
+                "jetty.webapp.addServerClasses+=,+org.eclipse.jetty.websocket.",
+                "jetty.webapp.addSystemClasses+=,-org.eclipse.jetty.websocket.",
                 // "jetty.server.dumpAfterStart=true",
-                // "jetty.webapp.addSystemClasses+=,org.eclipse.jetty.client.",
+                };
-                // "jetty.webapp.addServerClasses+=,-org.eclipse.jetty.client.",
-                // "jetty.webapp.addSystemClasses+=,org.eclipse.jetty.util.ssl.",
-                // "jetty.webapp.addServerClasses+=,-org.eclipse.jetty.util.ssl.",
-                // "jetty.webapp.addSystemClasses+=,org.eclipse.jetty.util.component.",
-                // "jetty.webapp.addServerClasses+=,-org.eclipse.jetty.util.component."
-            };
 
             try (DistributionTester.Run run2 = distribution.start(args2))
             {
@@ -358,45 +400,8 @@ public class DistributionTests extends AbstractDistributionTest
                 assertEquals(HttpStatus.OK_200, response.getStatus());
                 String content = response.getContentAsString();
                 assertThat(content, containsString("WebSocketEcho: success"));
-
+                assertThat(content, containsString("ConnectTimeout: 4999"));
-                // We cannot test the HttpClient timeout because it is a server class not exposed to the webapp.
-                // assertThat(content, containsString("ConnectTimeout: 4999"));
             }
         }
     }
-
-    public static class WsListener implements WebSocketListener
-    {
-        public BlockingArrayQueue<String> textMessages = new BlockingArrayQueue<>();
-        public final CountDownLatch closeLatch = new CountDownLatch(1);
-        public int closeCode;
-
-        @Override
-        public void onWebSocketClose(int statusCode, String reason)
-        {
-            this.closeCode = statusCode;
-            closeLatch.countDown();
-        }
-
-        @Override
-        public void onWebSocketConnect(Session session)
-        {
-        }
-
-        @Override
-        public void onWebSocketError(Throwable cause)
-        {
-        }
-
-        @Override
-        public void onWebSocketBinary(byte[] payload, int offset, int len)
-        {
-        }
-
-        @Override
-        public void onWebSocketText(String message)
-        {
-            textMessages.add(message);
-        }
-    }
 }

tests/test-webapps/pom.xml

@@ -44,5 +44,6 @@
     <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>

tests/test-webapps/test-websocket-client-provided-webapp/pom.xml

@@ -0,0 +1,33 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+  <parent>
+    <groupId>org.eclipse.jetty.tests</groupId>
+    <artifactId>test-webapps-parent</artifactId>
+    <version>9.4.33-SNAPSHOT</version>
+  </parent>
+
+  <modelVersion>4.0.0</modelVersion>
+  <artifactId>test-websocket-client-provided-webapp</artifactId>
+  <packaging>war</packaging>
+
+  <name>Test :: Jetty Websocket Simple Webapp with WebSocketClient</name>
+
+  <dependencies>
+    <dependency>
+      <groupId>javax.servlet</groupId>
+      <artifactId>javax.servlet-api</artifactId>
+      <scope>provided</scope>
+    </dependency>
+    <dependency>
+      <groupId>javax.websocket</groupId>
+      <artifactId>javax.websocket-api</artifactId>
+      <scope>provided</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.eclipse.jetty.websocket</groupId>
+      <artifactId>websocket-client</artifactId>
+      <version>${project.version}</version>
+      <scope>provided</scope>
+    </dependency>
+  </dependencies>
+</project>

tests/test-webapps/test-websocket-client-provided-webapp/src/main/java/org/eclipse/jetty/tests/webapp/websocket/EchoEndpoint.java

@@ -0,0 +1,32 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2020 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.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      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.tests.webapp.websocket;
+
+import javax.websocket.OnMessage;
+import javax.websocket.server.ServerEndpoint;
+
+@ServerEndpoint(value = "/echo")
+public class EchoEndpoint
+{
+    @OnMessage
+    public String echo(String message)
+    {
+        return message;
+    }
+}

tests/test-webapps/test-websocket-client-provided-webapp/src/main/java/org/eclipse/jetty/tests/webapp/websocket/WebSocketClientServlet.java

@@ -0,0 +1,129 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2020 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.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      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.tests.webapp.websocket;
+
+import java.io.PrintWriter;
+import java.net.URI;
+import java.util.concurrent.ArrayBlockingQueue;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import javax.servlet.ServletException;
+import javax.servlet.annotation.WebServlet;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.eclipse.jetty.websocket.api.Session;
+import org.eclipse.jetty.websocket.api.annotations.OnWebSocketClose;
+import org.eclipse.jetty.websocket.api.annotations.OnWebSocketConnect;
+import org.eclipse.jetty.websocket.api.annotations.OnWebSocketMessage;
+import org.eclipse.jetty.websocket.api.annotations.WebSocket;
+import org.eclipse.jetty.websocket.api.util.WSURI;
+import org.eclipse.jetty.websocket.client.WebSocketClient;
+
+@WebServlet("/")
+public class WebSocketClientServlet extends HttpServlet
+{
+    private WebSocketClient client;
+
+    @Override
+    public void init() throws ServletException
+    {
+        // We must rely on jetty-websocket-httpclient.xml as we do not have access to server classes like HttpClient.
+        client = new WebSocketClient();
+
+        try
+        {
+            client.start();
+        }
+        catch (Exception e)
+        {
+            throw new ServletException(e);
+        }
+    }
+
+    @Override
+    public void destroy()
+    {
+        try
+        {
+            client.stop();
+        }
+        catch (Exception e)
+        {
+            e.printStackTrace();
+        }
+    }
+
+    @Override
+    protected void doGet(HttpServletRequest req, HttpServletResponse resp)
+    {
+        try
+        {
+            resp.setContentType("text/html");
+
+            // Send and receive a websocket echo on the same server.
+            ClientSocket clientSocket = new ClientSocket();
+            URI wsUri = WSURI.toWebsocket(req.getRequestURL()).resolve("echo");
+            client.connect(clientSocket, wsUri).get(5, TimeUnit.SECONDS);
+            clientSocket.session.getRemote().sendString("test message");
+            String response = clientSocket.textMessages.poll(5, TimeUnit.SECONDS);
+            clientSocket.session.close();
+            clientSocket.closeLatch.await(5, TimeUnit.SECONDS);
+
+            PrintWriter writer = resp.getWriter();
+            writer.println("WebSocketEcho: " + ("test message".equals(response) ? "success" : "failure"));
+            writer.println("WebSocketEcho: success");
+            // We cannot test the HttpClient timeout because it is a server class not exposed to the webapp.
+            // writer.println("ConnectTimeout: " + client.getHttpClient().getConnectTimeout());
+        }
+        catch (Exception e)
+        {
+            throw new RuntimeException(e);
+        }
+    }
+
+    @WebSocket
+    public static class ClientSocket
+    {
+        public Session session;
+        public CountDownLatch openLatch = new CountDownLatch(1);
+        public CountDownLatch closeLatch = new CountDownLatch(1);
+        public ArrayBlockingQueue<String> textMessages = new ArrayBlockingQueue<>(10);
+
+        @OnWebSocketConnect
+        public void onOpen(Session session)
+        {
+            this.session = session;
+            openLatch.countDown();
+        }
+
+        @OnWebSocketMessage
+        public void onMessage(String message)
+        {
+            textMessages.add(message);
+        }
+
+        @OnWebSocketClose
+        public void onClose(int statusCode, String reason)
+        {
+            closeLatch.countDown();
+        }
+    }
+}

tests/test-webapps/test-websocket-client-provided-webapp/src/main/webapp/WEB-INF/web.xml

@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<web-app
+        xmlns="http://xmlns.jcp.org/xml/ns/javaee"
+        xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+        xsi:schemaLocation="http://xmlns.jcp.org/xml/ns/javaee http://xmlns.jcp.org/xml/ns/javaee/web-app_3_1.xsd"
+        metadata-complete="false"
+        version="3.1">
+</web-app>

tests/test-webapps/test-websocket-client-webapp/pom.xml

@@ -27,7 +27,6 @@
       <groupId>org.eclipse.jetty.websocket</groupId>
       <artifactId>websocket-client</artifactId>
       <version>${project.version}</version>
-      <scope>provided</scope>
     </dependency>
   </dependencies>
 </project>

tests/test-webapps/test-websocket-client-webapp/src/main/java/org/eclipse/jetty/tests/webapp/websocket/WebSocketClientServlet.java

@@ -29,6 +29,8 @@ import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
+import org.eclipse.jetty.client.HttpClient;
+import org.eclipse.jetty.util.ssl.SslContextFactory;
 import org.eclipse.jetty.websocket.api.Session;
 import org.eclipse.jetty.websocket.api.annotations.OnWebSocketClose;
 import org.eclipse.jetty.websocket.api.annotations.OnWebSocketConnect;
@@ -45,11 +47,15 @@ public class WebSocketClientServlet extends HttpServlet
     @Override
     public void init() throws ServletException
     {
-        client = new WebSocketClient();
+        // We can't use the jetty-websocket-httpclient.xml if the websocket client jars are in WEB-INF/lib.
+        SslContextFactory.Client sslContextFactory = new SslContextFactory.Client(true);
+        HttpClient httpClient = new HttpClient(sslContextFactory);
+        httpClient.setConnectTimeout(4999);
+        this.client = new WebSocketClient(httpClient);
 
         try
         {
-            client.start();
+            this.client.start();
         }
         catch (Exception e)
         {
@@ -89,8 +95,7 @@ public class WebSocketClientServlet extends HttpServlet
             PrintWriter writer = resp.getWriter();
             writer.println("WebSocketEcho: " + ("test message".equals(response) ? "success" : "failure"));
             writer.println("WebSocketEcho: success");
-            // We cannot test the HttpClient timeout because it is a server class not exposed to the webapp.
+            writer.println("ConnectTimeout: " + client.getHttpClient().getConnectTimeout());
-            // writer.println("ConnectTimeout: " + client.getHttpClient().getConnectTimeout());
         }
         catch (Exception e)
         {
@@ -125,10 +130,4 @@ public class WebSocketClientServlet extends HttpServlet
             closeLatch.countDown();
         }
     }
-
-    private void assertTrue(boolean value)
-    {
-        if (!value)
-            throw new RuntimeException("expected expression to be true but was false");
-    }
 }