Repository: netty/netty
Issue: CORS should support a whilelist of origins

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.


Issue comments:

Pull request review comments:
Maybe we should not throw an exception but delegate to not break existing apps?

Good point, let me fix that. Thanks

söndagen den 30:e mars 2014 skrev Norman Maurer notifications@github.com:

> In
> codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsConfig.java:
> 
> > @@ -228,15 +250,28 @@ public static Builder anyOrigin() {
> >       \* @return {@link Builder} to support method chaining.
> >       */
> >      public static Builder withOrigin(final String origin) {
> > -        if (origin.equals("*")) {
> 
> Maybe we should not throw an exception but delegate to not break existing
> apps?
> 
> ## 
> 
> Reply to this email directly or view it on GitHubhttps://github.com/netty/netty/pull/2347/files#r11097182
> .


Patch that resolved the issue:
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 5cd70046262..b7b2b0dcc0f 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.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 @@
  */
 public final class CorsConfig {
 
-    private final String origin;
+    private final Set<String> origins;
+    private final boolean anyOrigin;
     private final boolean enabled;
     private final Set<String> exposeHeaders;
     private final boolean allowCredentials;
@@ -47,7 +49,8 @@ public final class CorsConfig {
     private final Map<CharSequence, Callable<?>> preflightHeaders;
 
     private CorsConfig(final Builder builder) {
-        origin = builder.origin;
+        origins = new LinkedHashSet<String>(builder.origins);
+        anyOrigin = builder.anyOrigin;
         enabled = builder.enabled;
         exposeHeaders = builder.exposeHeaders;
         allowCredentials = builder.allowCredentials;
@@ -67,13 +70,31 @@ public boolean isCorsSupportEnabled() {
         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<String> origins() {
+        return origins;
     }
 
     /**
@@ -204,7 +225,8 @@ private static <T> T getValue(final Callable<T> callable) {
     @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 String toString() {
      *
      * @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 static Builder anyOrigin() {
      * @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<String> origins;
+        private final boolean anyOrigin;
         private boolean allowNullOrigin;
         private boolean enabled = true;
         private boolean allowCredentials;
@@ -250,10 +285,21 @@ public static class Builder {
         /**
          * 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... origins) {
+            this.origins = new LinkedHashSet<String>(Arrays.asList(origins));
+            anyOrigin = false;
+        }
+
+        /**
+         * Creates a new Builder instance allowing any origin, "*" which is the
+         * wildcard origin.
+         *
          */
-        public Builder(final String origin) {
-            this.origin = 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 d684b3d043c..727637a37af 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 @@ private boolean setOrigin(final HttpResponse response) {
         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/example/src/main/java/io/netty/example/http/cors/HttpServerInitializer.java b/example/src/main/java/io/netty/example/http/cors/HttpServerInitializer.java
index 2297299b1d8..4e53a30917f 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<SocketChannel> {
     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));


Test patch:
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 e0dfed89622..3f091825abb 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 bda47734418..9e35c11e5bc 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 void simpleRequestWithOrigin() {
         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 void preflightRequestDoNotAllowCredentials() {
 
     @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 void simpleRequestCustomHeaders() {
 
     @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"));
     }