Polish ServletServerHttpRequest query param handling

The method returning query parameters now returns only query string
parameters as opposed to any Servlet request parameter.

This commit also adds a ReadOnlyMultiValueMap.

spring-core/src/main/java/org/springframework/util/ReadOnlyMultiValueMap.java

@@ -0,0 +1,150 @@
+/*
+ * Copyright 2002-2013 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.util;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+
+/**
+ * Wraps a {@link MultiValueMap} to make it immutable.
+ *
+ * @author Rossen Stoyanchev
+ * @since 4.0
+ */
+public class ReadOnlyMultiValueMap<K, V> implements MultiValueMap<K, V> {
+
+	private final MultiValueMap<K, V> targetMap;
+
+
+	/**
+	 * Create a new ReadOnlyMultiValueMap that wraps the given target map.
+	 */
+	public ReadOnlyMultiValueMap(MultiValueMap<K, V> targetMap) {
+		this.targetMap = targetMap;
+	}
+
+	// MultiValueMap implementation
+
+	@Override
+	public void add(K key, V value) {
+		throw new UnsupportedOperationException("This map is immutable.");
+	}
+
+	@Override
+	public V getFirst(K key) {
+		return this.targetMap.getFirst(key);
+	}
+
+	@Override
+	public void set(K key, V value) {
+		throw new UnsupportedOperationException("This map is immutable.");
+	}
+
+	@Override
+	public void setAll(Map<K, V> values) {
+		throw new UnsupportedOperationException("This map is immutable.");
+	}
+
+	@Override
+	public Map<K, V> toSingleValueMap() {
+		return Collections.unmodifiableMap(this.targetMap.toSingleValueMap());
+	}
+
+
+	// Map implementation
+
+	@Override
+	public int size() {
+		return this.targetMap.size();
+	}
+
+	@Override
+	public boolean isEmpty() {
+		return this.targetMap.isEmpty();
+	}
+
+	@Override
+	public boolean containsKey(Object key) {
+		return this.targetMap.containsKey(key);
+	}
+
+	@Override
+	public boolean containsValue(Object value) {
+		return this.targetMap.containsValue(value);
+	}
+
+	@Override
+	public List<V> get(Object key) {
+		return this.targetMap.get(key);
+	}
+
+	@Override
+	public List<V> put(K key, List<V> value) {
+		throw new UnsupportedOperationException("This map is immutable.");
+	}
+
+	@Override
+	public List<V> remove(Object key) {
+		throw new UnsupportedOperationException("This map is immutable.");
+	}
+
+	@Override
+	public void putAll(Map<? extends K, ? extends List<V>> m) {
+		this.targetMap.putAll(m);
+	}
+
+	@Override
+	public void clear() {
+		throw new UnsupportedOperationException("This map is immutable.");
+	}
+
+	@Override
+	public Set<K> keySet() {
+		return Collections.unmodifiableSet(this.targetMap.keySet());
+	}
+
+	@Override
+	public Collection<List<V>> values() {
+		return Collections.unmodifiableCollection(this.targetMap.values());
+	}
+
+	@Override
+	public Set<Entry<K, List<V>>> entrySet() {
+		return Collections.unmodifiableSet(this.targetMap.entrySet());
+	}
+
+
+	@Override
+	public boolean equals(Object obj) {
+		return this.targetMap.equals(obj);
+	}
+
+	@Override
+	public int hashCode() {
+		return this.targetMap.hashCode();
+	}
+
+	@Override
+	public String toString() {
+		return this.targetMap.toString();
+	}
+
+}

spring-web/src/main/java/org/springframework/http/MediaType.java

