399173: UpgradeRequest.getParameterMap() should never return null

+ Making api.UpgradeRequest never return null, but also have no logic on
  how to populate the parameter map
+ Using MultiMap in websocket-client for parameter map parsing
+ Using HttpServletRequest.getParameterMap() as-is in websocket-server
+ Adding unit testing for both sides
This commit is contained in:
Joakim Erdfelt 2013-01-28 17:10:23 -07:00
parent ec254cd165
commit 335611815c
9 changed files with 360 additions and 39 deletions

View File

@ -21,6 +21,7 @@ package org.eclipse.jetty.websocket.api;
import java.net.HttpCookie;
import java.net.URI;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@ -35,6 +36,7 @@ public class UpgradeRequest
private List<ExtensionConfig> extensions = new ArrayList<>();
private List<HttpCookie> cookies = new ArrayList<>();
private Map<String, List<String>> headers = new HashMap<>();
private Map<String, String[]> parameters = new HashMap<>();
private Object session;
private String httpVersion;
private String method;
@ -168,10 +170,14 @@ public class UpgradeRequest
return getHeader("Origin");
}
/**
* Returns a map of the query parameters of the request.
*
* @return a unmodifiable map of query parameters of the request.
*/
public Map<String, String[]> getParameterMap()
{
// TODO Auto-generated method stub
return null;
return Collections.unmodifiableMap(parameters);
}
public String getQueryString()
@ -238,11 +244,17 @@ public class UpgradeRequest
this.method = method;
}
protected void setParameterMap(Map<String, String[]> parameters)
{
this.parameters.clear();
this.parameters.putAll(parameters);
}
public void setRequestURI(URI uri)
{
this.requestURI = uri;
this.host = this.requestURI.getHost();
// TODO: parse parameters
this.parameters.clear();
}
public void setSession(Object session)

View File

@ -21,13 +21,17 @@ package org.eclipse.jetty.websocket.client;
import java.net.CookieStore;
import java.net.HttpCookie;
import java.net.URI;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Random;
import java.util.Set;
import org.eclipse.jetty.util.B64Code;
import org.eclipse.jetty.util.MultiMap;
import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.util.UrlEncoded;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
import org.eclipse.jetty.websocket.api.UpgradeRequest;
@ -39,6 +43,7 @@ import org.eclipse.jetty.websocket.api.extensions.ExtensionConfig;
public class ClientUpgradeRequest extends UpgradeRequest
{
private final static Logger LOG = Log.getLogger(ClientUpgradeRequest.class);
private final static int MAX_KEYS = -1; // maximum number of parameter keys to decode
private static final Set<String> FORBIDDEN_HEADERS;
static
@ -203,4 +208,37 @@ public class ClientUpgradeRequest extends UpgradeRequest
setCookies(cookieStore.get(getRequestURI()));
}
@Override
public void setRequestURI(URI uri)
{
super.setRequestURI(uri);
// parse parameter map
Map<String, String[]> pmap = new HashMap<>();
String query = uri.getQuery();
if (StringUtil.isNotBlank(query))
{
MultiMap<String> params = new MultiMap<String>();
UrlEncoded.decodeTo(uri.getQuery(),params,"UTF-8",MAX_KEYS);
for (String key : params.keySet())
{
List<String> values = params.getValues(key);
if (values == null)
{
pmap.put(key,new String[0]);
}
else
{
int len = values.size();
pmap.put(key,values.toArray(new String[len]));
}
}
super.setParameterMap(pmap);
}
}
}

View File

@ -22,11 +22,13 @@ import static org.hamcrest.Matchers.*;
import java.net.InetSocketAddress;
import java.net.URI;
import java.util.Map;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import org.eclipse.jetty.toolchain.test.AdvancedRunner;
import org.eclipse.jetty.websocket.api.Session;
import org.eclipse.jetty.websocket.api.UpgradeRequest;
import org.eclipse.jetty.websocket.client.blockhead.BlockheadServer;
import org.eclipse.jetty.websocket.client.blockhead.BlockheadServer.ServerConnection;
import org.eclipse.jetty.websocket.common.WebSocketFrame;
@ -207,4 +209,45 @@ public class WebSocketClientTest
factSmall.stop();
}
}
@Test
public void testParameterMap() throws Exception
{
WebSocketClient fact = new WebSocketClient();
fact.start();
try
{
TrackingSocket wsocket = new TrackingSocket();
URI wsUri = server.getWsUri().resolve("/test?snack=cashews&amount=handful&brand=off");
Future<Session> future = client.connect(wsocket,wsUri);
ServerConnection ssocket = server.accept();
ssocket.upgrade();
future.get(500,TimeUnit.MILLISECONDS);
Assert.assertTrue(wsocket.openLatch.await(1,TimeUnit.SECONDS));
Session session = (Session)wsocket.getConnection();
UpgradeRequest req = session.getUpgradeRequest();
Assert.assertThat("Upgrade Request",req,notNullValue());
Map<String, String[]> parameterMap = req.getParameterMap();
Assert.assertThat("Parameter Map",parameterMap,notNullValue());
Assert.assertThat("Parameter[snack]",parameterMap.get("snack"),is(new String[]
{ "cashews" }));
Assert.assertThat("Parameter[amount]",parameterMap.get("amount"),is(new String[]
{ "handful" }));
Assert.assertThat("Parameter[brand]",parameterMap.get("brand"),is(new String[]
{ "off" }));
Assert.assertThat("Parameter[cost]",parameterMap.get("cost"),nullValue());
}
finally
{
fact.stop();
}
}
}

