416763 - WebSocket / Jsr Session.getPathParameters() is empty

+ Ensuring this works as intended with new test case

jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/JsrSession.java

@@ -89,6 +89,7 @@ public class JsrSession extends WebSocketSession implements javax.websocket.Sess
         this.messageHandlerFactory = new MessageHandlerFactory();
         this.wrappers = new MessageHandlerWrapper[MessageType.values().length];
         this.messageHandlerSet = new HashSet<>();
+
     }
 
     @Override

jetty-websocket/javax-websocket-server-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/server/AnnotatedServerEndpointConfig.java

@@ -51,37 +51,64 @@ public class AnnotatedServerEndpointConfig implements ServerEndpointConfig
 
     public AnnotatedServerEndpointConfig(Class<?> endpointClass, ServerEndpoint anno, ServerEndpointConfig baseConfig) throws DeploymentException
     {
-        List<Class<? extends Decoder>> compositeDecoders = new ArrayList<>();
-        List<Class<? extends Encoder>> compositeEncoders = new ArrayList<>();
-        List<String> compositeSubProtocols = new ArrayList<>();
-
         Configurator configr = null;
 
         // Copy from base config
         if (baseConfig != null)
         {
-            compositeDecoders.addAll(baseConfig.getDecoders());
-            compositeEncoders.addAll(baseConfig.getEncoders());
-            compositeSubProtocols.addAll(baseConfig.getSubprotocols());
             configr = baseConfig.getConfigurator();
         }
 
-        // now add from annotations
+        // Decoders (favor provided config over annotation)
-        compositeDecoders.addAll(Arrays.asList(anno.decoders()));
+        if (baseConfig != null && baseConfig.getDecoders() != null && baseConfig.getDecoders().size() > 0)
-        compositeEncoders.addAll(Arrays.asList(anno.encoders()));
+        {
-        compositeSubProtocols.addAll(Arrays.asList(anno.subprotocols()));
+            this.decoders = Collections.unmodifiableList(baseConfig.getDecoders());
+        }
+        else
+        {
+            this.decoders = Collections.unmodifiableList(Arrays.asList(anno.decoders()));
+        }
 
-        // Create unmodifiable lists
+        // Encoders (favor provided config over annotation)
-        this.decoders = Collections.unmodifiableList(compositeDecoders);
+        if (baseConfig != null && baseConfig.getEncoders() != null && baseConfig.getEncoders().size() > 0)
-        this.encoders = Collections.unmodifiableList(compositeEncoders);
+        {
-        this.subprotocols = Collections.unmodifiableList(compositeSubProtocols);
+            this.encoders = Collections.unmodifiableList(baseConfig.getEncoders());
+        }
+        else
+        {
+            this.encoders = Collections.unmodifiableList(Arrays.asList(anno.encoders()));
+        }
+
+        // Sub Protocols (favor provided config over annotation)
+        if (baseConfig != null && baseConfig.getSubprotocols() != null && baseConfig.getSubprotocols().size() > 0)
+        {
+            this.subprotocols = Collections.unmodifiableList(baseConfig.getSubprotocols());
+        }
+        else
+        {
+            this.subprotocols = Collections.unmodifiableList(Arrays.asList(anno.subprotocols()));
+        }
+
+        // Path (favor provided config over annotation)
+        if (baseConfig != null && baseConfig.getPath() != null && baseConfig.getPath().length() > 0)
+        {
+            this.path = baseConfig.getPath();
+        }
+        else
+        {
+            this.path = anno.value();
+        }
 
         // supplied by init lifecycle
         this.extensions = new ArrayList<>();
-        this.path = anno.value();
+        // always what is passed in
         this.endpointClass = endpointClass;
-        // no userProperties in annotation
+        // UserProperties in annotation
         this.userProperties = new HashMap<>();
+        if (baseConfig != null && baseConfig.getUserProperties() != null && baseConfig.getUserProperties().size() > 0)
+        {
+            userProperties.putAll(baseConfig.getUserProperties());
+        }
 
         if (anno.configurator() == null)
         {

jetty-websocket/javax-websocket-server-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/server/ServerContainer.java

@@ -91,7 +91,6 @@ public class ServerContainer extends ClientContainer implements javax.websocket.
         if (LOG.isDebugEnabled())
         {
             LOG.debug("addEndpoint({}) path={} endpoint={}",config,config.getPath(),config.getEndpointClass());
-            LOG.debug("Occurred from stack",new Throwable("stack"));
         }
         ServerEndpointMetadata metadata = getServerEndpointMetadata(config.getEndpointClass(),config);
         addEndpoint(metadata);

jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/SessionAltConfig.java

@@ -0,0 +1,49 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2013 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.jsr356.server;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import javax.websocket.Endpoint;
+import javax.websocket.server.ServerApplicationConfig;
+import javax.websocket.server.ServerEndpointConfig;
+
+public class SessionAltConfig implements ServerApplicationConfig
+{
+    @Override
+    public Set<ServerEndpointConfig> getEndpointConfigs(Set<Class<? extends Endpoint>> endpointClasses)
+    {
+        Set<ServerEndpointConfig> configs = new HashSet<>();
+        Class<?> endpointClass = SessionInfoSocket.class;
+        configs.add(ServerEndpointConfig.Builder.create(endpointClass,"/info/{a}/").build());
+        configs.add(ServerEndpointConfig.Builder.create(endpointClass,"/info/{a}/{b}/").build());
+        configs.add(ServerEndpointConfig.Builder.create(endpointClass,"/info/{a}/{b}/{c}/").build());
+        configs.add(ServerEndpointConfig.Builder.create(endpointClass,"/info/{a}/{b}/{c}/{d}/").build());
+        return configs;
+    }
+
+    @Override
+    public Set<Class<?>> getAnnotatedEndpointClasses(Set<Class<?>> scanned)
+    {
+        Set<Class<?>> annotated = new HashSet<>();
+        annotated.add(SessionInfoSocket.class);
+        return annotated;
+    }
+}

jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/SessionInfoSocket.java

@@ -0,0 +1,66 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2013 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.jsr356.server;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+
+import javax.websocket.OnMessage;
+import javax.websocket.Session;
+import javax.websocket.server.ServerEndpoint;
+
+@ServerEndpoint(value = "/info/")
+public class SessionInfoSocket
+{
+    @OnMessage
+    public String onMessage(Session session, String message)
+    {
+        if ("pathParams".equalsIgnoreCase(message))
+        {
+            StringBuilder ret = new StringBuilder();
+            ret.append("pathParams");
+            Map<String, String> pathParams = session.getPathParameters();
+            if (pathParams == null)
+            {
+                ret.append("=<null>");
+            }
+            else
+            {
+                ret.append('[').append(pathParams.size()).append(']');
+                List<String> keys = new ArrayList<>();
+                for (String key : pathParams.keySet())
+                {
+                    keys.add(key);
+                }
+                Collections.sort(keys);
+                for (String key : keys)
+                {
+                    String value = pathParams.get(key);
+                    ret.append(": '").append(key).append("'=").append(value);
+                }
+            }
+            return ret.toString();
+        }
+
+        // simple echo
+        return "echo:'" + message + "'";
+    }
+}

jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/SessionTest.java

@@ -0,0 +1,105 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2013 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.jsr356.server;
+
+import static org.hamcrest.Matchers.*;
+
+import java.net.URI;
+import java.util.Queue;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+
+import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
+import org.eclipse.jetty.webapp.WebAppContext;
+import org.eclipse.jetty.websocket.api.Session;
+import org.eclipse.jetty.websocket.client.WebSocketClient;
+import org.junit.AfterClass;
+import org.junit.Assert;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+public class SessionTest
+{
+    private static WSServer server;
+    private static URI serverUri;
+
+    @BeforeClass
+    public static void startServer() throws Exception
+    {
+        server = new WSServer(MavenTestingUtils.getTargetTestingDir(SessionTest.class.getSimpleName()),"app");
+        server.copyWebInf("empty-web.xml");
+        server.copyClass(SessionInfoSocket.class);
+        server.copyClass(SessionAltConfig.class);
+        server.start();
+        serverUri = server.getServerBaseURI();
+
+        WebAppContext webapp = server.createWebAppContext();
+        server.deployWebapp(webapp);
+    }
+
+    @AfterClass
+    public static void stopServer()
+    {
+        server.stop();
+    }
+
+    private void assertPathParams(String requestPath, String expectedResponse) throws Exception
+    {
+        WebSocketClient client = new WebSocketClient();
+        try
+        {
+            client.start();
+            JettyEchoSocket clientEcho = new JettyEchoSocket();
+            Future<Session> future = client.connect(clientEcho,serverUri.resolve(requestPath));
+            // wait for connect
+            future.get(1,TimeUnit.SECONDS);
+            clientEcho.sendMessage("pathParams");
+            Queue<String> msgs = clientEcho.awaitMessages(1);
+            Assert.assertThat("Expected message",msgs.poll(),is(expectedResponse));
+        }
+        finally
+        {
+            client.stop();
+        }
+    }
+
+    @Test
+    public void testPathParams_Empty() throws Exception
+    {
+        assertPathParams("info/","pathParams[0]");
+    }
+
+    @Test
+    public void testPathParams_Single() throws Exception
+    {
+        assertPathParams("info/apple/","pathParams[1]: 'a'=apple");
+    }
+
+    @Test
+    public void testPathParams_Double() throws Exception
+    {
+        assertPathParams("info/apple/pear/","pathParams[2]: 'a'=apple: 'b'=pear");
+    }
+    
+    @Test
+    public void testPathParams_Triple() throws Exception
+    {
+        assertPathParams("info/apple/pear/cherry/","pathParams[3]: 'a'=apple: 'b'=pear: 'c'=cherry");
+    }
+}

jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/WebSocketSession.java

@@ -27,9 +27,6 @@ import java.util.Map;
 import java.util.concurrent.Executor;
 
 import org.eclipse.jetty.io.ByteBufferPool;
-import org.eclipse.jetty.util.MultiMap;
-import org.eclipse.jetty.util.StringUtil;
-import org.eclipse.jetty.util.UrlEncoded;
 import org.eclipse.jetty.util.annotation.ManagedAttribute;
 import org.eclipse.jetty.util.annotation.ManagedObject;
 import org.eclipse.jetty.util.component.ContainerLifeCycle;
@@ -86,22 +83,6 @@ public class WebSocketSession extends ContainerLifeCycle implements Session, Inc
         this.incomingHandler = websocket;
 
         this.connection.getIOState().addListener(this);
-
-        // Get the parameter map (use the jetty MultiMap to do this right)
-        MultiMap<String> params = new MultiMap<>();
-        String query = requestURI.getQuery();
-        if (StringUtil.isNotBlank(query))
-        {
-            UrlEncoded.decodeTo(query,params,StringUtil.__UTF8_CHARSET,-1);
-        }
-
-        for (String name : params.keySet())
-        {
-            List<String> valueList = params.getValues(name);
-            String valueArr[] = new String[valueList.size()];
-            valueArr = valueList.toArray(valueArr);
-            parameterMap.put(name,valueArr);
-        }
     }
 
     @Override
@@ -428,6 +409,22 @@ public class WebSocketSession extends ContainerLifeCycle implements Session, Inc
     {
         this.upgradeRequest = request;
         this.protocolVersion = request.getProtocolVersion();
+        this.parameterMap.clear();
+        if (request.getParameterMap() != null)
+        {
+            for (Map.Entry<String, List<String>> entry : request.getParameterMap().entrySet())
+            {
+                List<String> values = entry.getValue();
+                if (values != null)
+                {
+                    this.parameterMap.put(entry.getKey(),values.toArray(new String[values.size()]));
+                }
+                else
+                {
+                    this.parameterMap.put(entry.getKey(),new String[0]);
+                }
+            }
+        }
     }
 
     public void setUpgradeResponse(UpgradeResponse response)