Fix NPE from WebSocketSession.getUserPrincipal() in Jetty10

jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/internal/RFC6455Handshaker.java

@@ -36,7 +36,7 @@
 
 public final class RFC6455Handshaker extends AbstractHandshaker
 {
-    private static final HttpField UPGRADE_WEBSOCKET = new PreEncodedHttpField(HttpHeader.UPGRADE, "WebSocket");
+    private static final HttpField UPGRADE_WEBSOCKET = new PreEncodedHttpField(HttpHeader.UPGRADE, "websocket");
     private static final HttpField CONNECTION_UPGRADE = new PreEncodedHttpField(HttpHeader.CONNECTION, HttpHeader.UPGRADE.asString());
 
     @Override

jetty-websocket/websocket-core-tests/src/test/java/org/eclipse/jetty/websocket/core/WebSocketNegotiationTest.java

@@ -332,7 +332,7 @@ public void onHandshakeResponse(HttpRequest request, HttpResponse response)
         // RFC6455: If the server does not agree to any of the client's requested subprotocols, the only acceptable
         // value is null. It MUST NOT send back a |Sec-WebSocket-Protocol| header field in its response.
         HttpFields httpFields = headers.get();
-        assertThat(httpFields.get(HttpHeader.UPGRADE), is("WebSocket"));
+        assertThat(httpFields.get(HttpHeader.UPGRADE), is("websocket"));
         assertNull(httpFields.get(HttpHeader.SEC_WEBSOCKET_SUBPROTOCOL));
     }
 

jetty-websocket/websocket-javax-client/src/main/java/org/eclipse/jetty/websocket/javax/client/internal/JavaxClientUpgradeRequest.java

@@ -16,8 +16,6 @@
 import java.net.URI;
 import java.security.Principal;
 
-import org.eclipse.jetty.client.HttpResponse;
-import org.eclipse.jetty.io.EndPoint;
 import org.eclipse.jetty.websocket.core.FrameHandler;
 import org.eclipse.jetty.websocket.core.client.CoreClientUpgradeRequest;
 import org.eclipse.jetty.websocket.core.client.WebSocketCoreClient;
@@ -34,13 +32,6 @@ public JavaxClientUpgradeRequest(JavaxWebSocketClientContainer clientContainer,
         frameHandler = clientContainer.newFrameHandler(websocketPojo, this);
     }
 
