Issue jetty#3458 - testing of Jetty WebSocket API ExtensionConfig

wire up the jetty-api extensions by copying the ExtensionConfig from
the jetty-api upgrade request to the core upgrade request

make UpgradeListener interface methods default for convenience

introduce test to test the functionality of API ExtensionConfig

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

jetty-websocket/jetty-websocket-client/src/main/java/org/eclipse/jetty/websocket/client/impl/JettyClientUpgradeRequest.java

@@ -22,6 +22,7 @@
 import java.net.URI;
 import java.util.List;
 import java.util.concurrent.CompletableFuture;
+import java.util.stream.Collectors;
 
 import org.eclipse.jetty.client.HttpResponse;
 import org.eclipse.jetty.http.HttpFields;
@@ -33,6 +34,7 @@
 import org.eclipse.jetty.websocket.api.UpgradeResponse;
 import org.eclipse.jetty.websocket.client.WebSocketClient;
 import org.eclipse.jetty.websocket.common.JettyWebSocketFrameHandler;
+import org.eclipse.jetty.websocket.core.ExtensionConfig;
 import org.eclipse.jetty.websocket.core.FrameHandler;
 import org.eclipse.jetty.websocket.core.client.ClientUpgradeRequest;
 import org.eclipse.jetty.websocket.core.client.WebSocketCoreClient;
@@ -74,9 +76,9 @@ public JettyClientUpgradeRequest(WebSocketClient clientContainer, WebSocketCoreC
             setSubProtocols(request.getSubProtocols());
 
             // Copy extensions
-            /* TODO or not?
-            setExtensions(request.getExtensions());
-            */
+            setExtensions(request.getExtensions().stream()
+                    .map(c -> new ExtensionConfig(c.getName(), c.getParameters()))
+                    .collect(Collectors.toList()));
 
             // Copy method from upgradeRequest object
             if (request.getMethod() != null)

jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/JettyWebSocketExtensionConfigTest.java

@@ -0,0 +1,138 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  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.websocket.tests;
+
+import java.net.URI;
+import java.util.List;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.TimeUnit;
+
+import org.eclipse.jetty.client.HttpRequest;
+import org.eclipse.jetty.client.HttpResponse;
+import org.eclipse.jetty.http.HttpHeader;
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.server.ServerConnector;
+import org.eclipse.jetty.servlet.ServletContextHandler;
+import org.eclipse.jetty.websocket.api.Session;
+import org.eclipse.jetty.websocket.api.UpgradeRequest;
+import org.eclipse.jetty.websocket.api.extensions.ExtensionConfig;
+import org.eclipse.jetty.websocket.client.ClientUpgradeRequest;
+import org.eclipse.jetty.websocket.client.WebSocketClient;
+import org.eclipse.jetty.websocket.core.client.UpgradeListener;
+import org.eclipse.jetty.websocket.server.JettyWebSocketServerContainer;
+import org.eclipse.jetty.websocket.server.JettyWebSocketServletContainerInitializer;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
+
+public class JettyWebSocketExtensionConfigTest
+{
+    Server server;
+    WebSocketClient client;
+
+    @BeforeEach
+    public void start() throws Exception
+    {
+        server = new Server();
+        ServerConnector connector = new ServerConnector(server);
+        connector.setPort(8080);
+        server.addConnector(connector);
+
+        ServletContextHandler contextHandler = new ServletContextHandler(ServletContextHandler.SESSIONS);
+        contextHandler.setContextPath("/");
+        server.setHandler(contextHandler);
+
+        JettyWebSocketServerContainer container = JettyWebSocketServletContainerInitializer.configureContext(contextHandler);
+        container.addMapping("/", (req, resp)->
+        {
+            assertEquals(req.getExtensions().stream().filter(e -> e.getName().equals("permessage-deflate")).count(), 1);
+            assertEquals(resp.getExtensions().stream().filter(e -> e.getName().equals("permessage-deflate")).count(), 1);
+
+            ExtensionConfig permessageDeflate = ExtensionConfig.parse("permessage-deflate");
+            ExtensionConfig nonRequestedExtension = ExtensionConfig.parse("identity");
+            assertNotNull(permessageDeflate);
+            assertNotNull(nonRequestedExtension);
+
+            try
+            {
+                resp.setExtensions(List.of(permessageDeflate, nonRequestedExtension));
+                fail("should not allow extensions not requested");
+            }
+            catch (IllegalArgumentException e)
+            {
+                resp.setExtensions(List.of(permessageDeflate));
+            }
+
+            // Check identity extension was not added because it was not requested
+            assertEquals(req.getExtensions().stream().filter(e -> e.getName().equals("identity")).count(), 0);
+            assertEquals(resp.getExtensions().stream().filter(e -> e.getName().equals("identity")).count(), 0);
+
+            return new EchoSocket();
+        });
+        server.start();
+
+        client = new WebSocketClient();
+        client.start();
+    }
+
+    @AfterEach
+    public void stop() throws Exception
+    {
+        client.stop();
+        server.stop();
+    }
+
+    @Test
+    public void test() throws Exception
+    {
+        URI uri = URI.create("ws://localhost:8080/filterPath");
+        EventSocket socket = new EventSocket();
+
+        UpgradeRequest request = new ClientUpgradeRequest();
+        request.addExtensions(ExtensionConfig.parse("permessage-deflate"));
+
+
+        UpgradeListener listener = new UpgradeListener()
+        {
+            @Override
+            public void onHandshakeResponse(HttpRequest request, HttpResponse response)
+            {
+                assertThat(response.getHeaders().get(HttpHeader.SEC_WEBSOCKET_EXTENSIONS), equalTo("permessage-deflate"));
+            }
+        };
+
+        CompletableFuture<Session> connect = client.connect(socket, uri, request, listener);
+        try(Session session = connect.get(5, TimeUnit.SECONDS))
+        {
+            session.getRemote().sendString("hello world");
+        }
+        assertTrue(socket.closed.await(10, TimeUnit.SECONDS));
+
+        String msg = socket.receivedMessages.poll();
+        assertThat(msg, is("hello world"));
+    }
+}

jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/UpgradeListener.java

@@ -28,13 +28,15 @@ public interface UpgradeListener
      *
      * @param request the request
      */
-    void onHandshakeRequest(HttpRequest request);
+    default void onHandshakeRequest(HttpRequest request)
+    {}
 
     /**
      * Event that triggers after the Handshake response has been received.
      *
      * @param request  the request that was used
      * @param response the response that was received
      */
-    void onHandshakeResponse(HttpRequest request, HttpResponse response);
+    default void onHandshakeResponse(HttpRequest request, HttpResponse response)
+    {}
 }

