From 04f954956cc5942fbee4fec8f07d46c284cf3037 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 24 Jan 2018 22:13:40 +0100 Subject: [PATCH] ServletServerHttpRequest.getURI() ignores malformed query string The resolved URI instance is also being cached now. This should not make a difference in a real Servlet environment but does affect tests which assumed they could modify an HttpServletRequest path behind a pre-created ServletServerHttpRequest instance. Our WebSocket test base class has been revised accordingly, re-creating the ServletServerHttpRequest in such a case. Issue: SPR-16414 (cherry picked from commit 0e6f8df) --- .../org/springframework/http/HttpRequest.java | 5 +- .../http/server/ServletServerHttpRequest.java | 55 +++++++++----- .../server/ServletServerHttpRequestTests.java | 55 ++++++++++---- .../web/socket/AbstractHttpRequestTests.java | 5 +- .../server/DefaultHandshakeHandlerTests.java | 48 ++++++------- .../HandshakeInterceptorChainTests.java | 6 +- .../sockjs/support/SockJsServiceTests.java | 71 ++++++++++--------- .../handler/DefaultSockJsServiceTests.java | 5 +- .../HttpSendingTransportHandlerTests.java | 4 +- 9 files changed, 153 insertions(+), 101 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/HttpRequest.java b/spring-web/src/main/java/org/springframework/http/HttpRequest.java index 2f670a37e7..6913f2a8e8 100644 --- a/spring-web/src/main/java/org/springframework/http/HttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/HttpRequest.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2018 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. @@ -35,7 +35,8 @@ public interface HttpRequest extends HttpMessage { HttpMethod getMethod(); /** - * Return the URI of the request. + * Return the URI of the request (including a query string if any, + * but only if it is well-formed for a URI representation). * @return the URI of the request (never {@code null}) */ URI getURI(); diff --git a/spring-web/src/main/java/org/springframework/http/server/ServletServerHttpRequest.java b/spring-web/src/main/java/org/springframework/http/server/ServletServerHttpRequest.java index 05ba9d94a2..d65449c03e 100644 --- a/spring-web/src/main/java/org/springframework/http/server/ServletServerHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/server/ServletServerHttpRequest.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 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. @@ -59,6 +59,8 @@ public class ServletServerHttpRequest implements ServerHttpRequest { private final HttpServletRequest servletRequest; + private URI uri; + private HttpHeaders headers; private ServerHttpAsyncRequestControl asyncRequestControl; @@ -88,17 +90,36 @@ public class ServletServerHttpRequest implements ServerHttpRequest { @Override public URI getURI() { - try { - StringBuffer url = this.servletRequest.getRequestURL(); - String query = this.servletRequest.getQueryString(); - if (StringUtils.hasText(query)) { - url.append('?').append(query); + if (this.uri == null) { + String urlString = null; + boolean hasQuery = false; + try { + StringBuffer url = this.servletRequest.getRequestURL(); + String query = this.servletRequest.getQueryString(); + hasQuery = StringUtils.hasText(query); + if (hasQuery) { + url.append('?').append(query); + } + urlString = url.toString(); + this.uri = new URI(urlString); + } + catch (URISyntaxException ex) { + if (!hasQuery) { + throw new IllegalStateException( + "Could not resolve HttpServletRequest as URI: " + urlString, ex); + } + // Maybe a malformed query string... try plain request URL + try { + urlString = this.servletRequest.getRequestURL().toString(); + this.uri = new URI(urlString); + } + catch (URISyntaxException ex2) { + throw new IllegalStateException( + "Could not resolve HttpServletRequest as URI: " + urlString, ex2); + } } - return new URI(url.toString()); - } - catch (URISyntaxException ex) { - throw new IllegalStateException("Could not get HttpServletRequest URI: " + ex.getMessage(), ex); } + return this.uri; } @Override @@ -106,8 +127,8 @@ public class ServletServerHttpRequest implements ServerHttpRequest { if (this.headers == null) { this.headers = new HttpHeaders(); - for (Enumeration headerNames = this.servletRequest.getHeaderNames(); headerNames.hasMoreElements();) { - String headerName = (String) headerNames.nextElement(); + for (Enumeration names = this.servletRequest.getHeaderNames(); names.hasMoreElements();) { + String headerName = (String) names.nextElement(); for (Enumeration headerValues = this.servletRequest.getHeaders(headerName); headerValues.hasMoreElements();) { String headerValue = (String) headerValues.nextElement(); @@ -115,7 +136,8 @@ public class ServletServerHttpRequest implements ServerHttpRequest { } } - // HttpServletRequest exposes some headers as properties: we should include those if not already present + // HttpServletRequest exposes some headers as properties: + // we should include those if not already present try { MediaType contentType = this.headers.getContentType(); if (contentType == null) { @@ -132,8 +154,8 @@ public class ServletServerHttpRequest implements ServerHttpRequest { Map params = new LinkedCaseInsensitiveMap(); params.putAll(contentType.getParameters()); params.put("charset", charSet.toString()); - MediaType newContentType = new MediaType(contentType.getType(), contentType.getSubtype(), params); - this.headers.setContentType(newContentType); + MediaType mediaType = new MediaType(contentType.getType(), contentType.getSubtype(), params); + this.headers.setContentType(mediaType); } } } @@ -181,7 +203,8 @@ public class ServletServerHttpRequest implements ServerHttpRequest { public ServerHttpAsyncRequestControl getAsyncRequestControl(ServerHttpResponse response) { if (this.asyncRequestControl == null) { if (!ServletServerHttpResponse.class.isInstance(response)) { - throw new IllegalArgumentException("Response must be a ServletServerHttpResponse: " + response.getClass()); + throw new IllegalArgumentException( + "Response must be a ServletServerHttpResponse: " + response.getClass()); } ServletServerHttpResponse servletServerResponse = (ServletServerHttpResponse) response; this.asyncRequestControl = new ServletServerHttpAsyncRequestControl(this, servletServerResponse); diff --git a/spring-web/src/test/java/org/springframework/http/server/ServletServerHttpRequestTests.java b/spring-web/src/test/java/org/springframework/http/server/ServletServerHttpRequestTests.java index 2afbbe8e72..83274aaab2 100644 --- a/spring-web/src/test/java/org/springframework/http/server/ServletServerHttpRequestTests.java +++ b/spring-web/src/test/java/org/springframework/http/server/ServletServerHttpRequestTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2018 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. @@ -16,8 +16,10 @@ package org.springframework.http.server; +import java.io.IOException; import java.net.URI; -import java.nio.charset.Charset; +import java.net.URISyntaxException; +import java.nio.charset.StandardCharsets; import java.util.List; import org.junit.Before; @@ -33,6 +35,7 @@ import static org.junit.Assert.*; /** * @author Arjen Poutsma + * @author Juergen Hoeller */ public class ServletServerHttpRequestTests { @@ -42,30 +45,56 @@ public class ServletServerHttpRequestTests { @Before - public void create() throws Exception { + public void create() { mockRequest = new MockHttpServletRequest(); request = new ServletServerHttpRequest(mockRequest); } @Test - public void getMethod() throws Exception { + public void getMethod() { mockRequest.setMethod("POST"); assertEquals("Invalid method", HttpMethod.POST, request.getMethod()); } @Test - public void getURI() throws Exception { + public void getUriForSimplePath() throws URISyntaxException { + URI uri = new URI("http://example.com/path"); + mockRequest.setServerName(uri.getHost()); + mockRequest.setServerPort(uri.getPort()); + mockRequest.setRequestURI(uri.getPath()); + mockRequest.setQueryString(uri.getQuery()); + assertEquals(uri, request.getURI()); + } + + @Test + public void getUriWithQueryString() throws URISyntaxException { URI uri = new URI("http://example.com/path?query"); mockRequest.setServerName(uri.getHost()); mockRequest.setServerPort(uri.getPort()); mockRequest.setRequestURI(uri.getPath()); mockRequest.setQueryString(uri.getQuery()); - assertEquals("Invalid uri", uri, request.getURI()); + assertEquals(uri, request.getURI()); + } + + @Test // SPR-16414 + public void getUriWithQueryParam() throws URISyntaxException { + mockRequest.setServerName("example.com"); + mockRequest.setRequestURI("/path"); + mockRequest.setQueryString("query=foo"); + assertEquals(new URI("http://example.com/path?query=foo"), request.getURI()); + } + + @Test // SPR-16414 + public void getUriWithMalformedQueryParam() throws URISyntaxException { + mockRequest.setServerName("example.com"); + mockRequest.setRequestURI("/path"); + mockRequest.setQueryString("query=foo%%x"); + assertEquals(new URI("http://example.com/path"), request.getURI()); } @Test // SPR-13876 - public void getUriWithEncoding() throws Exception { + public void getUriWithEncoding() throws URISyntaxException { URI uri = new URI("https://example.com/%E4%B8%AD%E6%96%87" + "?redirect=https%3A%2F%2Fgithub.com%2Fspring-projects%2Fspring-framework"); mockRequest.setScheme(uri.getScheme()); @@ -73,11 +102,11 @@ public class ServletServerHttpRequestTests { mockRequest.setServerPort(uri.getPort()); mockRequest.setRequestURI(uri.getRawPath()); mockRequest.setQueryString(uri.getRawQuery()); - assertEquals("Invalid uri", uri, request.getURI()); + assertEquals(uri, request.getURI()); } @Test - public void getHeaders() throws Exception { + public void getHeaders() { String headerName = "MyHeader"; String headerValue1 = "value1"; String headerValue2 = "value2"; @@ -93,12 +122,12 @@ public class ServletServerHttpRequestTests { assertEquals("Invalid header values returned", 2, headerValues.size()); assertTrue("Invalid header values returned", headerValues.contains(headerValue1)); assertTrue("Invalid header values returned", headerValues.contains(headerValue2)); - assertEquals("Invalid Content-Type", new MediaType("text", "plain", Charset.forName("UTF-8")), + assertEquals("Invalid Content-Type", new MediaType("text", "plain", StandardCharsets.UTF_8), headers.getContentType()); } @Test - public void getHeadersWithEmptyContentTypeAndEncoding() throws Exception { + public void getHeadersWithEmptyContentTypeAndEncoding() { String headerName = "MyHeader"; String headerValue1 = "value1"; String headerValue2 = "value2"; @@ -118,7 +147,7 @@ public class ServletServerHttpRequestTests { } @Test - public void getBody() throws Exception { + public void getBody() throws IOException { byte[] content = "Hello World".getBytes("UTF-8"); mockRequest.setContent(content); @@ -127,7 +156,7 @@ public class ServletServerHttpRequestTests { } @Test - public void getFormBody() throws Exception { + public void getFormBody() throws IOException { // Charset (SPR-8676) mockRequest.setContentType("application/x-www-form-urlencoded; charset=UTF-8"); mockRequest.setMethod("POST"); diff --git a/spring-websocket/src/test/java/org/springframework/web/socket/AbstractHttpRequestTests.java b/spring-websocket/src/test/java/org/springframework/web/socket/AbstractHttpRequestTests.java index 2580b10997..015accf1ec 100644 --- a/spring-websocket/src/test/java/org/springframework/web/socket/AbstractHttpRequestTests.java +++ b/spring-websocket/src/test/java/org/springframework/web/socket/AbstractHttpRequestTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2018 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. @@ -45,13 +45,14 @@ public abstract class AbstractHttpRequestTests { @Before - public void setUp() { + public void setup() { resetRequestAndResponse(); } protected void setRequest(String method, String requestUri) { this.servletRequest.setMethod(method); this.servletRequest.setRequestURI(requestUri); + this.request = new ServletServerHttpRequest(this.servletRequest); } protected void resetRequestAndResponse() { diff --git a/spring-websocket/src/test/java/org/springframework/web/socket/server/DefaultHandshakeHandlerTests.java b/spring-websocket/src/test/java/org/springframework/web/socket/server/DefaultHandshakeHandlerTests.java index 254ff68061..ea7565a9ba 100644 --- a/spring-websocket/src/test/java/org/springframework/web/socket/server/DefaultHandshakeHandlerTests.java +++ b/spring-websocket/src/test/java/org/springframework/web/socket/server/DefaultHandshakeHandlerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2018 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. @@ -50,19 +50,18 @@ public class DefaultHandshakeHandlerTests extends AbstractHttpRequestTests { @Before - public void setup() throws Exception { + public void setup() { + super.setup(); + MockitoAnnotations.initMocks(this); this.handshakeHandler = new DefaultHandshakeHandler(this.upgradeStrategy); } @Test - public void supportedSubProtocols() throws Exception { - + public void supportedSubProtocols() { this.handshakeHandler.setSupportedProtocols("stomp", "mqtt"); - given(this.upgradeStrategy.getSupportedVersions()).willReturn(new String[] {"13"}); - this.servletRequest.setMethod("GET"); WebSocketHttpHeaders headers = new WebSocketHttpHeaders(this.request.getHeaders()); @@ -73,22 +72,20 @@ public class DefaultHandshakeHandlerTests extends AbstractHttpRequestTests { headers.setSecWebSocketProtocol("STOMP"); WebSocketHandler handler = new TextWebSocketHandler(); - Map attributes = Collections.emptyMap(); + Map attributes = Collections.emptyMap(); this.handshakeHandler.doHandshake(this.request, this.response, handler, attributes); - verify(this.upgradeStrategy).upgrade(this.request, this.response, - "STOMP", Collections.emptyList(), null, handler, attributes); + verify(this.upgradeStrategy).upgrade(this.request, this.response, "STOMP", + Collections.emptyList(), null, handler, attributes); } - @Test - public void supportedExtensions() throws Exception { - + public void supportedExtensions() { WebSocketExtension extension1 = new WebSocketExtension("ext1"); WebSocketExtension extension2 = new WebSocketExtension("ext2"); given(this.upgradeStrategy.getSupportedVersions()).willReturn(new String[] {"13"}); - given(this.upgradeStrategy.getSupportedExtensions(this.request)).willReturn(Arrays.asList(extension1)); + given(this.upgradeStrategy.getSupportedExtensions(this.request)).willReturn(Collections.singletonList(extension1)); this.servletRequest.setMethod("GET"); @@ -103,14 +100,13 @@ public class DefaultHandshakeHandlerTests extends AbstractHttpRequestTests { Map attributes = Collections.emptyMap(); this.handshakeHandler.doHandshake(this.request, this.response, handler, attributes); - verify(this.upgradeStrategy).upgrade(this.request, this.response, null, Arrays.asList(extension1), - null, handler, attributes); + verify(this.upgradeStrategy).upgrade(this.request, this.response, null, + Collections.singletonList(extension1), null, handler, attributes); } @Test - public void subProtocolCapableHandler() throws Exception { - - given(this.upgradeStrategy.getSupportedVersions()).willReturn(new String[]{"13"}); + public void subProtocolCapableHandler() { + given(this.upgradeStrategy.getSupportedVersions()).willReturn(new String[] {"13"}); this.servletRequest.setMethod("GET"); @@ -125,14 +121,13 @@ public class DefaultHandshakeHandlerTests extends AbstractHttpRequestTests { Map attributes = Collections.emptyMap(); this.handshakeHandler.doHandshake(this.request, this.response, handler, attributes); - verify(this.upgradeStrategy).upgrade(this.request, this.response, - "v11.stomp", Collections.emptyList(), null, handler, attributes); + verify(this.upgradeStrategy).upgrade(this.request, this.response, "v11.stomp", + Collections.emptyList(), null, handler, attributes); } @Test - public void subProtocolCapableHandlerNoMatch() throws Exception { - - given(this.upgradeStrategy.getSupportedVersions()).willReturn(new String[]{"13"}); + public void subProtocolCapableHandlerNoMatch() { + given(this.upgradeStrategy.getSupportedVersions()).willReturn(new String[] {"13"}); this.servletRequest.setMethod("GET"); @@ -147,8 +142,8 @@ public class DefaultHandshakeHandlerTests extends AbstractHttpRequestTests { Map attributes = Collections.emptyMap(); this.handshakeHandler.doHandshake(this.request, this.response, handler, attributes); - verify(this.upgradeStrategy).upgrade(this.request, this.response, - null, Collections.emptyList(), null, handler, attributes); + verify(this.upgradeStrategy).upgrade(this.request, this.response, null, + Collections.emptyList(), null, handler, attributes); } @@ -156,8 +151,7 @@ public class DefaultHandshakeHandlerTests extends AbstractHttpRequestTests { private final List subProtocols; - - private SubProtocolCapableHandler(String... subProtocols) { + public SubProtocolCapableHandler(String... subProtocols) { this.subProtocols = Arrays.asList(subProtocols); } diff --git a/spring-websocket/src/test/java/org/springframework/web/socket/server/support/HandshakeInterceptorChainTests.java b/spring-websocket/src/test/java/org/springframework/web/socket/server/support/HandshakeInterceptorChainTests.java index 157815dad3..6c719feb46 100644 --- a/spring-websocket/src/test/java/org/springframework/web/socket/server/support/HandshakeInterceptorChainTests.java +++ b/spring-websocket/src/test/java/org/springframework/web/socket/server/support/HandshakeInterceptorChainTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2018 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. @@ -52,12 +52,14 @@ public class HandshakeInterceptorChainTests extends AbstractHttpRequestTests { @Before public void setup() { + super.setup(); + i1 = mock(HandshakeInterceptor.class); i2 = mock(HandshakeInterceptor.class); i3 = mock(HandshakeInterceptor.class); interceptors = Arrays.asList(i1, i2, i3); wsHandler = mock(WebSocketHandler.class); - attributes = new HashMap(); + attributes = new HashMap<>(); } diff --git a/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/support/SockJsServiceTests.java b/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/support/SockJsServiceTests.java index 853b9603c0..2cb97ba106 100644 --- a/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/support/SockJsServiceTests.java +++ b/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/support/SockJsServiceTests.java @@ -18,12 +18,13 @@ package org.springframework.web.socket.sockjs.support; import java.io.IOException; import java.util.Arrays; - +import java.util.Collections; import javax.servlet.ServletOutputStream; import javax.servlet.http.HttpServletResponse; import org.junit.Before; import org.junit.Test; + import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.server.ServerHttpRequest; @@ -54,15 +55,14 @@ public class SockJsServiceTests extends AbstractHttpRequestTests { @Override @Before - public void setUp() { - super.setUp(); + public void setup() { + super.setup(); this.service = new TestSockJsService(new ThreadPoolTaskScheduler()); } @Test - public void validateRequest() throws Exception { - + public void validateRequest() { this.service.setWebSocketEnabled(false); resetResponseAndHandleRequest("GET", "/echo/server/session/websocket", HttpStatus.NOT_FOUND); @@ -81,11 +81,12 @@ public class SockJsServiceTests extends AbstractHttpRequestTests { } @Test - public void handleInfoGet() throws Exception { + public void handleInfoGet() throws IOException { resetResponseAndHandleRequest("GET", "/echo/info", HttpStatus.OK); assertEquals("application/json;charset=UTF-8", this.servletResponse.getContentType()); - assertEquals("no-store, no-cache, must-revalidate, max-age=0", this.servletResponse.getHeader(HttpHeaders.CACHE_CONTROL)); + String header = this.servletResponse.getHeader(HttpHeaders.CACHE_CONTROL); + assertEquals("no-store, no-cache, must-revalidate, max-age=0", header); assertNull(this.servletResponse.getHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN)); assertNull(this.servletResponse.getHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_CREDENTIALS)); assertNull(this.servletResponse.getHeader(HttpHeaders.VARY)); @@ -103,7 +104,7 @@ public class SockJsServiceTests extends AbstractHttpRequestTests { assertEquals(",\"origins\":[\"*:*\"],\"cookie_needed\":false,\"websocket\":false}", body.substring(body.indexOf(','))); - this.service.setAllowedOrigins(Arrays.asList("http://mydomain1.com")); + this.service.setAllowedOrigins(Collections.singletonList("http://mydomain1.com")); resetResponseAndHandleRequest("GET", "/echo/info", HttpStatus.OK); assertNull(this.servletResponse.getHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN)); assertNull(this.servletResponse.getHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_CREDENTIALS)); @@ -111,34 +112,35 @@ public class SockJsServiceTests extends AbstractHttpRequestTests { } @Test // SPR-12226 and SPR-12660 - public void handleInfoGetWithOrigin() throws Exception { + public void handleInfoGetWithOrigin() throws IOException { this.servletRequest.setServerName("mydomain2.com"); this.servletRequest.addHeader(HttpHeaders.ORIGIN, "http://mydomain2.com"); resetResponseAndHandleRequest("GET", "/echo/info", HttpStatus.OK); assertEquals("application/json;charset=UTF-8", this.servletResponse.getContentType()); - assertEquals("no-store, no-cache, must-revalidate, max-age=0", this.servletResponse.getHeader(HttpHeaders.CACHE_CONTROL)); + String header = this.servletResponse.getHeader(HttpHeaders.CACHE_CONTROL); + assertEquals("no-store, no-cache, must-revalidate, max-age=0", header); String body = this.servletResponse.getContentAsString(); assertEquals("{\"entropy\"", body.substring(0, body.indexOf(':'))); - assertEquals(",\"origins\":[\"*:*\"],\"cookie_needed\":true,\"websocket\":true}", body.substring(body.indexOf(','))); + assertEquals(",\"origins\":[\"*:*\"],\"cookie_needed\":true,\"websocket\":true}", + body.substring(body.indexOf(','))); - this.service.setAllowedOrigins(Arrays.asList("http://mydomain1.com")); + this.service.setAllowedOrigins(Collections.singletonList("http://mydomain1.com")); resetResponseAndHandleRequest("GET", "/echo/info", HttpStatus.OK); this.service.setAllowedOrigins(Arrays.asList("http://mydomain1.com", "http://mydomain2.com", "http://mydomain3.com")); resetResponseAndHandleRequest("GET", "/echo/info", HttpStatus.OK); - this.service.setAllowedOrigins(Arrays.asList("*")); + this.service.setAllowedOrigins(Collections.singletonList("*")); resetResponseAndHandleRequest("GET", "/echo/info", HttpStatus.OK); this.servletRequest.setServerName("mydomain3.com"); - this.service.setAllowedOrigins(Arrays.asList("http://mydomain1.com")); + this.service.setAllowedOrigins(Collections.singletonList("http://mydomain1.com")); resetResponseAndHandleRequest("GET", "/echo/info", HttpStatus.FORBIDDEN); } @Test // SPR-11443 - public void handleInfoGetCorsFilter() throws Exception { - + public void handleInfoGetCorsFilter() { // Simulate scenario where Filter would have already set CORS headers this.servletResponse.setHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN, "foobar:123"); @@ -148,8 +150,7 @@ public class SockJsServiceTests extends AbstractHttpRequestTests { } @Test // SPR-11919 - @SuppressWarnings("unchecked") - public void handleInfoGetWildflyNPE() throws Exception { + public void handleInfoGetWildflyNPE() throws IOException { HttpServletResponse mockResponse = mock(HttpServletResponse.class); ServletOutputStream ous = mock(ServletOutputStream.class); given(mockResponse.getHeaders(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN)).willThrow(NullPointerException.class); @@ -162,18 +163,18 @@ public class SockJsServiceTests extends AbstractHttpRequestTests { } @Test // SPR-12660 - public void handleInfoOptions() throws Exception { + public void handleInfoOptions() { this.servletRequest.addHeader(HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS, "Last-Modified"); resetResponseAndHandleRequest("OPTIONS", "/echo/info", HttpStatus.NO_CONTENT); assertNull(this.service.getCorsConfiguration(this.servletRequest)); - this.service.setAllowedOrigins(Arrays.asList("http://mydomain1.com")); + this.service.setAllowedOrigins(Collections.singletonList("http://mydomain1.com")); resetResponseAndHandleRequest("OPTIONS", "/echo/info", HttpStatus.NO_CONTENT); assertNull(this.service.getCorsConfiguration(this.servletRequest)); } @Test // SPR-12226 and SPR-12660 - public void handleInfoOptionsWithAllowedOrigin() throws Exception { + public void handleInfoOptionsWithAllowedOrigin() { this.servletRequest.setServerName("mydomain2.com"); this.servletRequest.addHeader(HttpHeaders.ORIGIN, "http://mydomain2.com"); this.servletRequest.addHeader(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, "GET"); @@ -181,7 +182,7 @@ public class SockJsServiceTests extends AbstractHttpRequestTests { resetResponseAndHandleRequest("OPTIONS", "/echo/info", HttpStatus.NO_CONTENT); assertNotNull(this.service.getCorsConfiguration(this.servletRequest)); - this.service.setAllowedOrigins(Arrays.asList("http://mydomain1.com")); + this.service.setAllowedOrigins(Collections.singletonList("http://mydomain1.com")); resetResponseAndHandleRequest("OPTIONS", "/echo/info", HttpStatus.NO_CONTENT); assertNotNull(this.service.getCorsConfiguration(this.servletRequest)); @@ -189,13 +190,13 @@ public class SockJsServiceTests extends AbstractHttpRequestTests { resetResponseAndHandleRequest("OPTIONS", "/echo/info", HttpStatus.NO_CONTENT); assertNotNull(this.service.getCorsConfiguration(this.servletRequest)); - this.service.setAllowedOrigins(Arrays.asList("*")); + this.service.setAllowedOrigins(Collections.singletonList("*")); resetResponseAndHandleRequest("OPTIONS", "/echo/info", HttpStatus.NO_CONTENT); assertNotNull(this.service.getCorsConfiguration(this.servletRequest)); } @Test // SPR-16304 - public void handleInfoOptionsWithForbiddenOrigin() throws Exception { + public void handleInfoOptionsWithForbiddenOrigin() { this.servletRequest.setServerName("mydomain3.com"); this.servletRequest.addHeader(HttpHeaders.ORIGIN, "http://mydomain2.com"); this.servletRequest.addHeader(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, "GET"); @@ -204,23 +205,23 @@ public class SockJsServiceTests extends AbstractHttpRequestTests { CorsConfiguration corsConfiguration = this.service.getCorsConfiguration(this.servletRequest); assertTrue(corsConfiguration.getAllowedOrigins().isEmpty()); - this.service.setAllowedOrigins(Arrays.asList("http://mydomain1.com")); + this.service.setAllowedOrigins(Collections.singletonList("http://mydomain1.com")); resetResponseAndHandleRequest("OPTIONS", "/echo/info", HttpStatus.FORBIDDEN); corsConfiguration = this.service.getCorsConfiguration(this.servletRequest); - assertEquals(Arrays.asList("http://mydomain1.com"), corsConfiguration.getAllowedOrigins()); + assertEquals(Collections.singletonList("http://mydomain1.com"), corsConfiguration.getAllowedOrigins()); } @Test // SPR-12283 - public void handleInfoOptionsWithOriginAndCorsHeadersDisabled() throws Exception { + public void handleInfoOptionsWithOriginAndCorsHeadersDisabled() { this.servletRequest.addHeader(HttpHeaders.ORIGIN, "http://mydomain2.com"); - this.service.setAllowedOrigins(Arrays.asList("*")); + this.service.setAllowedOrigins(Collections.singletonList("*")); this.service.setSuppressCors(true); this.servletRequest.addHeader(HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS, "Last-Modified"); resetResponseAndHandleRequest("OPTIONS", "/echo/info", HttpStatus.NO_CONTENT); assertNull(this.service.getCorsConfiguration(this.servletRequest)); - this.service.setAllowedOrigins(Arrays.asList("http://mydomain1.com")); + this.service.setAllowedOrigins(Collections.singletonList("http://mydomain1.com")); resetResponseAndHandleRequest("OPTIONS", "/echo/info", HttpStatus.FORBIDDEN); assertNull(this.service.getCorsConfiguration(this.servletRequest)); @@ -230,7 +231,7 @@ public class SockJsServiceTests extends AbstractHttpRequestTests { } @Test - public void handleIframeRequest() throws Exception { + public void handleIframeRequest() throws IOException { resetResponseAndHandleRequest("GET", "/echo/iframe.html", HttpStatus.OK); assertEquals("text/html;charset=UTF-8", this.servletResponse.getContentType()); @@ -241,13 +242,13 @@ public class SockJsServiceTests extends AbstractHttpRequestTests { } @Test - public void handleIframeRequestNotModified() throws Exception { + public void handleIframeRequestNotModified() { this.servletRequest.addHeader("If-None-Match", "\"0096cbd37f2a5218c33bb0826a7c74cbf\""); resetResponseAndHandleRequest("GET", "/echo/iframe.html", HttpStatus.NOT_MODIFIED); } @Test - public void handleRawWebSocketRequest() throws Exception { + public void handleRawWebSocketRequest() throws IOException { resetResponseAndHandleRequest("GET", "/echo", HttpStatus.OK); assertEquals("Welcome to SockJS!\n", this.servletResponse.getContentAsString()); @@ -257,7 +258,7 @@ public class SockJsServiceTests extends AbstractHttpRequestTests { } @Test - public void handleEmptyContentType() throws Exception { + public void handleEmptyContentType() { this.servletRequest.setContentType(""); resetResponseAndHandleRequest("GET", "/echo/info", HttpStatus.OK); @@ -265,12 +266,12 @@ public class SockJsServiceTests extends AbstractHttpRequestTests { } - private void resetResponseAndHandleRequest(String httpMethod, String uri, HttpStatus httpStatus) throws IOException { + private void resetResponseAndHandleRequest(String httpMethod, String uri, HttpStatus httpStatus) { resetResponse(); handleRequest(httpMethod, uri, httpStatus); } - private void handleRequest(String httpMethod, String uri, HttpStatus httpStatus) throws IOException { + private void handleRequest(String httpMethod, String uri, HttpStatus httpStatus) { setRequest(httpMethod, uri); String sockJsPath = uri.substring("/echo".length()); this.service.handleRequest(this.request, this.response, sockJsPath, this.handler); diff --git a/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/transport/handler/DefaultSockJsServiceTests.java b/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/transport/handler/DefaultSockJsServiceTests.java index a45a3e701b..4f013d8ec0 100644 --- a/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/transport/handler/DefaultSockJsServiceTests.java +++ b/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/transport/handler/DefaultSockJsServiceTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2018 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. @@ -80,7 +80,7 @@ public class DefaultSockJsServiceTests extends AbstractHttpRequestTests { @Before public void setup() { - super.setUp(); + super.setup(); MockitoAnnotations.initMocks(this); Map attributes = Collections.emptyMap(); @@ -97,6 +97,7 @@ public class DefaultSockJsServiceTests extends AbstractHttpRequestTests { this.service = new TransportHandlingSockJsService(this.taskScheduler, this.xhrHandler, this.xhrSendHandler); } + @Test public void defaultTransportHandlers() { DefaultSockJsService service = new DefaultSockJsService(mock(TaskScheduler.class)); diff --git a/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/transport/handler/HttpSendingTransportHandlerTests.java b/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/transport/handler/HttpSendingTransportHandlerTests.java index 3b2501001e..2b07da5fb1 100644 --- a/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/transport/handler/HttpSendingTransportHandlerTests.java +++ b/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/transport/handler/HttpSendingTransportHandlerTests.java @@ -52,8 +52,8 @@ public class HttpSendingTransportHandlerTests extends AbstractHttpRequestTests @Override @Before - public void setUp() { - super.setUp(); + public void setup() { + super.setup(); this.webSocketHandler = mock(WebSocketHandler.class); this.taskScheduler = mock(TaskScheduler.class);