435206 - Can't add Cookie header on websocket ClientUpgradeRequest

+ Fixed competing cookie setters between WebSocketClient's use of
  CookieStore and UpgradeRequest.setCookies()
+ Added some utility methods to LazyList (for lack of existence of
  ListUtil or CollectionUtil in jetty-util)

6 files changed, 237 insertions, 34 deletions

diff --git a/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/UpgradeRequest.java b/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/UpgradeRequest.java
index 81059efbc3..1d000c6908 100644
--- a/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/UpgradeRequest.java
+++ b/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/UpgradeRequest.java
@@ -259,9 +259,12 @@ public class UpgradeRequest
     public void setCookies(List<HttpCookie> cookies)
     {
         this.cookies.clear();
-        this.cookies.addAll(cookies);
+        if (cookies != null && !cookies.isEmpty())
+        {
+            this.cookies.addAll(cookies);
+        }
     }
-
+    
     public void setExtensions(List<ExtensionConfig> configs)
     {
         this.extensions.clear();
diff --git a/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/ClientUpgradeRequest.java b/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/ClientUpgradeRequest.java
index 15d2f37e04..4993bf561f 100644
--- a/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/ClientUpgradeRequest.java
+++ b/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/ClientUpgradeRequest.java
@@ -31,6 +31,7 @@ import java.util.TreeSet;
 import java.util.concurrent.ThreadLocalRandom;
 
 import org.eclipse.jetty.util.B64Code;
+import org.eclipse.jetty.util.LazyList;
 import org.eclipse.jetty.util.MultiMap;
 import org.eclipse.jetty.util.StringUtil;
 import org.eclipse.jetty.util.UrlEncoded;
@@ -210,7 +211,19 @@ public class ClientUpgradeRequest extends UpgradeRequest
             return;
         }
 
-        setCookies(cookieStore.get(getRequestURI()));
+        List<HttpCookie> existing = getCookies();
+        List<HttpCookie> extra = cookieStore.get(getRequestURI());
+
+        List<HttpCookie> cookies = new ArrayList<>();
+        if (LazyList.hasEntry(existing))
+        {
+            cookies.addAll(existing);
+        }
+        if (LazyList.hasEntry(extra))
+        {
+            cookies.addAll(extra);
+        }
+        setCookies(cookies);
     }
 
     @Override
diff --git a/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java b/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java
index 7ccb4f55c9..433f3cedd2 100644
--- a/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java
+++ b/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java
@@ -36,7 +36,6 @@ import org.eclipse.jetty.io.SelectorManager;
 import org.eclipse.jetty.util.HttpCookieStore;
 import org.eclipse.jetty.util.StringUtil;
 import org.eclipse.jetty.util.component.ContainerLifeCycle;
-import org.eclipse.jetty.util.component.LifeCycle;
 import org.eclipse.jetty.util.log.Log;
 import org.eclipse.jetty.util.log.Logger;
 import org.eclipse.jetty.util.ssl.SslContextFactory;