View File

@ -49,6 +49,9 @@ public class ServletWebSocketRequest extends UpgradeRequest
setMethod(request.getMethod());
setHttpVersion(request.getProtocol());
// Copy parameters
super.setParameterMap(request.getParameterMap());
// Copy Cookies
cookieMap = new HashMap<String, String>();
for (Cookie cookie : request.getCookies())
@ -98,6 +101,40 @@ public class ServletWebSocketRequest extends UpgradeRequest
}
}
public Principal getPrincipal()
{
return req.getUserPrincipal();
}
public StringBuffer getRequestURL()
{
return req.getRequestURL();
}
public Map<String, Object> getServletAttributes()
{
Map<String, Object> attributes = new HashMap<String, Object>();
for (String name : Collections.list(req.getAttributeNames()))
{
attributes.put(name,req.getAttribute(name));
}
return attributes;
}
public Map<String, List<String>> getServletParameters()
{
Map<String, List<String>> parameters = new HashMap<String, List<String>>();
for (String name : Collections.list(req.getParameterNames()))
{
parameters.put(name,Collections.unmodifiableList(Arrays.asList(req.getParameterValues(name))));
}
return parameters;
}
protected String[] parseProtocols(String protocol)
{
if (protocol == null)
@ -121,38 +158,4 @@ public class ServletWebSocketRequest extends UpgradeRequest
{
this.req.setAttribute(name,o);
}
public Principal getPrincipal()
{
return req.getUserPrincipal();
}
public StringBuffer getRequestURL()
{
return req.getRequestURL();
}
public Map<String, Object> getServletAttributes()
{
Map<String, Object> attributes = new HashMap<String,Object>();
for (String name : Collections.list((Enumeration<String>)req.getAttributeNames()))
{
attributes.put(name, req.getAttribute(name));
}
return attributes;
}
public Map<String, List<String>> getServletParameters()
{
Map<String, List<String>> parameters = new HashMap<String, List<String>>();
for (String name : Collections.list((Enumeration<String>)req.getParameterNames()))
{
parameters.put(name, Collections.unmodifiableList(Arrays.asList(req.getParameterValues(name))));
}
return parameters;
}
}

View File

@ -0,0 +1,92 @@
//
// ========================================================================
// 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.server;
import static org.hamcrest.Matchers.*;
import java.net.URI;
import java.util.concurrent.TimeUnit;
import org.eclipse.jetty.toolchain.test.AdvancedRunner;
import org.eclipse.jetty.websocket.api.Session;
import org.eclipse.jetty.websocket.common.WebSocketFrame;
import org.eclipse.jetty.websocket.server.blockhead.BlockheadClient;
import org.eclipse.jetty.websocket.server.helper.IncomingFramesCapture;
import org.eclipse.jetty.websocket.server.helper.SessionServlet;
import org.junit.AfterClass;
import org.junit.Assert;
import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.runner.RunWith;
/**
* Testing various aspects of the server side support for WebSocket {@link Session}
*/
@RunWith(AdvancedRunner.class)
public class WebSocketServerSessionTest
{
private static SimpleServletServer server;
@BeforeClass
public static void startServer() throws Exception
{
server = new SimpleServletServer(new SessionServlet());
server.start();
}
@AfterClass
public static void stopServer()
{
server.stop();
}
@Test
public void testUpgradeRequestResponse() throws Exception
{
URI uri = server.getServerUri().resolve("/test?snack=cashews&amount=handful&brand=off");
BlockheadClient client = new BlockheadClient(uri);
try
{
client.connect();
client.sendStandardRequest();
client.expectUpgradeResponse();
// Ask the server socket for specific parameter map info
client.write(WebSocketFrame.text("getParameterMap|snack"));
client.write(WebSocketFrame.text("getParameterMap|amount"));
client.write(WebSocketFrame.text("getParameterMap|brand"));
client.write(WebSocketFrame.text("getParameterMap|cost")); // intentionall invalid
// Read frame (hopefully text frame)
IncomingFramesCapture capture = client.readFrames(4,TimeUnit.MILLISECONDS,500);
WebSocketFrame tf = capture.getFrames().pop();
Assert.assertThat("Parameter Map[snack]",tf.getPayloadAsUTF8(),is("[cashews]"));
tf = capture.getFrames().pop();
Assert.assertThat("Parameter Map[amount]",tf.getPayloadAsUTF8(),is("[handful]"));
tf = capture.getFrames().pop();
Assert.assertThat("Parameter Map[brand]",tf.getPayloadAsUTF8(),is("[off]"));
tf = capture.getFrames().pop();
Assert.assertThat("Parameter Map[cost]",tf.getPayloadAsUTF8(),is("<null>"));
}
finally
{
client.close();
}
}
}

