From 4fc9afa1023935b674f624933fc2510702d1c2b5 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Sun, 30 Mar 2014 09:19:06 +0200 Subject: [PATCH] Adding origins whitelist support for CORS Motivation: Currently the CORS support only handles a single origin, or a wildcard origin. This task should enhance Netty's CORS support to allow multiple origins to be specified. Just being allowed to specify one origin is particulary limiting when a site support both http and https for example. Modifications: - Updated CorsConfig and its Builder to accept multiple origins. Result: Users are now able to configure multiple origins for CORS. [https://github.com/netty/netty/issues/2346] --- .../handler/codec/http/cors/CorsConfig.java | 66 ++++++++++++++++--- .../handler/codec/http/cors/CorsHandler.java | 19 ++++-- .../codec/http/cors/CorsConfigTest.java | 43 ++++++++---- .../codec/http/cors/CorsHandlerTest.java | 30 +++++++-- .../http/cors/HttpServerInitializer.java | 2 +- 5 files changed, 127 insertions(+), 33 deletions(-) diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsConfig.java b/codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsConfig.java index 5cd7004626..b7b2b0dcc0 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsConfig.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsConfig.java @@ -26,6 +26,7 @@ import java.util.Collections; import java.util.Date; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.Map; import java.util.Map.Entry; import java.util.Set; @@ -36,7 +37,8 @@ import java.util.concurrent.Callable; */ public final class CorsConfig { - private final String origin; + private final Set origins; + private final boolean anyOrigin; private final boolean enabled; private final Set exposeHeaders; private final boolean allowCredentials; @@ -47,7 +49,8 @@ public final class CorsConfig { private final Map> preflightHeaders; private CorsConfig(final Builder builder) { - origin = builder.origin; + origins = new LinkedHashSet(builder.origins); + anyOrigin = builder.anyOrigin; enabled = builder.enabled; exposeHeaders = builder.exposeHeaders; allowCredentials = builder.allowCredentials; @@ -67,13 +70,31 @@ public final class CorsConfig { return enabled; } + /** + * Determines whether a wildcard origin, '*', is supported. + * + * @return {@code boolean} true if any origin is allowed. + */ + public boolean isAnyOriginSupported() { + return anyOrigin; + } + /** * Returns the allowed origin. This can either be a wildcard or an origin value. * * @return the value that will be used for the CORS response header 'Access-Control-Allow-Origin' */ public String origin() { - return origin; + return origins.isEmpty() ? "*" : origins.iterator().next(); + } + + /** + * Returns the set of allowed origins. + * + * @return {@code Set} the allowed origins. + */ + public Set origins() { + return origins; } /** @@ -204,7 +225,8 @@ public final class CorsConfig { @Override public String toString() { return StringUtil.simpleClassName(this) + "[enabled=" + enabled + - ", origin=" + origin + + ", origins=" + origins + + ", anyOrigin=" + anyOrigin + ", exposedHeaders=" + exposeHeaders + ", isCredentialsAllowed=" + allowCredentials + ", maxAge=" + maxAge + @@ -218,8 +240,8 @@ public final class CorsConfig { * * @return Builder to support method chaining. */ - public static Builder anyOrigin() { - return new Builder("*"); + public static Builder withAnyOrigin() { + return new Builder(); } /** @@ -228,15 +250,28 @@ public final class CorsConfig { * @return {@link Builder} to support method chaining. */ public static Builder withOrigin(final String origin) { + if (origin.equals("*")) { + return new Builder(); + } return new Builder(origin); } + /** + * Creates a {@link Builder} instance with the specified origins. + * + * @return {@link Builder} to support method chaining. + */ + public static Builder withOrigins(final String... origins) { + return new Builder(origins); + } + /** * Builder used to configure and build a CorsConfig instance. */ public static class Builder { - private final String origin; + private final Set origins; + private final boolean anyOrigin; private boolean allowNullOrigin; private boolean enabled = true; private boolean allowCredentials; @@ -250,10 +285,21 @@ public final class CorsConfig { /** * Creates a new Builder instance with the origin passed in. * - * @param origin the origin to be used for this builder. + * @param origins the origin to be used for this builder. */ - public Builder(final String origin) { - this.origin = origin; + public Builder(final String... origins) { + this.origins = new LinkedHashSet(Arrays.asList(origins)); + anyOrigin = false; + } + + /** + * Creates a new Builder instance allowing any origin, "*" which is the + * wildcard origin. + * + */ + public Builder() { + anyOrigin = true; + origins = Collections.emptySet(); } /** diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsHandler.java b/codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsHandler.java index f8bd594d73..abb08fbe52 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsHandler.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsHandler.java @@ -85,15 +85,26 @@ public class CorsHandler extends ChannelDuplexHandler { final String origin = request.headers().get(ORIGIN); if (origin != null) { if ("null".equals(origin) && config.isNullOriginAllowed()) { - response.headers().set(ACCESS_CONTROL_ALLOW_ORIGIN, "*"); - } else { - response.headers().set(ACCESS_CONTROL_ALLOW_ORIGIN, config.origin()); + setAnyOrigin(response); + return true; } - return true; + if (config.isAnyOriginSupported()) { + setAnyOrigin(response); + return true; + } + if (config.origins().contains(origin)) { + response.headers().set(ACCESS_CONTROL_ALLOW_ORIGIN, origin); + return true; + } + logger.debug("Request origin [" + origin + "] was not among the configured origins " + config.origins()); } return false; } + private static void setAnyOrigin(final HttpResponse response) { + response.headers().set(ACCESS_CONTROL_ALLOW_ORIGIN, "*"); + } + private void setAllowCredentials(final HttpResponse response) { if (config.isCredentialsAllowed()) { response.headers().set(ACCESS_CONTROL_ALLOW_CREDENTIALS, "true"); diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/cors/CorsConfigTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/cors/CorsConfigTest.java index e0dfed8962..3f091825ab 100644 --- a/codec-http/src/test/java/io/netty/handler/codec/http/cors/CorsConfigTest.java +++ b/codec-http/src/test/java/io/netty/handler/codec/http/cors/CorsConfigTest.java @@ -28,74 +28,93 @@ public class CorsConfigTest { @Test public void disabled() { - final CorsConfig cors = withOrigin("*").disable().build(); + final CorsConfig cors = withAnyOrigin().disable().build(); assertThat(cors.isCorsSupportEnabled(), is(false)); } + @Test + public void anyOrigin() { + final CorsConfig cors = withAnyOrigin().build(); + assertThat(cors.isAnyOriginSupported(), is(true)); + assertThat(cors.origin(), is("*")); + assertThat(cors.origins().isEmpty(), is(true)); + } + @Test public void wildcardOrigin() { - final CorsConfig cors = anyOrigin().build(); - assertThat(cors.origin(), is(equalTo("*"))); + final CorsConfig cors = withOrigin("*").build(); + assertThat(cors.isAnyOriginSupported(), is(true)); + assertThat(cors.origin(), equalTo("*")); + assertThat(cors.origins().isEmpty(), is(true)); } @Test public void origin() { final CorsConfig cors = withOrigin("http://localhost:7888").build(); assertThat(cors.origin(), is(equalTo("http://localhost:7888"))); + assertThat(cors.isAnyOriginSupported(), is(false)); + } + + @Test + public void origins() { + final String[] origins = {"http://localhost:7888", "https://localhost:7888"}; + final CorsConfig cors = withOrigins(origins).build(); + assertThat(cors.origins(), hasItems(origins)); + assertThat(cors.isAnyOriginSupported(), is(false)); } @Test public void exposeHeaders() { - final CorsConfig cors = withOrigin("*").exposeHeaders("custom-header1", "custom-header2").build(); + final CorsConfig cors = withAnyOrigin().exposeHeaders("custom-header1", "custom-header2").build(); assertThat(cors.exposedHeaders(), hasItems("custom-header1", "custom-header2")); } @Test public void allowCredentials() { - final CorsConfig cors = withOrigin("*").allowCredentials().build(); + final CorsConfig cors = withAnyOrigin().allowCredentials().build(); assertThat(cors.isCredentialsAllowed(), is(true)); } @Test public void maxAge() { - final CorsConfig cors = withOrigin("*").maxAge(3000).build(); + final CorsConfig cors = withAnyOrigin().maxAge(3000).build(); assertThat(cors.maxAge(), is(3000L)); } @Test public void requestMethods() { - final CorsConfig cors = withOrigin("*").allowedRequestMethods(HttpMethod.POST, HttpMethod.GET).build(); + final CorsConfig cors = withAnyOrigin().allowedRequestMethods(HttpMethod.POST, HttpMethod.GET).build(); assertThat(cors.allowedRequestMethods(), hasItems(HttpMethod.POST, HttpMethod.GET)); } @Test public void requestHeaders() { - final CorsConfig cors = withOrigin("*").allowedRequestHeaders("preflight-header1", "preflight-header2").build(); + final CorsConfig cors = withAnyOrigin().allowedRequestHeaders("preflight-header1", "preflight-header2").build(); assertThat(cors.allowedRequestHeaders(), hasItems("preflight-header1", "preflight-header2")); } @Test public void preflightResponseHeadersSingleValue() { - final CorsConfig cors = withOrigin("*").preflightResponseHeader("SingleValue", "value").build(); + final CorsConfig cors = withAnyOrigin().preflightResponseHeader("SingleValue", "value").build(); assertThat(cors.preflightResponseHeaders().get("SingleValue"), equalTo("value")); } @Test public void preflightResponseHeadersMultipleValues() { - final CorsConfig cors = withOrigin("*").preflightResponseHeader("MultipleValues", "value1", "value2").build(); + final CorsConfig cors = withAnyOrigin().preflightResponseHeader("MultipleValues", "value1", "value2").build(); assertThat(cors.preflightResponseHeaders().getAll("MultipleValues"), hasItems("value1", "value2")); } @Test public void defaultPreflightResponseHeaders() { - final CorsConfig cors = withOrigin("*").build(); + final CorsConfig cors = withAnyOrigin().build(); assertThat(cors.preflightResponseHeaders().get(Names.DATE), is(notNullValue())); assertThat(cors.preflightResponseHeaders().get(Names.CONTENT_LENGTH), is("0")); } @Test public void emptyPreflightResponseHeaders() { - final CorsConfig cors = withOrigin("*").noPreflightResponseHeaders().build(); + final CorsConfig cors = withAnyOrigin().noPreflightResponseHeaders().build(); assertThat(cors.preflightResponseHeaders(), equalTo(HttpHeaders.EMPTY_HEADERS)); } diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/cors/CorsHandlerTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/cors/CorsHandlerTest.java index 8dc2d83040..a10a44d227 100644 --- a/codec-http/src/test/java/io/netty/handler/codec/http/cors/CorsHandlerTest.java +++ b/codec-http/src/test/java/io/netty/handler/codec/http/cors/CorsHandlerTest.java @@ -39,13 +39,13 @@ public class CorsHandlerTest { @Test public void nonCorsRequest() { - final HttpResponse response = simpleRequest(CorsConfig.anyOrigin().build(), null); + final HttpResponse response = simpleRequest(CorsConfig.withAnyOrigin().build(), null); assertThat(response.headers().contains(ACCESS_CONTROL_ALLOW_ORIGIN), is(false)); } @Test public void simpleRequestWithAnyOrigin() { - final HttpResponse response = simpleRequest(CorsConfig.anyOrigin().build(), "http://localhost:7777"); + final HttpResponse response = simpleRequest(CorsConfig.withAnyOrigin().build(), "http://localhost:7777"); assertThat(response.headers().get(ACCESS_CONTROL_ALLOW_ORIGIN), is("*")); } @@ -56,6 +56,24 @@ public class CorsHandlerTest { assertThat(response.headers().get(ACCESS_CONTROL_ALLOW_ORIGIN), is(origin)); } + @Test + public void simpleRequestWithOrigins() { + final String origin1 = "http://localhost:8888"; + final String origin2 = "https://localhost:8888"; + final String[] origins = {origin1, origin2}; + final HttpResponse response1 = simpleRequest(CorsConfig.withOrigins(origins).build(), origin1); + assertThat(response1.headers().get(ACCESS_CONTROL_ALLOW_ORIGIN), is(origin1)); + final HttpResponse response2 = simpleRequest(CorsConfig.withOrigins(origins).build(), origin2); + assertThat(response2.headers().get(ACCESS_CONTROL_ALLOW_ORIGIN), is(origin2)); + } + + @Test + public void simpleRequestWithNoMatchingOrigin() { + final String origin = "http://localhost:8888"; + final HttpResponse response = simpleRequest(CorsConfig.withOrigins("https://localhost:8888").build(), origin); + assertThat(response.headers().get(ACCESS_CONTROL_ALLOW_ORIGIN), is(nullValue())); + } + @Test public void preflightDeleteRequestWithCustomHeaders() { final CorsConfig config = CorsConfig.withOrigin("http://localhost:8888") @@ -152,7 +170,7 @@ public class CorsHandlerTest { @Test public void simpleRequestCustomHeaders() { - final CorsConfig config = CorsConfig.anyOrigin().exposeHeaders("custom1", "custom2").build(); + final CorsConfig config = CorsConfig.withAnyOrigin().exposeHeaders("custom1", "custom2").build(); final HttpResponse response = simpleRequest(config, "http://localhost:7777", ""); assertThat(response.headers().get(ACCESS_CONTROL_ALLOW_ORIGIN), equalTo("*")); assertThat(response.headers().getAll(ACCESS_CONTROL_EXPOSE_HEADERS), hasItems("custom1", "custom1")); @@ -160,21 +178,21 @@ public class CorsHandlerTest { @Test public void simpleRequestAllowCredentials() { - final CorsConfig config = CorsConfig.anyOrigin().allowCredentials().build(); + final CorsConfig config = CorsConfig.withAnyOrigin().allowCredentials().build(); final HttpResponse response = simpleRequest(config, "http://localhost:7777", ""); assertThat(response.headers().get(ACCESS_CONTROL_ALLOW_CREDENTIALS), equalTo("true")); } @Test public void simpleRequestDoNotAllowCredentials() { - final CorsConfig config = CorsConfig.anyOrigin().build(); + final CorsConfig config = CorsConfig.withAnyOrigin().build(); final HttpResponse response = simpleRequest(config, "http://localhost:7777", ""); assertThat(response.headers().contains(ACCESS_CONTROL_ALLOW_CREDENTIALS), is(false)); } @Test public void simpleRequestExposeHeaders() { - final CorsConfig config = CorsConfig.anyOrigin().exposeHeaders("one", "two").build(); + final CorsConfig config = CorsConfig.withAnyOrigin().exposeHeaders("one", "two").build(); final HttpResponse response = simpleRequest(config, "http://localhost:7777", ""); assertThat(response.headers().getAll(ACCESS_CONTROL_EXPOSE_HEADERS), hasItems("one", "two")); } diff --git a/example/src/main/java/io/netty/example/http/cors/HttpServerInitializer.java b/example/src/main/java/io/netty/example/http/cors/HttpServerInitializer.java index 2297299b1d..4e53a30917 100644 --- a/example/src/main/java/io/netty/example/http/cors/HttpServerInitializer.java +++ b/example/src/main/java/io/netty/example/http/cors/HttpServerInitializer.java @@ -74,7 +74,7 @@ public class HttpServerInitializer extends ChannelInitializer { public void initChannel(SocketChannel ch) throws Exception { ChannelPipeline pipeline = ch.pipeline(); - CorsConfig corsConfig = CorsConfig.anyOrigin().build(); + CorsConfig corsConfig = CorsConfig.withAnyOrigin().build(); pipeline.addLast("encoder", new HttpResponseEncoder()); pipeline.addLast("decoder", new HttpRequestDecoder()); pipeline.addLast("aggregator", new HttpObjectAggregator(65536));