diff --git a/jetty-websocket/websocket-client/src/test/java/org/eclipse/jetty/websocket/client/CookieTest.java b/jetty-websocket/websocket-client/src/test/java/org/eclipse/jetty/websocket/client/CookieTest.java
new file mode 100644
index 0000000000..b9af1986dc
--- /dev/null
+++ b/jetty-websocket/websocket-client/src/test/java/org/eclipse/jetty/websocket/client/CookieTest.java
@@ -0,0 +1,173 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2014 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.client;
+
+import static org.hamcrest.Matchers.*;
+
+import java.net.CookieManager;
+import java.net.HttpCookie;
+import java.util.Collections;
+import java.util.List;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+
+import org.eclipse.jetty.toolchain.test.EventQueue;
+import org.eclipse.jetty.util.log.Log;
+import org.eclipse.jetty.util.log.Logger;
+import org.eclipse.jetty.websocket.api.Session;
+import org.eclipse.jetty.websocket.api.StatusCode;
+import org.eclipse.jetty.websocket.api.WebSocketAdapter;
+import org.eclipse.jetty.websocket.api.util.QuoteUtil;
+import org.eclipse.jetty.websocket.common.frames.TextFrame;
+import org.eclipse.jetty.websocket.common.test.BlockheadServer;
+import org.eclipse.jetty.websocket.common.test.BlockheadServer.ServerConnection;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+public class CookieTest
+{
+    private static final Logger LOG = Log.getLogger(CookieTest.class);
+
+    public static class CookieTrackingSocket extends WebSocketAdapter
+    {
+        public EventQueue<String> messageQueue = new EventQueue<>();
+        public EventQueue<Throwable> errorQueue = new EventQueue<>();
+
+        @Override
+        public void onWebSocketText(String message)
+        {
+            messageQueue.add(message);
+        }
+
+        @Override
+        public void onWebSocketError(Throwable cause)
+        {
+            errorQueue.add(cause);
+        }
+    }
+
+    private WebSocketClient client;
+    private BlockheadServer server;
+
+    @Before
+    public void startClient() throws Exception
+    {
+        client = new WebSocketClient();
+        client.start();
+    }
+
+    @Before
+    public void startServer() throws Exception
+    {
+        server = new BlockheadServer();
+        server.start();
+    }
+
+    @After
+    public void stopClient() throws Exception
+    {
+        if (client.isRunning())
+        {
+            client.stop();
+        }
+    }
+
+    @After
+    public void stopServer() throws Exception
+    {
+        server.stop();
+    }
+
+    @Test
+    public void testViaCookieManager() throws Exception
+    {
+        // Setup client
+        CookieManager cookieMgr = new CookieManager();
+        client.setCookieStore(cookieMgr.getCookieStore());
+        HttpCookie cookie = new HttpCookie("hello","world");
+        cookie.setPath("/");
+        cookie.setMaxAge(100000);
+        cookieMgr.getCookieStore().add(server.getWsUri(),cookie);
+
+        // Client connects
+        CookieTrackingSocket clientSocket = new CookieTrackingSocket();
+        Future<Session> clientConnectFuture = client.connect(clientSocket,server.getWsUri());
+
+        // Server accepts connect
+        ServerConnection serverConn = server.accept();
+
+        // client confirms upgrade and receipt of frame
+        String serverCookies = confirmClientUpgradeAndCookies(clientSocket,clientConnectFuture,serverConn);
+
+        Assert.assertThat("Cookies seen at server side",serverCookies,containsString("hello=\"world\""));
+    }
+    
+    @Test
+    public void testViaServletUpgradeRequest() throws Exception
+    {
+        // Setup client
+        HttpCookie cookie = new HttpCookie("hello","world");
+        cookie.setPath("/");
+        cookie.setMaxAge(100000);
+        
+        ClientUpgradeRequest request = new ClientUpgradeRequest();
+        request.setCookies(Collections.singletonList(cookie));
+
+        // Client connects
+        CookieTrackingSocket clientSocket = new CookieTrackingSocket();
+        Future<Session> clientConnectFuture = client.connect(clientSocket,server.getWsUri(),request);
+
+        // Server accepts connect
+        ServerConnection serverConn = server.accept();
+
+        // client confirms upgrade and receipt of frame
+        String serverCookies = confirmClientUpgradeAndCookies(clientSocket,clientConnectFuture,serverConn);
+
+        Assert.assertThat("Cookies seen at server side",serverCookies,containsString("hello=\"world\""));
+    }
+
+    private String confirmClientUpgradeAndCookies(CookieTrackingSocket clientSocket, Future<Session> clientConnectFuture, ServerConnection serverConn)
+            throws Exception
+    {
+        // Server upgrades
+        List<String> upgradeRequestLines = serverConn.upgrade();
+        List<String> upgradeRequestCookies = serverConn.regexFind(upgradeRequestLines,"^Cookie: (.*)$");
+
+        // Server responds with cookies it knows about
+        TextFrame serverCookieFrame = new TextFrame();
+        serverCookieFrame.setFin(true);
+        serverCookieFrame.setPayload(QuoteUtil.join(upgradeRequestCookies,","));
+        serverConn.write(serverCookieFrame);
+
+        // Server closes connection
+        serverConn.close(StatusCode.NORMAL);
+
+        // Confirm client connect on future
+        clientConnectFuture.get(500,TimeUnit.MILLISECONDS);
+
+        // Wait for client receipt of cookie frame via client websocket
+        clientSocket.messageQueue.awaitEventCount(1,2,TimeUnit.SECONDS);
+
+        String cookies = clientSocket.messageQueue.poll();
+        LOG.debug("Cookies seen at server: {}",cookies);
+        return cookies;
+    }
+}
diff --git a/jetty-websocket/websocket-client/src/test/resources/jetty-logging.properties b/jetty-websocket/websocket-client/src/test/resources/jetty-logging.properties
index 9668b13105..826f50f1ca 100644
--- a/jetty-websocket/websocket-client/src/test/resources/jetty-logging.properties
+++ b/jetty-websocket/websocket-client/src/test/resources/jetty-logging.properties
@@ -7,7 +7,7 @@ org.eclipse.jetty.LEVEL=WARN
 # org.eclipse.jetty.io.FillInterest.LEVEL=DEBUG
 # org.eclipse.jetty.io.AbstractConnection.LEVEL=DEBUG
 # org.eclipse.jetty.websocket.LEVEL=WARN
-# org.eclipse.jetty.websocket.LEVEL=DEBUG
+org.eclipse.jetty.websocket.LEVEL=DEBUG
 # org.eclipse.jetty.websocket.client.LEVEL=DEBUG
 # org.eclipse.jetty.websocket.common.io.AbstractWebSocketConnection.LEVEL=DEBUG
 # org.eclipse.jetty.websocket.common.io.IOState.LEVEL=DEBUG