@@ -686,7 +686,9 @@ public class MediaType implements Comparable<MediaType> {
 	 * @throws InvalidMediaTypeException if the string cannot be parsed
 	 */
 	public static MediaType parseMediaType(String mediaType) {
-		Assert.hasLength(mediaType, "'mediaType' must not be empty");
+		if (!StringUtils.hasLength(mediaType)) {
+			throw new InvalidMediaTypeException(mediaType, "'mediaType' must not be empty");
+		}
 		String[] parts = StringUtils.tokenizeToStringArray(mediaType, ";");
 
 		String fullType = parts[0].trim();

spring-web/src/main/java/org/springframework/http/server/ServerHttpResponse.java

@@ -37,7 +37,9 @@ public interface ServerHttpResponse extends HttpOutputMessage, Closeable {
 	void setStatusCode(HttpStatus status);
 
 	/**
-	 * TODO
+	 * Ensure the headers and the content of the response are written out. After the first
+	 * flush, headers can no longer be changed, only further content writing and flushing
+	 * is possible.
 	 */
 	void flush() throws IOException;
 

spring-web/src/main/java/org/springframework/http/server/ServletServerHttpRequest.java

@@ -34,6 +34,8 @@ import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import javax.servlet.http.HttpServletRequest;
 
@@ -44,6 +46,7 @@ import org.springframework.http.MediaType;
 import org.springframework.util.Assert;
 import org.springframework.util.LinkedMultiValueMap;
 import org.springframework.util.MultiValueMap;
+import org.springframework.util.ReadOnlyMultiValueMap;
 
 /**
  * {@link ServerHttpRequest} implementation that is based on a {@link HttpServletRequest}.
@@ -59,6 +62,8 @@ public class ServletServerHttpRequest implements ServerHttpRequest {
 
 	private static final String METHOD_POST = "POST";
 
+	private static final Pattern QUERY_PARAM_PATTERN = Pattern.compile("([^&=]+)(=?)([^&]+)?");
+
 	private final HttpServletRequest servletRequest;
 
 	private HttpHeaders headers;
@@ -167,13 +172,19 @@ public class ServletServerHttpRequest implements ServerHttpRequest {
 	@Override
 	public MultiValueMap<String, String> getQueryParams() {
 		if (this.queryParams == null) {
-			// TODO: extract from query string
-			this.queryParams = new LinkedMultiValueMap<String, String>(this.servletRequest.getParameterMap().size());
-			for (String name : this.servletRequest.getParameterMap().keySet()) {
-				for (String value : this.servletRequest.getParameterValues(name)) {
-					this.queryParams.add(name, value);
+			MultiValueMap<String, String> result = new LinkedMultiValueMap<String, String>();
+			String queryString = this.servletRequest.getQueryString();
+			if (queryString != null) {
+				Matcher m = QUERY_PARAM_PATTERN.matcher(queryString);
+				while (m.find()) {
+					String name = m.group(1);
+					String[] values = this.servletRequest.getParameterValues(name);
+					if (values != null) {
+						result.put(name, Arrays.asList(values));
+					}
 				}
 			}
+			this.queryParams = new ReadOnlyMultiValueMap<String, String>(result);
 		}
 		return this.queryParams;
 	}

spring-web/src/test/java/org/springframework/http/server/ServletServerHttpRequestTests.java

@@ -18,11 +18,11 @@ package org.springframework.http.server;
 
 import java.net.URI;
 import java.nio.charset.Charset;
+import java.util.Arrays;
 import java.util.List;
 
 import org.junit.Before;
 import org.junit.Test;
-
 import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpMethod;
 import org.springframework.http.MediaType;
@@ -106,4 +106,21 @@ public class ServletServerHttpRequestTests {
 		assertArrayEquals("Invalid content returned", content, result);
 	}
 
+	@Test
+	public void getQueryParams() throws Exception {
+		mockRequest.setQueryString("foo=bar");
+		mockRequest.addParameter("foo", "bar");
+		mockRequest.addParameter("a", "b");
+		assertEquals(Arrays.asList("bar"), request.getQueryParams().get("foo"));
+		assertNull(request.getQueryParams().get("a"));
+	}
+
+	@Test
+	public void getQueryParamsTwoValues() throws Exception {
+		mockRequest.setQueryString("baz=qux&baz=42");
+		mockRequest.addParameter("baz", "qux");
+		mockRequest.addParameter("baz", "42");
+		assertEquals(Arrays.asList("qux", "42"), request.getQueryParams().get("baz"));
+	}
+
 }

spring-websocket/src/main/java/org/springframework/web/socket/sockjs/SockJsService.java

@@ -33,6 +33,16 @@ import org.springframework.web.socket.WebSocketHandler;
 public interface SockJsService {
 
 
+	/**
+	 * Process a SockJS request.
+	 *
+	 * @param request the current request
+	 * @param response the current response
+	 * @param handler the handler to process messages with
+	 *
+	 * @throws IOException raised if writing the to response of the current request fails
+	 * @throws SockJsProcessingException
+	 */
 	void handleRequest(ServerHttpRequest request, ServerHttpResponse response, WebSocketHandler handler)
 			throws IOException, SockJsProcessingException;
 

spring-websocket/src/main/java/org/springframework/web/socket/sockjs/support/AbstractSockJsService.java

@@ -34,6 +34,7 @@ import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.springframework.http.HttpMethod;
 import org.springframework.http.HttpStatus;
+import org.springframework.http.InvalidMediaTypeException;
 import org.springframework.http.MediaType;
 import org.springframework.http.server.ServerHttpRequest;
 import org.springframework.http.server.ServerHttpResponse;
@@ -283,8 +284,8 @@ public abstract class AbstractSockJsService implements SockJsService {
 		try {
 			request.getHeaders();
 		}
-		catch (IllegalArgumentException ex) {
-			// Ignore invalid Content-Type (TODO)
+		catch (InvalidMediaTypeException ex) {
+			logger.warn("Invalid media type ignored: " + ex.getMediaType());
 		}
 
 		try {

spring-websocket/src/test/java/org/springframework/web/socket/sockjs/support/AbstractSockJsServiceTests.java

@@ -164,6 +164,7 @@ public class AbstractSockJsServiceTests extends AbstractHttpRequestTests {
 		this.servletRequest.addHeader("Access-Control-Request-Headers", "Last-Modified");
 
 		handleRequest("OPTIONS", "/a/info", HttpStatus.NO_CONTENT);
+		this.response.flush();
 
 		assertEquals("*", this.servletResponse.getHeader("Access-Control-Allow-Origin"));
 		assertEquals("true", this.servletResponse.getHeader("Access-Control-Allow-Credentials"));
@@ -214,6 +215,15 @@ public class AbstractSockJsServiceTests extends AbstractHttpRequestTests {
 		assertEquals(httpStatus.value(), this.servletResponse.getStatus());
 	}
 
+	@Test
+	public void handleEmptyContentType() throws Exception {
+
+		servletRequest.setContentType("");
+		handleRequest("GET", "/a/info", HttpStatus.OK);
+
+		assertEquals("Invalid/empty content should have been ignored", 200, this.servletResponse.getStatus());
+	}
+
 
 	private static class TestSockJsService extends AbstractSockJsService {
 
@@ -228,16 +238,15 @@ public class AbstractSockJsServiceTests extends AbstractHttpRequestTests {
 		}
 
 		@Override
-		protected void handleRawWebSocketRequest(ServerHttpRequest request,
-				ServerHttpResponse response, WebSocketHandler handler) throws IOException {
+		protected void handleRawWebSocketRequest(ServerHttpRequest req, ServerHttpResponse res,
+				WebSocketHandler handler) throws IOException {
 
 			this.handler = handler;
 		}
 
 		@Override
-		protected void handleTransportRequest(ServerHttpRequest request, ServerHttpResponse response,
-				WebSocketHandler handler, String sessionId, String transport)
-						throws IOException, SockJsProcessingException {
+		protected void handleTransportRequest(ServerHttpRequest req, ServerHttpResponse res, WebSocketHandler handler,
+				String sessionId, String transport) throws IOException, SockJsProcessingException {
 
 			this.sessionId = sessionId;
 			this.transport = transport;

spring-websocket/src/test/java/org/springframework/web/socket/sockjs/transport/handler/DefaultSockJsServiceTests.java

@@ -130,6 +130,7 @@ public class DefaultSockJsServiceTests extends AbstractHttpRequestTests {
 		setRequest("POST", sessionUrlPrefix + "xhr");
 		this.service.setDummySessionCookieEnabled(true);
 		this.service.handleRequest(this.request, this.response, this.wsHandler);
+		this.response.flush();
 
 		assertEquals(200, this.servletResponse.getStatus());
 		assertEquals("JSESSIONID=dummy;path=/", this.servletResponse.getHeader("Set-Cookie"));

spring-websocket/src/test/java/org/springframework/web/socket/sockjs/transport/handler/HttpSendingTransportHandlerTests.java

@@ -25,12 +25,6 @@ import org.springframework.web.socket.AbstractHttpRequestTests;
 import org.springframework.web.socket.WebSocketHandler;
 import org.springframework.web.socket.sockjs.support.frame.SockJsFrame;
 import org.springframework.web.socket.sockjs.support.frame.SockJsFrame.FrameFormat;
-import org.springframework.web.socket.sockjs.transport.handler.AbstractHttpSendingTransportHandler;
-import org.springframework.web.socket.sockjs.transport.handler.EventSourceTransportHandler;
-import org.springframework.web.socket.sockjs.transport.handler.HtmlFileTransportHandler;
-import org.springframework.web.socket.sockjs.transport.handler.JsonpPollingTransportHandler;
-import org.springframework.web.socket.sockjs.transport.handler.XhrPollingTransportHandler;
-import org.springframework.web.socket.sockjs.transport.handler.XhrStreamingTransportHandler;
 import org.springframework.web.socket.sockjs.transport.session.AbstractSockJsSession;
 import org.springframework.web.socket.sockjs.transport.session.PollingSockJsSession;
 import org.springframework.web.socket.sockjs.transport.session.StreamingSockJsSession;
@@ -106,6 +100,7 @@ public class HttpSendingTransportHandlerTests  extends AbstractHttpRequestTests
 		assertEquals("\"callback\" parameter required", this.servletResponse.getContentAsString());
 
 		resetRequestAndResponse();
+		this.servletRequest.setQueryString("c=callback");
 		this.servletRequest.addParameter("c", "callback");
 		transportHandler.handleRequest(this.request, this.response, this.webSocketHandler, session);
 
@@ -141,6 +136,7 @@ public class HttpSendingTransportHandlerTests  extends AbstractHttpRequestTests
 		assertEquals("\"callback\" parameter required", this.servletResponse.getContentAsString());
 
 		resetRequestAndResponse();
+		this.servletRequest.setQueryString("c=callback");
 		this.servletRequest.addParameter("c", "callback");
 		transportHandler.handleRequest(this.request, this.response, this.webSocketHandler, session);
 
@@ -166,6 +162,7 @@ public class HttpSendingTransportHandlerTests  extends AbstractHttpRequestTests
 	@Test
 	public void frameFormats() throws Exception {
 
+		this.servletRequest.setQueryString("c=callback");
 		this.servletRequest.addParameter("c", "callback");
 
 		SockJsFrame frame = SockJsFrame.openFrame();