jetty-websocket/websocket-servlet/src/main/java/org/eclipse/jetty/websocket/servlet/ServletUpgradeResponse.java

@@ -18,11 +18,6 @@
 
 package org.eclipse.jetty.websocket.servlet;
 
-import org.eclipse.jetty.http.HttpHeader;
-import org.eclipse.jetty.websocket.core.ExtensionConfig;
-import org.eclipse.jetty.websocket.core.server.Negotiation;
-
-import javax.servlet.http.HttpServletResponse;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collections;
@@ -33,6 +28,12 @@
 import java.util.Set;
 import java.util.stream.Collectors;
 
+import javax.servlet.http.HttpServletResponse;
+
+import org.eclipse.jetty.http.HttpHeader;
+import org.eclipse.jetty.websocket.core.ExtensionConfig;
+import org.eclipse.jetty.websocket.core.server.Negotiation;
+
 /**
  * Servlet Specific UpgradeResponse implementation.
  */
@@ -166,6 +167,39 @@ public void setAcceptedSubProtocol(String protocol)
 
     public void setExtensions(List<ExtensionConfig> configs)
     {
+        for (ExtensionConfig config : configs)
+        {
+            List<ExtensionConfig> collect = negotiation.getOfferedExtensions().stream()
+                    .filter(e -> e.getName().equals(config.getName()))
+                    .collect(Collectors.toList());
+
+            if (collect.size() == 1)
+            {
+                ExtensionConfig requestedConfig = collect.get(0);
+                if (config.getParameters().equals(requestedConfig.getParameters()))
+                {
+                    continue;
+                }
+                else
+                {
+                    // TODO: how to handle extension with parameters different to those requested
+                    /*
+                    for (String key : requestedConfig.getParameters().keySet())
+                    {
+                        String value = requestedConfig.getParameter(key, null);
+                        if (config.getParameter(key, null) != value)
+                            throw new IllegalArgumentException("Extension parameter differs from requested parameter");
+                    }
+                    */
+                    throw new IllegalArgumentException("Extension parameters of response do not match those requested");
+                }
+            }
+            else if (collect.size() == 0)
+                throw new IllegalArgumentException("Extension not a requested extension");
+            else if (collect.size() > 1)
+                throw new IllegalArgumentException("Multiple extensions of the same name");
+        }
+
         negotiation.setNegotiatedExtensions(configs);
     }