diff --git a/jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/test/BlockheadServer.java b/jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/test/BlockheadServer.java
index 8db7f7dd7c..ecb712f281 100644
--- a/jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/test/BlockheadServer.java
+++ b/jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/test/BlockheadServer.java
@@ -18,6 +18,8 @@
 
 package org.eclipse.jetty.websocket.common.test;
 
+import static org.hamcrest.Matchers.*;
+
 import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.InputStream;
@@ -52,9 +54,9 @@ import org.eclipse.jetty.websocket.api.WebSocketPolicy;
 import org.eclipse.jetty.websocket.api.WriteCallback;
 import org.eclipse.jetty.websocket.api.extensions.ExtensionConfig;
 import org.eclipse.jetty.websocket.api.extensions.Frame;
+import org.eclipse.jetty.websocket.api.extensions.Frame.Type;
 import org.eclipse.jetty.websocket.api.extensions.IncomingFrames;
 import org.eclipse.jetty.websocket.api.extensions.OutgoingFrames;
-import org.eclipse.jetty.websocket.api.extensions.Frame.Type;
 import org.eclipse.jetty.websocket.common.AcceptHash;
 import org.eclipse.jetty.websocket.common.CloseInfo;
 import org.eclipse.jetty.websocket.common.Generator;
@@ -66,9 +68,6 @@ import org.eclipse.jetty.websocket.common.extensions.WebSocketExtensionFactory;
 import org.eclipse.jetty.websocket.common.frames.CloseFrame;
 import org.junit.Assert;
 
-import static org.hamcrest.Matchers.is;
-import static org.hamcrest.Matchers.notNullValue;
-
 /**
  * A overly simplistic websocket server used during testing.
  * <p>
@@ -280,20 +279,14 @@ public class BlockheadServer
         public List<ExtensionConfig> parseExtensions(List<String> requestLines)
         {
             List<ExtensionConfig> extensionConfigs = new ArrayList<>();
+            
+            List<String> hits = regexFind(requestLines, "^Sec-WebSocket-Extensions: (.*)$");
 
-            Pattern patExts = Pattern.compile("^Sec-WebSocket-Extensions: (.*)$",Pattern.CASE_INSENSITIVE);
-
-            Matcher mat;
-            for (String line : requestLines)
+            for (String econf : hits)
             {
-                mat = patExts.matcher(line);
-                if (mat.matches())
-                {
-                    // found extensions
-                    String econf = mat.group(1);
-                    ExtensionConfig config = ExtensionConfig.parse(econf);
-                    extensionConfigs.add(config);
-                }
+                // found extensions
+                ExtensionConfig config = ExtensionConfig.parse(econf);
+                extensionConfigs.add(config);
             }
 
             return extensionConfigs;
@@ -301,20 +294,15 @@ public class BlockheadServer
 
         public String parseWebSocketKey(List<String> requestLines)
         {
-            String key = null;
-
-            Pattern patKey = Pattern.compile("^Sec-WebSocket-Key: (.*)$",Pattern.CASE_INSENSITIVE);
-
-            Matcher mat;
-            for (String line : requestLines)
+            List<String> hits = regexFind(requestLines,"^Sec-WebSocket-Key: (.*)$");
+            if (hits.size() <= 0)
             {
-                mat = patKey.matcher(line);
-                if (mat.matches())
-                {
-                    key = mat.group(1);
-                }
+                return null;
             }
-
+            
+            Assert.assertThat("Number of Sec-WebSocket-Key headers", hits.size(), is(1));
+            
+            String key = hits.get(0);
             return key;
         }
 
@@ -415,6 +403,32 @@ public class BlockheadServer
             return lines;
         }
 
+        public List<String> regexFind(List<String> lines, String pattern)
+        {
+            List<String> hits = new ArrayList<>();
+
+            Pattern patKey = Pattern.compile(pattern,Pattern.CASE_INSENSITIVE);
+
+            Matcher mat;
+            for (String line : lines)
+            {
+                mat = patKey.matcher(line);
+                if (mat.matches())
+                {
+                    if (mat.groupCount() >= 1)
+                    {
+                        hits.add(mat.group(1));
+                    }
+                    else
+                    {
+                        hits.add(mat.group(0));
+                    }
+                }
+            }
+
+            return hits;
+        }
+
         public void respond(String rawstr) throws IOException
         {
             LOG.debug("respond(){}{}","\n",rawstr);
@@ -486,7 +500,7 @@ public class BlockheadServer
             echoing.set(false);
         }
 
-        public void upgrade() throws IOException
+        public List<String> upgrade() throws IOException
         {
             List<String> requestLines = readRequestLines();
             List<ExtensionConfig> extensionConfigs = parseExtensions(requestLines);
@@ -559,6 +573,7 @@ public class BlockheadServer
             // Write Response
             LOG.debug("Response: {}",resp.toString());
             write(resp.toString().getBytes());
+            return requestLines;
         }
 
         private void write(byte[] bytes) throws IOException