View File

@ -130,7 +130,8 @@ public class BlockheadClient implements IncomingFrames, OutgoingFrames
{
scheme = "https";
}
this.destHttpURI = new URI(scheme,destWebsocketURI.getSchemeSpecificPart(),destWebsocketURI.getFragment());
this.destHttpURI = new URI(scheme,destWebsocketURI.getUserInfo(),destWebsocketURI.getHost(),destWebsocketURI.getPort(),destWebsocketURI.getPath(),
destWebsocketURI.getQuery(),destWebsocketURI.getFragment());
this.bufferPool = new MappedByteBufferPool(8192);
this.generator = new Generator(policy,bufferPool);
@ -556,7 +557,13 @@ public class BlockheadClient implements IncomingFrames, OutgoingFrames
public void sendStandardRequest() throws IOException
{
StringBuilder req = new StringBuilder();
req.append("GET /chat HTTP/1.1\r\n");
req.append("GET ");
req.append(destHttpURI.getPath());
if (StringUtil.isNotBlank(destHttpURI.getQuery()))
{
req.append('?').append(destHttpURI.getQuery());
}
req.append(" HTTP/1.1\r\n");
req.append("Host: ").append(destHttpURI.getHost());
if (destHttpURI.getPort() > 0)
{

View File

@ -0,0 +1,32 @@
//
// ========================================================================
// 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.server.helper;
import org.eclipse.jetty.websocket.servlet.WebSocketServlet;
import org.eclipse.jetty.websocket.servlet.WebSocketServletFactory;
@SuppressWarnings("serial")
public class SessionServlet extends WebSocketServlet
{
@Override
public void configure(WebSocketServletFactory factory)
{
factory.register(SessionSocket.class);
}
}

View File

@ -0,0 +1,93 @@
//
// ========================================================================
// 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.server.helper;
import java.util.Map;
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.WebSocketConnection;
import org.eclipse.jetty.websocket.api.annotations.OnWebSocketConnect;
import org.eclipse.jetty.websocket.api.annotations.OnWebSocketMessage;
import org.eclipse.jetty.websocket.api.annotations.WebSocket;
@WebSocket
public class SessionSocket
{
private static final Logger LOG = Log.getLogger(SessionSocket.class);
private Session session;
@OnWebSocketConnect
public void onConnect(WebSocketConnection conn)
{
this.session = (Session)conn;
}
@OnWebSocketMessage
public void onText(String message)
{
LOG.debug("onText({})",message);
if (message == null)
{
return;
}
try
{
if (message.startsWith("getParameterMap"))
{
Map<String, String[]> parameterMap = session.getUpgradeRequest().getParameterMap();
int idx = message.indexOf('|');
String key = message.substring(idx + 1);
String values[] = parameterMap.get(key);
if (values == null)
{
session.getRemote().sendStringByFuture("<null>");
return;
}
StringBuilder valueStr = new StringBuilder();
valueStr.append('[');
boolean delim = false;
for (String value : values)
{
if (delim)
{
valueStr.append(", ");
}
valueStr.append(value);
delim = true;
}
valueStr.append(']');
session.getRemote().sendStringByFuture(valueStr.toString());
return;
}
// echo the message back.
this.session.getRemote().sendStringByFuture(message);
}
catch (Throwable t)
{
LOG.warn(t);
}
}
}

View File

@ -10,6 +10,7 @@ org.eclipse.jetty.LEVEL=WARN
# org.eclipse.jetty.websocket.common.Generator.LEVEL=DEBUG
# org.eclipse.jetty.websocket.server.ab.Fuzzer.LEVEL=DEBUG
# org.eclipse.jetty.websocket.server.blockhead.LEVEL=DEBUG
# org.eclipse.jetty.websocket.server.helper.LEVEL=DEBUG
### Show state changes on BrowserDebugTool
# org.eclipse.jetty.websocket.server.browser.LEVEL=DEBUG