From 412f719aa82a830966d6aa7ee0164ccd18b9280c Mon Sep 17 00:00:00 2001 From: Trustin Lee Date: Thu, 17 Dec 2015 15:05:44 +0900 Subject: [PATCH] Extract the builder of CorsConfig to top level Motivation: Consistency in API design Modifications: - Deprecate CorsConfig.Builder and its factory methods - Deprecate CorsConfig.DateValueGenerator - Add CorsConfigBuilder and its factory methods - Fix typo (curcuit -> circuit) Result: Consistency with other builder APIs such as SslContextBuilder and Http2ConnectionHandlerBuilder --- .../handler/codec/http/cors/CorsConfig.java | 274 ++++---------- .../codec/http/cors/CorsConfigBuilder.java | 352 ++++++++++++++++++ .../handler/codec/http/cors/CorsHandler.java | 17 +- .../codec/http/cors/CorsConfigTest.java | 42 +-- .../codec/http/cors/CorsHandlerTest.java | 57 +-- .../http/cors/HttpCorsServerInitializer.java | 3 +- 6 files changed, 480 insertions(+), 265 deletions(-) create mode 100644 codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsConfigBuilder.java 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 c2efc591fa..4932f6ef7e 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 @@ -17,16 +17,12 @@ package io.netty.handler.codec.http.cors; import io.netty.handler.codec.http.DefaultHttpHeaders; import io.netty.handler.codec.http.EmptyHttpHeaders; -import io.netty.handler.codec.http.HttpHeaderNames; import io.netty.handler.codec.http.HttpHeaders; import io.netty.handler.codec.http.HttpMethod; import io.netty.util.internal.StringUtil; -import java.util.Arrays; 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; @@ -48,9 +44,9 @@ public final class CorsConfig { private final Set allowedRequestHeaders; private final boolean allowNullOrigin; private final Map> preflightHeaders; - private final boolean shortCurcuit; + private final boolean shortCircuit; - private CorsConfig(final Builder builder) { + CorsConfig(final CorsConfigBuilder builder) { origins = new LinkedHashSet(builder.origins); anyOrigin = builder.anyOrigin; enabled = builder.enabled; @@ -61,7 +57,7 @@ public final class CorsConfig { allowedRequestHeaders = builder.requestHeaders; allowNullOrigin = builder.allowNullOrigin; preflightHeaders = builder.preflightHeaders; - shortCurcuit = builder.shortCurcuit; + shortCircuit = builder.shortCircuit; } /** @@ -227,8 +223,16 @@ public final class CorsConfig { * * @return {@code true} if a CORS request should short-curcuit upon receiving an invalid Origin header. */ + public boolean isShortCircuit() { + return shortCircuit; + } + + /** + * @deprecated Use {@link #isShortCircuit()} instead. + */ + @Deprecated public boolean isShortCurcuit() { - return shortCurcuit; + return isShortCircuit(); } private static T getValue(final Callable callable) { @@ -253,19 +257,17 @@ public final class CorsConfig { } /** - * Creates a Builder instance with it's origin set to '*'. - * - * @return Builder to support method chaining. + * @deprecated Use {@link CorsConfigBuilder#forAnyOrigin()} instead. */ + @Deprecated public static Builder withAnyOrigin() { return new Builder(); } /** - * Creates a {@link Builder} instance with the specified origin. - * - * @return {@link Builder} to support method chaining. + * @deprecated Use {@link CorsConfigBuilder#forOrigin(String)} instead. */ + @Deprecated public static Builder withOrigin(final String origin) { if ("*".equals(origin)) { return new Builder(); @@ -274,297 +276,158 @@ public final class CorsConfig { } /** - * Creates a {@link Builder} instance with the specified origins. - * - * @return {@link Builder} to support method chaining. + * @deprecated Use {@link CorsConfigBuilder#forOrigins(String...)} instead. */ + @Deprecated public static Builder withOrigins(final String... origins) { return new Builder(origins); } /** - * Builder used to configure and build a CorsConfig instance. + * @deprecated Use {@link CorsConfigBuilder} instead. */ + @Deprecated public static class Builder { - private final Set origins; - private final boolean anyOrigin; - private boolean allowNullOrigin; - private boolean enabled = true; - private boolean allowCredentials; - private final Set exposeHeaders = new HashSet(); - private long maxAge; - private final Set requestMethods = new HashSet(); - private final Set requestHeaders = new HashSet(); - private final Map> preflightHeaders = new HashMap>(); - private boolean noPreflightHeaders; - private boolean shortCurcuit; + private final CorsConfigBuilder builder; /** - * Creates a new Builder instance with the origin passed in. - * - * @param origins the origin to be used for this builder. + * @deprecated Use {@link CorsConfigBuilder} instead. */ + @Deprecated public Builder(final String... origins) { - this.origins = new LinkedHashSet(Arrays.asList(origins)); - anyOrigin = false; + builder = new CorsConfigBuilder(origins); } /** - * Creates a new Builder instance allowing any origin, "*" which is the - * wildcard origin. - * + * @deprecated Use {@link CorsConfigBuilder} instead. */ + @Deprecated public Builder() { - anyOrigin = true; - origins = Collections.emptySet(); + builder = new CorsConfigBuilder(); } /** - * Web browsers may set the 'Origin' request header to 'null' if a resource is loaded - * from the local file system. Calling this method will enable a successful CORS response - * with a wildcard for the the CORS response header 'Access-Control-Allow-Origin'. - * - * @return {@link Builder} to support method chaining. + * @deprecated Use {@link CorsConfigBuilder#allowNullOrigin()} instead. */ + @Deprecated public Builder allowNullOrigin() { - allowNullOrigin = true; + builder.allowNullOrigin(); return this; } /** - * Disables CORS support. - * - * @return {@link Builder} to support method chaining. + * @deprecated Use {@link CorsConfigBuilder#disable()} instead. */ + @Deprecated public Builder disable() { - enabled = false; + builder.disable(); return this; } /** - * Specifies the headers to be exposed to calling clients. - * - * During a simple CORS request, only certain response headers are made available by the - * browser, for example using: - * 
-         * xhr.getResponseHeader("Content-Type");
-         *
- * - * The headers that are available by default are: - *
    - *
  • Cache-Control
    • - *
  • Content-Language
    • - *
  • Content-Type
    • - *
  • Expires
    • - *
  • Last-Modified
    • - *
  • Pragma
    • - *
- * - * To expose other headers they need to be specified which is what this method enables by - * adding the headers to the CORS 'Access-Control-Expose-Headers' response header. - * - * @param headers the values to be added to the 'Access-Control-Expose-Headers' response header - * @return {@link Builder} to support method chaining. + * @deprecated Use {@link CorsConfigBuilder#exposeHeaders(String...)} instead. */ + @Deprecated public Builder exposeHeaders(final String... headers) { - exposeHeaders.addAll(Arrays.asList(headers)); + builder.exposeHeaders(headers); return this; } /** - * By default cookies are not included in CORS requests, but this method will enable cookies to - * be added to CORS requests. Calling this method will set the CORS 'Access-Control-Allow-Credentials' - * response header to true. - * - * Please note, that cookie support needs to be enabled on the client side as well. - * The client needs to opt-in to send cookies by calling: - * 
-         * xhr.withCredentials = true;
-         *
- * The default value for 'withCredentials' is false in which case no cookies are sent. - * Settning this to true will included cookies in cross origin requests. - * - * @return {@link Builder} to support method chaining. + * @deprecated Use {@link CorsConfigBuilder#allowCredentials()} instead. */ + @Deprecated public Builder allowCredentials() { - allowCredentials = true; + builder.allowCredentials(); return this; } /** - * When making a preflight request the client has to perform two request with can be inefficient. - * This setting will set the CORS 'Access-Control-Max-Age' response header and enables the - * caching of the preflight response for the specified time. During this time no preflight - * request will be made. - * - * @param max the maximum time, in seconds, that the preflight response may be cached. - * @return {@link Builder} to support method chaining. + * @deprecated Use {@link CorsConfigBuilder#maxAge(long)} instead. */ + @Deprecated public Builder maxAge(final long max) { - maxAge = max; + builder.maxAge(max); return this; } /** - * Specifies the allowed set of HTTP Request Methods that should be returned in the - * CORS 'Access-Control-Request-Method' response header. - * - * @param methods the {@link HttpMethod}s that should be allowed. - * @return {@link Builder} to support method chaining. + * @deprecated Use {@link CorsConfigBuilder#allowedRequestMethods(HttpMethod...)} instead. */ + @Deprecated public Builder allowedRequestMethods(final HttpMethod... methods) { - requestMethods.addAll(Arrays.asList(methods)); + builder.allowedRequestMethods(methods); return this; } /** - * Specifies the if headers that should be returned in the CORS 'Access-Control-Allow-Headers' - * response header. - * - * If a client specifies headers on the request, for example by calling: - * 
-         * xhr.setRequestHeader('My-Custom-Header', "SomeValue");
-         *
- * the server will recieve the above header name in the 'Access-Control-Request-Headers' of the - * preflight request. The server will then decide if it allows this header to be sent for the - * real request (remember that a preflight is not the real request but a request asking the server - * if it allow a request). - * - * @param headers the headers to be added to the preflight 'Access-Control-Allow-Headers' response header. - * @return {@link Builder} to support method chaining. + * @deprecated Use {@link CorsConfigBuilder#allowedRequestHeaders(String...)} instead. */ + @Deprecated public Builder allowedRequestHeaders(final String... headers) { - requestHeaders.addAll(Arrays.asList(headers)); + builder.allowedRequestHeaders(headers); return this; } /** - * Returns HTTP response headers that should be added to a CORS preflight response. - * - * An intermediary like a load balancer might require that a CORS preflight request - * have certain headers set. This enables such headers to be added. - * - * @param name the name of the HTTP header. - * @param values the values for the HTTP header. - * @return {@link Builder} to support method chaining. + * @deprecated Use {@link CorsConfigBuilder#preflightResponseHeader(CharSequence, Object...)} instead. */ + @Deprecated public Builder preflightResponseHeader(final CharSequence name, final Object... values) { - if (values.length == 1) { - preflightHeaders.put(name, new ConstantValueGenerator(values[0])); - } else { - preflightResponseHeader(name, Arrays.asList(values)); - } + builder.preflightResponseHeader(name, values); return this; } /** - * Returns HTTP response headers that should be added to a CORS preflight response. - * - * An intermediary like a load balancer might require that a CORS preflight request - * have certain headers set. This enables such headers to be added. - * - * @param name the name of the HTTP header. - * @param value the values for the HTTP header. - * @param the type of values that the Iterable contains. - * @return {@link Builder} to support method chaining. + * @deprecated Use {@link CorsConfigBuilder#preflightResponseHeader(CharSequence, Iterable)} instead. */ + @Deprecated public Builder preflightResponseHeader(final CharSequence name, final Iterable value) { - preflightHeaders.put(name, new ConstantValueGenerator(value)); + builder.preflightResponseHeader(name, value); return this; } /** - * Returns HTTP response headers that should be added to a CORS preflight response. - * - * An intermediary like a load balancer might require that a CORS preflight request - * have certain headers set. This enables such headers to be added. - * - * Some values must be dynamically created when the HTTP response is created, for - * example the 'Date' response header. This can be occomplished by using a Callable - * which will have its 'call' method invoked when the HTTP response is created. - * - * @param name the name of the HTTP header. - * @param valueGenerator a Callable which will be invoked at HTTP response creation. - * @param the type of the value that the Callable can return. - * @return {@link Builder} to support method chaining. + * @deprecated Use {@link CorsConfigBuilder#preflightResponseHeader(CharSequence, Callable)} instead. */ + @Deprecated public Builder preflightResponseHeader(final String name, final Callable valueGenerator) { - preflightHeaders.put(name, valueGenerator); + builder.preflightResponseHeader(name, valueGenerator); return this; } /** - * Specifies that no preflight response headers should be added to a preflight response. - * - * @return {@link Builder} to support method chaining. + * @deprecated Use {@link CorsConfigBuilder#noPreflightResponseHeaders()} instead. */ + @Deprecated public Builder noPreflightResponseHeaders() { - noPreflightHeaders = true; + builder.noPreflightResponseHeaders(); return this; } /** - * Builds a {@link CorsConfig} with settings specified by previous method calls. - * - * @return {@link CorsConfig} the configured CorsConfig instance. + * @deprecated Use {@link CorsConfigBuilder#build()} instead. */ + @Deprecated public CorsConfig build() { - if (preflightHeaders.isEmpty() && !noPreflightHeaders) { - preflightHeaders.put(HttpHeaderNames.DATE, new DateValueGenerator()); - preflightHeaders.put(HttpHeaderNames.CONTENT_LENGTH, new ConstantValueGenerator("0")); - } - return new CorsConfig(this); + return builder.build(); } /** - * Specifies that a CORS request should be rejected if it's invalid before being - * further processing. - * - * CORS headers are set after a request is processed. This may not always be desired - * and this setting will check that the Origin is valid and if it is not valid no - * further processing will take place, and a error will be returned to the calling client. - * - * @return {@link Builder} to support method chaining. + * @deprecated Use {@link CorsConfigBuilder#shortCircuit()} instead. */ + @Deprecated public Builder shortCurcuit() { - shortCurcuit = true; + builder.shortCircuit(); return this; } } /** - * This class is used for preflight HTTP response values that do not need to be - * generated, but instead the value is "static" in that the same value will be returned - * for each call. - */ - private static final class ConstantValueGenerator implements Callable { - - private final Object value; - - /** - * Sole constructor. - * - * @param value the value that will be returned when the call method is invoked. - */ - private ConstantValueGenerator(final Object value) { - if (value == null) { - throw new IllegalArgumentException("value must not be null"); - } - this.value = value; - } - - @Override - public Object call() { - return value; - } - } - - /** - * This callable is used for the DATE preflight HTTP response HTTP header. - * It's value must be generated when the response is generated, hence will be - * different for every call. + * @deprecated Removed without alternatives. */ + @Deprecated public static final class DateValueGenerator implements Callable { @Override @@ -572,5 +435,4 @@ public final class CorsConfig { return new Date(); } } - } diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsConfigBuilder.java b/codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsConfigBuilder.java new file mode 100644 index 0000000000..81e0a8a213 --- /dev/null +++ b/codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsConfigBuilder.java @@ -0,0 +1,352 @@ +/* + * Copyright 2015 The Netty Project + * + * The Netty Project licenses this file to you 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 io.netty.handler.codec.http.cors; + +import io.netty.handler.codec.http.HttpHeaderNames; +import io.netty.handler.codec.http.HttpMethod; + +import java.util.Arrays; +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.Set; +import java.util.concurrent.Callable; + +/** + * Builder used to configure and build a {@link CorsConfig} instance. + */ +public final class CorsConfigBuilder { + + /** + * Creates a Builder instance with it's origin set to '*'. + * + * @return Builder to support method chaining. + */ + public static CorsConfigBuilder forAnyOrigin() { + return new CorsConfigBuilder(); + } + + /** + * Creates a {@link CorsConfigBuilder} instance with the specified origin. + * + * @return {@link CorsConfigBuilder} to support method chaining. + */ + public static CorsConfigBuilder forOrigin(final String origin) { + if ("*".equals(origin)) { + return new CorsConfigBuilder(); + } + return new CorsConfigBuilder(origin); + } + + /** + * Creates a {@link CorsConfigBuilder} instance with the specified origins. + * + * @return {@link CorsConfigBuilder} to support method chaining. + */ + public static CorsConfigBuilder forOrigins(final String... origins) { + return new CorsConfigBuilder(origins); + } + + final Set origins; + final boolean anyOrigin; + boolean allowNullOrigin; + boolean enabled = true; + boolean allowCredentials; + final Set exposeHeaders = new HashSet(); + long maxAge; + final Set requestMethods = new HashSet(); + final Set requestHeaders = new HashSet(); + final Map> preflightHeaders = new HashMap>(); + private boolean noPreflightHeaders; + boolean shortCircuit; + + /** + * Creates a new Builder instance with the origin passed in. + * + * @param origins the origin to be used for this builder. + */ + CorsConfigBuilder(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. + * + */ + CorsConfigBuilder() { + anyOrigin = true; + origins = Collections.emptySet(); + } + + /** + * Web browsers may set the 'Origin' request header to 'null' if a resource is loaded + * from the local file system. Calling this method will enable a successful CORS response + * with a wildcard for the the CORS response header 'Access-Control-Allow-Origin'. + * + * @return {@link CorsConfigBuilder} to support method chaining. + */ + CorsConfigBuilder allowNullOrigin() { + allowNullOrigin = true; + return this; + } + + /** + * Disables CORS support. + * + * @return {@link CorsConfigBuilder} to support method chaining. + */ + public CorsConfigBuilder disable() { + enabled = false; + return this; + } + + /** + * Specifies the headers to be exposed to calling clients. + * + * During a simple CORS request, only certain response headers are made available by the + * browser, for example using: + * 
+     * xhr.getResponseHeader("Content-Type");
+     *
+ * + * The headers that are available by default are: + *
    + *
  • Cache-Control
    • + *
  • Content-Language
    • + *
  • Content-Type
    • + *
  • Expires
    • + *
  • Last-Modified
    • + *
  • Pragma
    • + *
+ * + * To expose other headers they need to be specified which is what this method enables by + * adding the headers to the CORS 'Access-Control-Expose-Headers' response header. + * + * @param headers the values to be added to the 'Access-Control-Expose-Headers' response header + * @return {@link CorsConfigBuilder} to support method chaining. + */ + public CorsConfigBuilder exposeHeaders(final String... headers) { + exposeHeaders.addAll(Arrays.asList(headers)); + return this; + } + + /** + * By default cookies are not included in CORS requests, but this method will enable cookies to + * be added to CORS requests. Calling this method will set the CORS 'Access-Control-Allow-Credentials' + * response header to true. + * + * Please note, that cookie support needs to be enabled on the client side as well. + * The client needs to opt-in to send cookies by calling: + * 
+     * xhr.withCredentials = true;
+     *
+ * The default value for 'withCredentials' is false in which case no cookies are sent. + * Setting this to true will included cookies in cross origin requests. + * + * @return {@link CorsConfigBuilder} to support method chaining. + */ + public CorsConfigBuilder allowCredentials() { + allowCredentials = true; + return this; + } + + /** + * When making a preflight request the client has to perform two request with can be inefficient. + * This setting will set the CORS 'Access-Control-Max-Age' response header and enables the + * caching of the preflight response for the specified time. During this time no preflight + * request will be made. + * + * @param max the maximum time, in seconds, that the preflight response may be cached. + * @return {@link CorsConfigBuilder} to support method chaining. + */ + public CorsConfigBuilder maxAge(final long max) { + maxAge = max; + return this; + } + + /** + * Specifies the allowed set of HTTP Request Methods that should be returned in the + * CORS 'Access-Control-Request-Method' response header. + * + * @param methods the {@link HttpMethod}s that should be allowed. + * @return {@link CorsConfigBuilder} to support method chaining. + */ + public CorsConfigBuilder allowedRequestMethods(final HttpMethod... methods) { + requestMethods.addAll(Arrays.asList(methods)); + return this; + } + + /** + * Specifies the if headers that should be returned in the CORS 'Access-Control-Allow-Headers' + * response header. + * + * If a client specifies headers on the request, for example by calling: + * 
+     * xhr.setRequestHeader('My-Custom-Header', "SomeValue");
+     *
+ * the server will receive the above header name in the 'Access-Control-Request-Headers' of the + * preflight request. The server will then decide if it allows this header to be sent for the + * real request (remember that a preflight is not the real request but a request asking the server + * if it allow a request). + * + * @param headers the headers to be added to the preflight 'Access-Control-Allow-Headers' response header. + * @return {@link CorsConfigBuilder} to support method chaining. + */ + public CorsConfigBuilder allowedRequestHeaders(final String... headers) { + requestHeaders.addAll(Arrays.asList(headers)); + return this; + } + + /** + * Returns HTTP response headers that should be added to a CORS preflight response. + * + * An intermediary like a load balancer might require that a CORS preflight request + * have certain headers set. This enables such headers to be added. + * + * @param name the name of the HTTP header. + * @param values the values for the HTTP header. + * @return {@link CorsConfigBuilder} to support method chaining. + */ + public CorsConfigBuilder preflightResponseHeader(final CharSequence name, final Object... values) { + if (values.length == 1) { + preflightHeaders.put(name, new ConstantValueGenerator(values[0])); + } else { + preflightResponseHeader(name, Arrays.asList(values)); + } + return this; + } + + /** + * Returns HTTP response headers that should be added to a CORS preflight response. + * + * An intermediary like a load balancer might require that a CORS preflight request + * have certain headers set. This enables such headers to be added. + * + * @param name the name of the HTTP header. + * @param value the values for the HTTP header. + * @param the type of values that the Iterable contains. + * @return {@link CorsConfigBuilder} to support method chaining. + */ + public CorsConfigBuilder preflightResponseHeader(final CharSequence name, final Iterable value) { + preflightHeaders.put(name, new ConstantValueGenerator(value)); + return this; + } + + /** + * Returns HTTP response headers that should be added to a CORS preflight response. + * + * An intermediary like a load balancer might require that a CORS preflight request + * have certain headers set. This enables such headers to be added. + * + * Some values must be dynamically created when the HTTP response is created, for + * example the 'Date' response header. This can be accomplished by using a Callable + * which will have its 'call' method invoked when the HTTP response is created. + * + * @param name the name of the HTTP header. + * @param valueGenerator a Callable which will be invoked at HTTP response creation. + * @param the type of the value that the Callable can return. + * @return {@link CorsConfigBuilder} to support method chaining. + */ + public CorsConfigBuilder preflightResponseHeader(final CharSequence name, final Callable valueGenerator) { + preflightHeaders.put(name, valueGenerator); + return this; + } + + /** + * Specifies that no preflight response headers should be added to a preflight response. + * + * @return {@link CorsConfigBuilder} to support method chaining. + */ + public CorsConfigBuilder noPreflightResponseHeaders() { + noPreflightHeaders = true; + return this; + } + + /** + * Specifies that a CORS request should be rejected if it's invalid before being + * further processing. + * + * CORS headers are set after a request is processed. This may not always be desired + * and this setting will check that the Origin is valid and if it is not valid no + * further processing will take place, and a error will be returned to the calling client. + * + * @return {@link CorsConfigBuilder} to support method chaining. + */ + public CorsConfigBuilder shortCircuit() { + shortCircuit = true; + return this; + } + + /** + * Builds a {@link CorsConfig} with settings specified by previous method calls. + * + * @return {@link CorsConfig} the configured CorsConfig instance. + */ + public CorsConfig build() { + if (preflightHeaders.isEmpty() && !noPreflightHeaders) { + preflightHeaders.put(HttpHeaderNames.DATE, DateValueGenerator.INSTANCE); + preflightHeaders.put(HttpHeaderNames.CONTENT_LENGTH, new ConstantValueGenerator("0")); + } + return new CorsConfig(this); + } + + /** + * This class is used for preflight HTTP response values that do not need to be + * generated, but instead the value is "static" in that the same value will be returned + * for each call. + */ + private static final class ConstantValueGenerator implements Callable { + + private final Object value; + + /** + * Sole constructor. + * + * @param value the value that will be returned when the call method is invoked. + */ + private ConstantValueGenerator(final Object value) { + if (value == null) { + throw new IllegalArgumentException("value must not be null"); + } + this.value = value; + } + + @Override + public Object call() { + return value; + } + } + + /** + * This callable is used for the DATE preflight HTTP response HTTP header. + * It's value must be generated when the response is generated, hence will be + * different for every call. + */ + private static final class DateValueGenerator implements Callable { + + static final DateValueGenerator INSTANCE = new DateValueGenerator(); + + @Override + public Date call() throws Exception { + return new Date(); + } + } +} 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 5084465820..048ecabe45 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 @@ -30,6 +30,7 @@ import io.netty.util.internal.logging.InternalLoggerFactory; import static io.netty.handler.codec.http.HttpMethod.*; import static io.netty.handler.codec.http.HttpResponseStatus.*; import static io.netty.util.ReferenceCountUtil.*; +import static io.netty.util.internal.ObjectUtil.checkNotNull; /** * Handles Cross Origin Resource Sharing (CORS) requests. @@ -45,8 +46,11 @@ public class CorsHandler extends ChannelDuplexHandler { private HttpRequest request; + /** + * Creates a new instance with the specified {@link CorsConfig}. + */ public CorsHandler(final CorsConfig config) { - this.config = config; + this.config = checkNotNull(config, "config"); } @Override @@ -57,7 +61,7 @@ public class CorsHandler extends ChannelDuplexHandler { handlePreflight(ctx, request); return; } - if (config.isShortCurcuit() && !validateOrigin()) { + if (config.isShortCircuit() && !validateOrigin()) { forbidden(ctx, request); return; } @@ -109,7 +113,7 @@ public class CorsHandler extends ChannelDuplexHandler { setVaryHeader(response); return true; } - logger.debug("Request origin [" + origin + "] was not among the configured origins " + config.origins()); + logger.debug("Request origin [{}]] was not among the configured origins [{}]", origin, config.origins()); } return false; } @@ -194,16 +198,9 @@ public class CorsHandler extends ChannelDuplexHandler { ctx.writeAndFlush(msg, promise); } - @Override - public void exceptionCaught(final ChannelHandlerContext ctx, final Throwable cause) throws Exception { - logger.error("Caught error in CorsHandler", cause); - ctx.fireExceptionCaught(cause); - } - private static void forbidden(final ChannelHandlerContext ctx, final HttpRequest request) { ctx.writeAndFlush(new DefaultFullHttpResponse(request.protocolVersion(), FORBIDDEN)) .addListener(ChannelFutureListener.CLOSE); release(request); } } - 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 b418655fa3..91f6c8cdff 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 @@ -22,9 +22,9 @@ import io.netty.handler.codec.http.HttpMethod; import org.junit.Test; import static io.netty.handler.codec.http.HttpHeadersTestUtils.of; -import static io.netty.handler.codec.http.cors.CorsConfig.withAnyOrigin; -import static io.netty.handler.codec.http.cors.CorsConfig.withOrigin; -import static io.netty.handler.codec.http.cors.CorsConfig.withOrigins; +import static io.netty.handler.codec.http.cors.CorsConfigBuilder.forAnyOrigin; +import static io.netty.handler.codec.http.cors.CorsConfigBuilder.forOrigin; +import static io.netty.handler.codec.http.cors.CorsConfigBuilder.forOrigins; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.hasItems; import static org.hamcrest.CoreMatchers.is; @@ -35,13 +35,13 @@ public class CorsConfigTest { @Test public void disabled() { - final CorsConfig cors = withAnyOrigin().disable().build(); + final CorsConfig cors = forAnyOrigin().disable().build(); assertThat(cors.isCorsSupportEnabled(), is(false)); } @Test public void anyOrigin() { - final CorsConfig cors = withAnyOrigin().build(); + final CorsConfig cors = forAnyOrigin().build(); assertThat(cors.isAnyOriginSupported(), is(true)); assertThat(cors.origin(), is("*")); assertThat(cors.origins().isEmpty(), is(true)); @@ -49,7 +49,7 @@ public class CorsConfigTest { @Test public void wildcardOrigin() { - final CorsConfig cors = withOrigin("*").build(); + final CorsConfig cors = forOrigin("*").build(); assertThat(cors.isAnyOriginSupported(), is(true)); assertThat(cors.origin(), equalTo("*")); assertThat(cors.origins().isEmpty(), is(true)); @@ -57,7 +57,7 @@ public class CorsConfigTest { @Test public void origin() { - final CorsConfig cors = withOrigin("http://localhost:7888").build(); + final CorsConfig cors = forOrigin("http://localhost:7888").build(); assertThat(cors.origin(), is(equalTo("http://localhost:7888"))); assertThat(cors.isAnyOriginSupported(), is(false)); } @@ -65,75 +65,75 @@ public class CorsConfigTest { @Test public void origins() { final String[] origins = {"http://localhost:7888", "https://localhost:7888"}; - final CorsConfig cors = withOrigins(origins).build(); + final CorsConfig cors = forOrigins(origins).build(); assertThat(cors.origins(), hasItems(origins)); assertThat(cors.isAnyOriginSupported(), is(false)); } @Test public void exposeHeaders() { - final CorsConfig cors = withAnyOrigin().exposeHeaders("custom-header1", "custom-header2").build(); + final CorsConfig cors = forAnyOrigin().exposeHeaders("custom-header1", "custom-header2").build(); assertThat(cors.exposedHeaders(), hasItems("custom-header1", "custom-header2")); } @Test public void allowCredentials() { - final CorsConfig cors = withAnyOrigin().allowCredentials().build(); + final CorsConfig cors = forAnyOrigin().allowCredentials().build(); assertThat(cors.isCredentialsAllowed(), is(true)); } @Test public void maxAge() { - final CorsConfig cors = withAnyOrigin().maxAge(3000).build(); + final CorsConfig cors = forAnyOrigin().maxAge(3000).build(); assertThat(cors.maxAge(), is(3000L)); } @Test public void requestMethods() { - final CorsConfig cors = withAnyOrigin().allowedRequestMethods(HttpMethod.POST, HttpMethod.GET).build(); + final CorsConfig cors = forAnyOrigin().allowedRequestMethods(HttpMethod.POST, HttpMethod.GET).build(); assertThat(cors.allowedRequestMethods(), hasItems(HttpMethod.POST, HttpMethod.GET)); } @Test public void requestHeaders() { - final CorsConfig cors = withAnyOrigin().allowedRequestHeaders("preflight-header1", "preflight-header2").build(); + final CorsConfig cors = forAnyOrigin().allowedRequestHeaders("preflight-header1", "preflight-header2").build(); assertThat(cors.allowedRequestHeaders(), hasItems("preflight-header1", "preflight-header2")); } @Test public void preflightResponseHeadersSingleValue() { - final CorsConfig cors = withAnyOrigin().preflightResponseHeader("SingleValue", "value").build(); + final CorsConfig cors = forAnyOrigin().preflightResponseHeader("SingleValue", "value").build(); assertThat(cors.preflightResponseHeaders().get(of("SingleValue")), equalTo("value")); } @Test public void preflightResponseHeadersMultipleValues() { - final CorsConfig cors = withAnyOrigin().preflightResponseHeader("MultipleValues", "value1", "value2").build(); + final CorsConfig cors = forAnyOrigin().preflightResponseHeader("MultipleValues", "value1", "value2").build(); assertThat(cors.preflightResponseHeaders().getAll(of("MultipleValues")), hasItems("value1", "value2")); } @Test public void defaultPreflightResponseHeaders() { - final CorsConfig cors = withAnyOrigin().build(); + final CorsConfig cors = forAnyOrigin().build(); assertThat(cors.preflightResponseHeaders().get(HttpHeaderNames.DATE), is(notNullValue())); assertThat(cors.preflightResponseHeaders().get(HttpHeaderNames.CONTENT_LENGTH), is("0")); } @Test public void emptyPreflightResponseHeaders() { - final CorsConfig cors = withAnyOrigin().noPreflightResponseHeaders().build(); + final CorsConfig cors = forAnyOrigin().noPreflightResponseHeaders().build(); assertThat(cors.preflightResponseHeaders(), equalTo((HttpHeaders) EmptyHttpHeaders.INSTANCE)); } @Test (expected = IllegalArgumentException.class) public void shouldThrowIfValueIsNull() { - withOrigin("*").preflightResponseHeader("HeaderName", new Object[]{null}).build(); + forOrigin("*").preflightResponseHeader("HeaderName", new Object[]{null}).build(); } @Test - public void shortCurcuit() { - final CorsConfig cors = withOrigin("http://localhost:8080").shortCurcuit().build(); - assertThat(cors.isShortCurcuit(), is(true)); + public void shortCircuit() { + final CorsConfig cors = forOrigin("http://localhost:8080").shortCircuit().build(); + assertThat(cors.isShortCircuit(), is(true)); } } 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 55589a847d..6d3eec888d 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 @@ -46,6 +46,9 @@ import static io.netty.handler.codec.http.HttpMethod.OPTIONS; import static io.netty.handler.codec.http.HttpResponseStatus.FORBIDDEN; import static io.netty.handler.codec.http.HttpResponseStatus.OK; import static io.netty.handler.codec.http.HttpVersion.HTTP_1_1; +import static io.netty.handler.codec.http.cors.CorsConfigBuilder.forAnyOrigin; +import static io.netty.handler.codec.http.cors.CorsConfigBuilder.forOrigin; +import static io.netty.handler.codec.http.cors.CorsConfigBuilder.forOrigins; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.is; @@ -57,20 +60,20 @@ public class CorsHandlerTest { @Test public void nonCorsRequest() { - final HttpResponse response = simpleRequest(CorsConfig.withAnyOrigin().build(), null); + final HttpResponse response = simpleRequest(forAnyOrigin().build(), null); assertThat(response.headers().contains(ACCESS_CONTROL_ALLOW_ORIGIN), is(false)); } @Test public void simpleRequestWithAnyOrigin() { - final HttpResponse response = simpleRequest(CorsConfig.withAnyOrigin().build(), "http://localhost:7777"); + final HttpResponse response = simpleRequest(forAnyOrigin().build(), "http://localhost:7777"); assertThat(response.headers().get(ACCESS_CONTROL_ALLOW_ORIGIN), is("*")); } @Test public void simpleRequestWithOrigin() { final String origin = "http://localhost:8888"; - final HttpResponse response = simpleRequest(CorsConfig.withOrigin(origin).build(), origin); + final HttpResponse response = simpleRequest(forOrigin(origin).build(), origin); assertThat(response.headers().get(ACCESS_CONTROL_ALLOW_ORIGIN), is(origin)); } @@ -79,22 +82,23 @@ public class CorsHandlerTest { 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); + final HttpResponse response1 = simpleRequest(forOrigins(origins).build(), origin1); assertThat(response1.headers().get(ACCESS_CONTROL_ALLOW_ORIGIN), is(origin1)); - final HttpResponse response2 = simpleRequest(CorsConfig.withOrigins(origins).build(), origin2); + final HttpResponse response2 = simpleRequest(forOrigins(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); + final HttpResponse response = simpleRequest( + forOrigins("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") + final CorsConfig config = forOrigin("http://localhost:8888") .allowedRequestMethods(GET, DELETE) .build(); final HttpResponse response = preflightRequest(config, "http://localhost:8888", "content-type, xheader1"); @@ -106,7 +110,7 @@ public class CorsHandlerTest { @Test public void preflightGetRequestWithCustomHeaders() { - final CorsConfig config = CorsConfig.withOrigin("http://localhost:8888") + final CorsConfig config = forOrigin("http://localhost:8888") .allowedRequestMethods(OPTIONS, GET, DELETE) .allowedRequestHeaders("content-type", "xheader1") .build(); @@ -121,7 +125,7 @@ public class CorsHandlerTest { @Test public void preflightRequestWithDefaultHeaders() { - final CorsConfig config = CorsConfig.withOrigin("http://localhost:8888").build(); + final CorsConfig config = forOrigin("http://localhost:8888").build(); final HttpResponse response = preflightRequest(config, "http://localhost:8888", "content-type, xheader1"); assertThat(response.headers().get(CONTENT_LENGTH), is("0")); assertThat(response.headers().get(DATE), is(notNullValue())); @@ -130,7 +134,7 @@ public class CorsHandlerTest { @Test public void preflightRequestWithCustomHeader() { - final CorsConfig config = CorsConfig.withOrigin("http://localhost:8888") + final CorsConfig config = forOrigin("http://localhost:8888") .preflightResponseHeader("CustomHeader", "somevalue") .build(); final HttpResponse response = preflightRequest(config, "http://localhost:8888", "content-type, xheader1"); @@ -143,7 +147,7 @@ public class CorsHandlerTest { final String headerName = "CustomHeader"; final String value1 = "value1"; final String value2 = "value2"; - final CorsConfig config = CorsConfig.withOrigin("http://localhost:8888") + final CorsConfig config = forOrigin("http://localhost:8888") .preflightResponseHeader(headerName, value1, value2) .build(); final HttpResponse response = preflightRequest(config, "http://localhost:8888", "content-type, xheader1"); @@ -156,7 +160,7 @@ public class CorsHandlerTest { final String headerName = "CustomHeader"; final String value1 = "value1"; final String value2 = "value2"; - final CorsConfig config = CorsConfig.withOrigin("http://localhost:8888") + final CorsConfig config = forOrigin("http://localhost:8888") .preflightResponseHeader(headerName, Arrays.asList(value1, value2)) .build(); final HttpResponse response = preflightRequest(config, "http://localhost:8888", "content-type, xheader1"); @@ -166,7 +170,7 @@ public class CorsHandlerTest { @Test public void preflightRequestWithValueGenerator() { - final CorsConfig config = CorsConfig.withOrigin("http://localhost:8888") + final CorsConfig config = forOrigin("http://localhost:8888") .preflightResponseHeader("GenHeader", new Callable() { @Override public String call() throws Exception { @@ -181,7 +185,7 @@ public class CorsHandlerTest { @Test public void preflightRequestWithNullOrigin() { final String origin = "null"; - final CorsConfig config = CorsConfig.withOrigin(origin) + final CorsConfig config = forOrigin(origin) .allowNullOrigin() .allowCredentials() .build(); @@ -193,14 +197,14 @@ public class CorsHandlerTest { @Test public void preflightRequestAllowCredentials() { final String origin = "null"; - final CorsConfig config = CorsConfig.withOrigin(origin).allowCredentials().build(); + final CorsConfig config = forOrigin(origin).allowCredentials().build(); final HttpResponse response = preflightRequest(config, origin, "content-type, xheader1"); assertThat(response.headers().get(ACCESS_CONTROL_ALLOW_CREDENTIALS), is(equalTo("true"))); } @Test public void preflightRequestDoNotAllowCredentials() { - final CorsConfig config = CorsConfig.withOrigin("http://localhost:8888").build(); + final CorsConfig config = forOrigin("http://localhost:8888").build(); final HttpResponse response = preflightRequest(config, "http://localhost:8888", ""); // the only valid value for Access-Control-Allow-Credentials is true. assertThat(response.headers().contains(ACCESS_CONTROL_ALLOW_CREDENTIALS), is(false)); @@ -208,7 +212,7 @@ public class CorsHandlerTest { @Test public void simpleRequestCustomHeaders() { - final CorsConfig config = CorsConfig.withAnyOrigin().exposeHeaders("custom1", "custom2").build(); + final CorsConfig config = forAnyOrigin().exposeHeaders("custom1", "custom2").build(); final HttpResponse response = simpleRequest(config, "http://localhost:7777"); assertThat(response.headers().get(ACCESS_CONTROL_ALLOW_ORIGIN), equalTo("*")); assertThat(response.headers().get(ACCESS_CONTROL_EXPOSE_HEADERS), containsString("custom1")); @@ -217,21 +221,21 @@ public class CorsHandlerTest { @Test public void simpleRequestAllowCredentials() { - final CorsConfig config = CorsConfig.withAnyOrigin().allowCredentials().build(); + final CorsConfig config = forAnyOrigin().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.withAnyOrigin().build(); + final CorsConfig config = forAnyOrigin().build(); final HttpResponse response = simpleRequest(config, "http://localhost:7777"); assertThat(response.headers().contains(ACCESS_CONTROL_ALLOW_CREDENTIALS), is(false)); } @Test public void anyOriginAndAllowCredentialsShouldEchoRequestOrigin() { - final CorsConfig config = CorsConfig.withAnyOrigin().allowCredentials().build(); + final CorsConfig config = forAnyOrigin().allowCredentials().build(); final HttpResponse response = simpleRequest(config, "http://localhost:7777"); assertThat(response.headers().get(ACCESS_CONTROL_ALLOW_CREDENTIALS), equalTo("true")); assertThat(response.headers().get(ACCESS_CONTROL_ALLOW_ORIGIN), equalTo("http://localhost:7777")); @@ -240,7 +244,7 @@ public class CorsHandlerTest { @Test public void simpleRequestExposeHeaders() { - final CorsConfig config = CorsConfig.withAnyOrigin().exposeHeaders("one", "two").build(); + final CorsConfig config = forAnyOrigin().exposeHeaders("one", "two").build(); final HttpResponse response = simpleRequest(config, "http://localhost:7777"); assertThat(response.headers().get(ACCESS_CONTROL_EXPOSE_HEADERS), containsString("one")); assertThat(response.headers().get(ACCESS_CONTROL_EXPOSE_HEADERS), containsString("two")); @@ -248,14 +252,14 @@ public class CorsHandlerTest { @Test public void simpleRequestShortCurcuit() { - final CorsConfig config = CorsConfig.withOrigin("http://localhost:8080").shortCurcuit().build(); + final CorsConfig config = forOrigin("http://localhost:8080").shortCircuit().build(); final HttpResponse response = simpleRequest(config, "http://localhost:7777"); assertThat(response.status(), is(FORBIDDEN)); } @Test public void simpleRequestNoShortCurcuit() { - final CorsConfig config = CorsConfig.withOrigin("http://localhost:8080").build(); + final CorsConfig config = forOrigin("http://localhost:8080").build(); final HttpResponse response = simpleRequest(config, "http://localhost:7777"); assertThat(response.status(), is(OK)); assertThat(response.headers().get(ACCESS_CONTROL_ALLOW_ORIGIN), is(nullValue())); @@ -263,7 +267,7 @@ public class CorsHandlerTest { @Test public void shortCurcuitNonCorsRequest() { - final CorsConfig config = CorsConfig.withOrigin("https://localhost").shortCurcuit().build(); + final CorsConfig config = forOrigin("https://localhost").shortCircuit().build(); final HttpResponse response = simpleRequest(config, null); assertThat(response.status(), is(OK)); assertThat(response.headers().get(ACCESS_CONTROL_ALLOW_ORIGIN), is(nullValue())); @@ -271,7 +275,7 @@ public class CorsHandlerTest { @Test public void preflightRequestShouldReleaseRequest() { - final CorsConfig config = CorsConfig.withOrigin("http://localhost:8888") + final CorsConfig config = forOrigin("http://localhost:8888") .preflightResponseHeader("CustomHeader", Arrays.asList("value1", "value2")) .build(); final EmbeddedChannel channel = new EmbeddedChannel(new CorsHandler(config)); @@ -282,7 +286,7 @@ public class CorsHandlerTest { @Test public void forbiddenShouldReleaseRequest() { - final CorsConfig config = CorsConfig.withOrigin("https://localhost").shortCurcuit().build(); + final CorsConfig config = forOrigin("https://localhost").shortCircuit().build(); final EmbeddedChannel channel = new EmbeddedChannel(new CorsHandler(config), new EchoHandler()); final FullHttpRequest request = createHttpRequest(GET); request.headers().set(ORIGIN, "http://localhost:8888"); @@ -349,5 +353,4 @@ public class CorsHandlerTest { assertThat(header, containsString(value)); } } - } diff --git a/example/src/main/java/io/netty/example/http/cors/HttpCorsServerInitializer.java b/example/src/main/java/io/netty/example/http/cors/HttpCorsServerInitializer.java index 3852aecd1d..01c422bac4 100644 --- a/example/src/main/java/io/netty/example/http/cors/HttpCorsServerInitializer.java +++ b/example/src/main/java/io/netty/example/http/cors/HttpCorsServerInitializer.java @@ -22,6 +22,7 @@ import io.netty.handler.codec.http.HttpObjectAggregator; import io.netty.handler.codec.http.HttpRequestDecoder; import io.netty.handler.codec.http.HttpResponseEncoder; import io.netty.handler.codec.http.cors.CorsConfig; +import io.netty.handler.codec.http.cors.CorsConfigBuilder; import io.netty.handler.codec.http.cors.CorsHandler; import io.netty.handler.ssl.SslContext; import io.netty.handler.stream.ChunkedWriteHandler; @@ -79,7 +80,7 @@ public class HttpCorsServerInitializer extends ChannelInitializer @Override public void initChannel(SocketChannel ch) { - CorsConfig corsConfig = CorsConfig.withAnyOrigin().build(); + CorsConfig corsConfig = CorsConfigBuilder.forAnyOrigin().build(); ChannelPipeline pipeline = ch.pipeline(); if (sslCtx != null) { pipeline.addLast(sslCtx.newHandler(ch.alloc()));