From 50d98ab52773f5f1a060a811f9f6ac540d61ac16 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Fri, 6 Sep 2013 13:40:49 -0700 Subject: [PATCH] 416763 - WebSocket / Jsr Session.getPathParameters() is empty + Ensuring this works as intended with new test case --- .../jetty/websocket/jsr356/JsrSession.java | 1 + .../server/AnnotatedServerEndpointConfig.java | 61 +++++++--- .../jsr356/server/ServerContainer.java | 1 - .../jsr356/server/SessionAltConfig.java | 49 ++++++++ .../jsr356/server/SessionInfoSocket.java | 66 +++++++++++ .../websocket/jsr356/server/SessionTest.java | 105 ++++++++++++++++++ .../websocket/common/WebSocketSession.java | 35 +++--- 7 files changed, 281 insertions(+), 37 deletions(-) create mode 100644 jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/SessionAltConfig.java create mode 100644 jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/SessionInfoSocket.java create mode 100644 jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/SessionTest.java diff --git a/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/JsrSession.java b/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/JsrSession.java index ab62677625d..c57299364e9 100644 --- a/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/JsrSession.java +++ b/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 diff --git a/jetty-websocket/javax-websocket-server-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/server/AnnotatedServerEndpointConfig.java b/jetty-websocket/javax-websocket-server-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/server/AnnotatedServerEndpointConfig.java index ed33f390b8d..684699142f1 100644 --- a/jetty-websocket/javax-websocket-server-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/server/AnnotatedServerEndpointConfig.java +++ b/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> compositeDecoders = new ArrayList<>(); - List> compositeEncoders = new ArrayList<>(); - List 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 - compositeDecoders.addAll(Arrays.asList(anno.decoders())); - compositeEncoders.addAll(Arrays.asList(anno.encoders())); - compositeSubProtocols.addAll(Arrays.asList(anno.subprotocols())); + // Decoders (favor provided config over annotation) + if (baseConfig != null && baseConfig.getDecoders() != null && baseConfig.getDecoders().size() > 0) + { + this.decoders = Collections.unmodifiableList(baseConfig.getDecoders()); + } + else + { + this.decoders = Collections.unmodifiableList(Arrays.asList(anno.decoders())); + } - // Create unmodifiable lists - this.decoders = Collections.unmodifiableList(compositeDecoders); - this.encoders = Collections.unmodifiableList(compositeEncoders); - this.subprotocols = Collections.unmodifiableList(compositeSubProtocols); + // Encoders (favor provided config over annotation) + if (baseConfig != null && baseConfig.getEncoders() != null && baseConfig.getEncoders().size() > 0) + { + 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) { diff --git a/jetty-websocket/javax-websocket-server-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/server/ServerContainer.java b/jetty-websocket/javax-websocket-server-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/server/ServerContainer.java index 2e58bcf6377..77ecbba8182 100644 --- a/jetty-websocket/javax-websocket-server-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/server/ServerContainer.java +++ b/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); diff --git a/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/SessionAltConfig.java b/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/SessionAltConfig.java new file mode 100644 index 00000000000..06dd48933e1 --- /dev/null +++ b/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 getEndpointConfigs(Set> endpointClasses) + { + Set 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> getAnnotatedEndpointClasses(Set> scanned) + { + Set> annotated = new HashSet<>(); + annotated.add(SessionInfoSocket.class); + return annotated; + } +} diff --git a/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/SessionInfoSocket.java b/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/SessionInfoSocket.java new file mode 100644 index 00000000000..cb73a0687e5 --- /dev/null +++ b/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 pathParams = session.getPathParameters(); + if (pathParams == null) + { + ret.append("="); + } + else + { + ret.append('[').append(pathParams.size()).append(']'); + List 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 + "'"; + } +} diff --git a/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/SessionTest.java b/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/SessionTest.java new file mode 100644 index 00000000000..ba67a6eb47b --- /dev/null +++ b/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 future = client.connect(clientEcho,serverUri.resolve(requestPath)); + // wait for connect + future.get(1,TimeUnit.SECONDS); + clientEcho.sendMessage("pathParams"); + Queue 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"); + } +} diff --git a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/WebSocketSession.java b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/WebSocketSession.java index 10e0b43543c..6188a29ef1e 100644 --- a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/WebSocketSession.java +++ b/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 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 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> entry : request.getParameterMap().entrySet()) + { + List 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)