Apply allowedOriginPatterns in SockJsService

See gh-26108

Author: rstoyanchev

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

@@ -25,7 +25,6 @@
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Random;
-import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
 import javax.servlet.http.HttpServletRequest;
@@ -97,9 +96,7 @@ public abstract class AbstractSockJsService implements SockJsService, CorsConfig
 
 	private boolean suppressCors = false;
 
-	protected final Set<String> allowedOrigins = new LinkedHashSet<>();
-
-	protected final Set<String> allowedOriginPatterns = new LinkedHashSet<>();
+	protected final CorsConfiguration corsConfiguration;
 
 	private final SockJsRequestHandler infoHandler = new InfoHandler();
 
@@ -109,6 +106,18 @@ public abstract class AbstractSockJsService implements SockJsService, CorsConfig
 	public AbstractSockJsService(TaskScheduler scheduler) {
 		Assert.notNull(scheduler, "TaskScheduler must not be null");
 		this.taskScheduler = scheduler;
+		this.corsConfiguration = initCorsConfiguration();
+	}
+
+	private static CorsConfiguration initCorsConfiguration() {
+		CorsConfiguration config = new CorsConfiguration();
+		config.addAllowedMethod("*");
+		config.setAllowedOrigins(Collections.emptyList());
+		config.setAllowedOriginPatterns(Collections.emptyList());
+		config.setAllowCredentials(true);
+		config.setMaxAge(ONE_YEAR);
+		config.addAllowedHeader("*");
+		return config;
 	}
 
 