-    @Override
-    public void upgrade(HttpResponse response, EndPoint endPoint)
-    {
-        frameHandler.setUpgradeRequest(this);
-        super.upgrade(response, endPoint);
-    }
-
     @Override
     public FrameHandler getFrameHandler()
     {

jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketFrameHandler.java

@@ -68,7 +68,7 @@ public class JavaxWebSocketFrameHandler implements FrameHandler
     private MethodHandle pongHandle;
     private JavaxWebSocketMessageMetadata textMetadata;
     private JavaxWebSocketMessageMetadata binaryMetadata;
-    private UpgradeRequest upgradeRequest;
+    private final UpgradeRequest upgradeRequest;
     private EndpointConfig endpointConfig;
     private final Map<Byte, RegisteredMessageHandler> messageHandlerMap = new HashMap<>();
     private MessageSink textSink;
@@ -79,6 +79,7 @@ public class JavaxWebSocketFrameHandler implements FrameHandler
     protected byte dataType = OpCode.UNDEFINED;
 
     public JavaxWebSocketFrameHandler(JavaxWebSocketContainer container,
+                                      UpgradeRequest upgradeRequest,
                                       Object endpointInstance,
                                       MethodHandle openHandle, MethodHandle closeHandle, MethodHandle errorHandle,
                                       JavaxWebSocketMessageMetadata textMetadata,
@@ -89,14 +90,14 @@ public JavaxWebSocketFrameHandler(JavaxWebSocketContainer container,
         this.logger = LoggerFactory.getLogger(endpointInstance.getClass());
 
         this.container = container;
+        this.upgradeRequest = upgradeRequest;
         if (endpointInstance instanceof ConfiguredEndpoint)
         {
             RuntimeException oops = new RuntimeException("ConfiguredEndpoint needs to be unwrapped");
             logger.warn("Unexpected ConfiguredEndpoint", oops);
             throw oops;
         }
         this.endpointInstance = endpointInstance;
-
         this.openHandle = openHandle;
         this.closeHandle = closeHandle;
         this.errorHandle = errorHandle;
@@ -636,11 +637,6 @@ public void onContinuation(Frame frame, Callback callback)
         }
     }
 
-    public void setUpgradeRequest(UpgradeRequest upgradeRequest)
-    {
-        this.upgradeRequest = upgradeRequest;
-    }
-
     public UpgradeRequest getUpgradeRequest()
     {
         return upgradeRequest;

jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketFrameHandlerFactory.java

@@ -167,6 +167,7 @@ public JavaxWebSocketFrameHandler newJavaxWebSocketFrameHandler(Object endpointI
 
         return new JavaxWebSocketFrameHandler(
             container,
+            upgradeRequest,
             endpoint,
             openHandle, closeHandle, errorHandle,
             textMetadata, binaryMetadata,

jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/UpgradeRequestResponseTest.java

@@ -0,0 +1,104 @@
+//
+// ========================================================================
+// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others.
+//
+// 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.
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+// ========================================================================
+//
+
+package org.eclipse.jetty.websocket.javax.tests;
+
+import java.net.URI;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.TimeUnit;
+import javax.websocket.ClientEndpoint;
+import javax.websocket.ClientEndpointConfig;
+import javax.websocket.Session;
+import javax.websocket.server.ServerEndpoint;
+
+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.javax.client.internal.JavaxWebSocketClientContainer;
+import org.eclipse.jetty.websocket.javax.server.config.JavaxWebSocketServletContainerInitializer;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class UpgradeRequestResponseTest
+{
+    private ServerConnector connector;
+    private JavaxWebSocketClientContainer client;
+    private static CompletableFuture<EventSocket> serverSocketFuture;
+
+    @ServerEndpoint("/")
+    public static class ServerSocket extends EchoSocket
+    {
+        public ServerSocket()
+        {
+            serverSocketFuture.complete(this);
+        }
+    }
+
+    public static class PermessageDeflateConfig extends ClientEndpointConfig.Configurator
+    {
+        @Override
+        public void beforeRequest(Map<String, List<String>> headers)
+        {
+            headers.put(HttpHeader.SEC_WEBSOCKET_EXTENSIONS.asString(), Collections.singletonList("permessage-deflate"));
+        }
+    }
+
+    @ClientEndpoint(configurator = PermessageDeflateConfig.class)
+    public static class ClientSocket extends EventSocket
+    {
+    }
+
+    @BeforeEach
+    public void start() throws Exception
+    {
+        Server server = new Server();
+        connector = new ServerConnector(server);
+        server.addConnector(connector);
+        serverSocketFuture = new CompletableFuture<>();
+
+        ServletContextHandler contextHandler = new ServletContextHandler(ServletContextHandler.SESSIONS);
+        server.setHandler(contextHandler);
+        contextHandler.setContextPath("/");
+        JavaxWebSocketServletContainerInitializer.configure(contextHandler, (context, container) ->
+            container.addEndpoint(ServerSocket.class));
+
+        client = new JavaxWebSocketClientContainer();
+        server.start();
+        client.start();
+    }
+
+    @Test
+    public void testUpgradeRequestResponse() throws Exception
+    {
+        URI uri = URI.create("ws://localhost:" + connector.getLocalPort());
+        EventSocket socket = new ClientSocket();
+
+        Session clientSession = client.connectToServer(socket, uri);
+        EventSocket serverSocket = serverSocketFuture.get(5, TimeUnit.SECONDS);
+        assertTrue(serverSocket.openLatch.await(5, TimeUnit.SECONDS));
+
+        // The user principal is found on the base UpgradeRequest.
+        assertDoesNotThrow(clientSession::getUserPrincipal);
+        assertDoesNotThrow(serverSocket.session::getUserPrincipal);
+
+        clientSession.close();
+        assertTrue(socket.closeLatch.await(5, TimeUnit.SECONDS));
+    }
+}

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

@@ -0,0 +1,126 @@
+//
+// ========================================================================
+// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others.
+//
+// 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.
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+// ========================================================================
+//
+
+package org.eclipse.jetty.websocket.tests;
+
+import java.net.URI;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.TimeUnit;
+
+import org.eclipse.jetty.http.HttpHeader;
+import org.eclipse.jetty.http.HttpStatus;
+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.UpgradeResponse;
+import org.eclipse.jetty.websocket.client.ClientUpgradeRequest;
+import org.eclipse.jetty.websocket.client.WebSocketClient;
+import org.eclipse.jetty.websocket.server.config.JettyWebSocketServletContainerInitializer;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class UpgradeRequestResponseTest
+{
+    private ServerConnector connector;
+    private WebSocketClient client;
+    private EventSocket serverSocket;
+
+    @BeforeEach
+    public void start() throws Exception
+    {
+        Server server = new Server();
+        connector = new ServerConnector(server);
+        server.addConnector(connector);
+        serverSocket = new EchoSocket();
+
+        ServletContextHandler contextHandler = new ServletContextHandler(ServletContextHandler.SESSIONS);
+        server.setHandler(contextHandler);
+        contextHandler.setContextPath("/");
+        JettyWebSocketServletContainerInitializer.configure(contextHandler, (context, container) ->
+            container.addMapping("/", (req, resp) -> serverSocket));
+
+        client = new WebSocketClient();
+
+        server.start();
+        client.start();
+    }
+
+    @Test
+    public void testClientUpgradeRequestResponse() throws Exception
+    {
+        URI uri = URI.create("ws://localhost:" + connector.getLocalPort());
+        EventSocket socket = new EventSocket();
+        ClientUpgradeRequest request = new ClientUpgradeRequest();
+        request.addExtensions("permessage-deflate");
+
+        CompletableFuture<Session> connect = client.connect(socket, uri, request);
+        Session session = connect.get(5, TimeUnit.SECONDS);
+        UpgradeRequest upgradeRequest = session.getUpgradeRequest();
+        UpgradeResponse upgradeResponse = session.getUpgradeResponse();
+
+        assertNotNull(upgradeRequest);
+        assertThat(upgradeRequest.getHeader(HttpHeader.UPGRADE.asString()), is("websocket"));
+        assertThat(upgradeRequest.getHeader(HttpHeader.SEC_WEBSOCKET_EXTENSIONS.asString()), is("permessage-deflate"));
+        assertThat(upgradeRequest.getExtensions().size(), is(1));
+        assertThat(upgradeRequest.getExtensions().get(0).getName(), is("permessage-deflate"));
+
+        assertNotNull(upgradeResponse);
+        assertThat(upgradeResponse.getStatusCode(), is(HttpStatus.SWITCHING_PROTOCOLS_101));
+        assertThat(upgradeResponse.getHeader(HttpHeader.UPGRADE.asString()), is("websocket"));
+        assertThat(upgradeResponse.getHeader(HttpHeader.SEC_WEBSOCKET_EXTENSIONS.asString()), is("permessage-deflate"));
+        assertThat(upgradeResponse.getExtensions().size(), is(1));
+        assertThat(upgradeResponse.getExtensions().get(0).getName(), is("permessage-deflate"));
+
+        session.close();
+        assertTrue(socket.closeLatch.await(5, TimeUnit.SECONDS));
+    }
+
+    @Test
+    public void testServerUpgradeRequestResponse() throws Exception
+    {
+        URI uri = URI.create("ws://localhost:" + connector.getLocalPort());
+        EventSocket socket = new EventSocket();
+        ClientUpgradeRequest request = new ClientUpgradeRequest();
+        request.addExtensions("permessage-deflate");
+
+        CompletableFuture<Session> connect = client.connect(socket, uri, request);
+        Session session = connect.get(5, TimeUnit.SECONDS);
+        assertTrue(serverSocket.openLatch.await(5, TimeUnit.SECONDS));
+        UpgradeRequest upgradeRequest = serverSocket.session.getUpgradeRequest();
+        UpgradeResponse upgradeResponse = serverSocket.session.getUpgradeResponse();
+
+        assertNotNull(upgradeRequest);
+        assertThat(upgradeRequest.getHeader(HttpHeader.UPGRADE.asString()), is("websocket"));
+        assertThat(upgradeRequest.getHeader(HttpHeader.SEC_WEBSOCKET_EXTENSIONS.asString()), is("permessage-deflate"));
+        assertThat(upgradeRequest.getExtensions().size(), is(1));
+        assertThat(upgradeRequest.getExtensions().get(0).getName(), is("permessage-deflate"));
+
+        assertNotNull(upgradeResponse);
+        /* TODO: The HttpServletResponse is eventually recycled so we lose this information.
+        assertThat(upgradeResponse.getStatusCode(), is(HttpStatus.SWITCHING_PROTOCOLS_101));
+        assertThat(upgradeResponse.getHeader(HttpHeader.UPGRADE.asString()), is("websocket"));
+        assertThat(upgradeResponse.getHeader(HttpHeader.SEC_WEBSOCKET_EXTENSIONS.asString()), is("permessage-deflate"));
+        assertThat(upgradeResponse.getExtensions().size(), is(1));
+        assertThat(upgradeResponse.getExtensions().get(0).getName(), is("permessage-deflate"));
+        */
+        session.close();
+        assertTrue(socket.closeLatch.await(5, TimeUnit.SECONDS));
+    }
+}