@@ -317,10 +326,18 @@ public boolean shouldSuppressCors() {
 	 */
 	public void setAllowedOrigins(Collection<String> allowedOrigins) {
 		Assert.notNull(allowedOrigins, "Allowed origins Collection must not be null");
-		this.allowedOrigins.clear();
-		this.allowedOrigins.addAll(allowedOrigins);
+		this.corsConfiguration.setAllowedOrigins(new ArrayList<>(allowedOrigins));
 	}
 
+	/**
+	 * Return configure allowed {@code Origin} header values.
+	 * @since 4.1.2
+	 * @see #setAllowedOrigins
+	 */
+	@SuppressWarnings("ConstantConditions")
+	public Collection<String> getAllowedOrigins() {
+		return this.corsConfiguration.getAllowedOrigins();
+	}
 	/**
 	 * A variant of {@link #setAllowedOrigins(Collection)} that accepts flexible
 	 * domain patterns, e.g. {@code "https://*.domain1.com"}. Furthermore it
@@ -331,26 +348,17 @@ public void setAllowedOrigins(Collection<String> allowedOrigins) {
 	 */
 	public void setAllowedOriginPatterns(Collection<String> allowedOriginPatterns) {
 		Assert.notNull(allowedOriginPatterns, "Allowed origin patterns Collection must not be null");
-		this.allowedOriginPatterns.clear();
-		this.allowedOriginPatterns.addAll(allowedOriginPatterns);
-	}
-
-	/**
-	 * Return configure allowed {@code Origin} header values.
-	 * @since 4.1.2
-	 * @see #setAllowedOrigins
-	 */
-	public Collection<String> getAllowedOrigins() {
-		return Collections.unmodifiableSet(this.allowedOrigins);
+		this.corsConfiguration.setAllowedOriginPatterns(new ArrayList<>(allowedOriginPatterns));
 	}
 
 	/**
-	 * Return configure allowed {@code Origin} pattern header values.
+	 * Return {@link #setAllowedOriginPatterns(Collection) configured} origin patterns.
 	 * @since 5.3.2
 	 * @see #setAllowedOriginPatterns
 	 */
+	@SuppressWarnings("ConstantConditions")
 	public Collection<String> getAllowedOriginPatterns() {
-		return Collections.unmodifiableSet(this.allowedOriginPatterns);
+		return this.corsConfiguration.getAllowedOriginPatterns();
 	}
 
 
@@ -396,15 +404,16 @@ else if (sockJsPath.equals("/info")) {
 			}
 
 			else if (sockJsPath.matches("/iframe[0-9-.a-z_]*.html")) {
-				if (!this.allowedOrigins.isEmpty() && !this.allowedOrigins.contains("*")) {
+				if (!getAllowedOrigins().isEmpty() && !getAllowedOrigins().contains("*") ||
+						!getAllowedOriginPatterns().isEmpty()) {
 					if (requestInfo != null) {
 						logger.debug("Iframe support is disabled when an origin check is required. " +
 								"Ignoring transport request: " + requestInfo);
 					}
 					response.setStatusCode(HttpStatus.NOT_FOUND);
 					return;
 				}
-				if (this.allowedOrigins.isEmpty()) {
+				if (getAllowedOrigins().isEmpty()) {
 					response.getHeaders().add(XFRAME_OPTIONS_HEADER, "SAMEORIGIN");
 				}
 				if (requestInfo != null) {
@@ -506,7 +515,7 @@ protected boolean checkOrigin(ServerHttpRequest request, ServerHttpResponse resp
 			return true;
 		}
 
-		if (!WebUtils.isValidOrigin(request, this.allowedOrigins)) {
+		if (this.corsConfiguration.checkOrigin(request.getHeaders().getOrigin()) == null) {
 			if (logger.isWarnEnabled()) {
 				logger.warn("Origin header value '" + request.getHeaders().getOrigin() + "' not allowed.");
 			}
@@ -521,14 +530,7 @@ protected boolean checkOrigin(ServerHttpRequest request, ServerHttpResponse resp
 	@Nullable
 	public CorsConfiguration getCorsConfiguration(HttpServletRequest request) {
 		if (!this.suppressCors && (request.getHeader(HttpHeaders.ORIGIN) != null)) {
-			CorsConfiguration config = new CorsConfiguration();
-			config.setAllowedOrigins(new ArrayList<>(this.allowedOrigins));
-			config.setAllowedOriginPatterns(new ArrayList<>(this.allowedOriginPatterns));
-			config.addAllowedMethod("*");
-			config.setAllowCredentials(true);
-			config.setMaxAge(ONE_YEAR);
-			config.addAllowedHeader("*");
-			return config;
+			return this.corsConfiguration;
 		}
 		return null;
 	}

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

@@ -344,7 +344,8 @@ protected boolean validateRequest(String serverId, String sessionId, String tran
 			return false;
 		}
 
-		if (!this.allowedOrigins.contains("*")) {
+		if (!getAllowedOrigins().isEmpty() && !getAllowedOrigins().contains("*") ||
+				!getAllowedOriginPatterns().isEmpty()) {
 			TransportType transportType = TransportType.fromValue(transport);
 			if (transportType == null || !transportType.supportsOrigin()) {
 				if (logger.isWarnEnabled()) {

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

@@ -215,18 +215,20 @@ public void handleInfoOptionsWithForbiddenOrigin() {
 	@Test  // SPR-12283
 	public void handleInfoOptionsWithOriginAndCorsHeadersDisabled() {
 		this.servletRequest.addHeader(HttpHeaders.ORIGIN, "https://mydomain2.example");
-		this.service.setAllowedOrigins(Collections.singletonList("*"));
+		this.service.setAllowedOriginPatterns(Collections.singletonList("*"));
 		this.service.setSuppressCors(true);
 
 		this.servletRequest.addHeader(HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS, "Last-Modified");
 		resetResponseAndHandleRequest("OPTIONS", "/echo/info", HttpStatus.NO_CONTENT);
 		assertThat(this.service.getCorsConfiguration(this.servletRequest)).isNull();
 
 		this.service.setAllowedOrigins(Collections.singletonList("https://mydomain1.example"));
+		this.service.setAllowedOriginPatterns(Collections.emptyList());
 		resetResponseAndHandleRequest("OPTIONS", "/echo/info", HttpStatus.FORBIDDEN);
 		assertThat(this.service.getCorsConfiguration(this.servletRequest)).isNull();
 
 		this.service.setAllowedOrigins(Arrays.asList("https://mydomain1.example", "https://mydomain2.example", "http://mydomain3.example"));
+		this.service.setAllowedOriginPatterns(Collections.emptyList());
 		resetResponseAndHandleRequest("OPTIONS", "/echo/info", HttpStatus.NO_CONTENT);
 		assertThat(this.service.getCorsConfiguration(this.servletRequest)).isNull();
 	}