From 0557927b655d8ae66933170657e4aad160f8c56e Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Mon, 2 May 2016 19:19:06 +0200 Subject: [PATCH] Updating allowNullOrigin to return 'null' instead of '*'. Motivation: Currently the way a 'null' origin, a request that most often indicated that the request is coming from a file on the local file system, is handled is incorrect. We are currently returning a wildcard origin '*' but should be returning 'null' for the 'Access-Control-Allow-Origin' which is valid according to the specification [1]. Modifications: Updated CorsHandler to add a 'null' origin instead of the '*' origin in the case the request origin is 'null. Result: All test pass and the CORS example as does the cors.html example if you try to serve it by opening the file directly in a web browser. [1] https://www.w3.org/TR/cors/#access-control-allow-origin-response-header --- .../handler/codec/http/cors/CorsConfigBuilder.java | 2 +- .../netty/handler/codec/http/cors/CorsHandler.java | 9 +++++++-- .../handler/codec/http/cors/CorsHandlerTest.java | 13 +++++++++++-- .../http/cors/HttpCorsServerInitializer.java | 2 +- 4 files changed, 20 insertions(+), 6 deletions(-) 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 index 52457b3231..f7fb99b1b8 100644 --- 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 @@ -99,7 +99,7 @@ public final class 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'. + * with a 'null' value for the the CORS response header 'Access-Control-Allow-Origin'. * * @return {@link CorsConfigBuilder} to support method chaining. */ 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 048ecabe45..d4a43fcaa2 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 @@ -42,6 +42,7 @@ public class CorsHandler extends ChannelDuplexHandler { private static final InternalLogger logger = InternalLoggerFactory.getInstance(CorsHandler.class); private static final String ANY_ORIGIN = "*"; + private static final String NULL_ORIGIN = "null"; private final CorsConfig config; private HttpRequest request; @@ -95,8 +96,8 @@ public class CorsHandler extends ChannelDuplexHandler { private boolean setOrigin(final HttpResponse response) { final String origin = request.headers().get(HttpHeaderNames.ORIGIN); if (origin != null) { - if ("null".equals(origin) && config.isNullOriginAllowed()) { - setAnyOrigin(response); + if (NULL_ORIGIN.equals(origin) && config.isNullOriginAllowed()) { + setNullOrigin(response); return true; } if (config.isAnyOriginSupported()) { @@ -148,6 +149,10 @@ public class CorsHandler extends ChannelDuplexHandler { setOrigin(response, ANY_ORIGIN); } + private static void setNullOrigin(final HttpResponse response) { + setOrigin(response, NULL_ORIGIN); + } + private static void setOrigin(final HttpResponse response, final String origin) { response.headers().set(HttpHeaderNames.ACCESS_CONTROL_ALLOW_ORIGIN, origin); } 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 6d3eec888d..df075e7fdb 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 @@ -70,6 +70,15 @@ public class CorsHandlerTest { assertThat(response.headers().get(ACCESS_CONTROL_ALLOW_ORIGIN), is("*")); } + @Test + public void simpleRequestWithNullOrigin() { + final HttpResponse response = simpleRequest(forOrigin("http://test.com").allowNullOrigin() + .allowCredentials() + .build(), "null"); + assertThat(response.headers().get(ACCESS_CONTROL_ALLOW_ORIGIN), is("null")); + assertThat(response.headers().get(ACCESS_CONTROL_ALLOW_CREDENTIALS), is(equalTo("true"))); + } + @Test public void simpleRequestWithOrigin() { final String origin = "http://localhost:8888"; @@ -190,8 +199,8 @@ public class CorsHandlerTest { .allowCredentials() .build(); final HttpResponse response = preflightRequest(config, origin, "content-type, xheader1"); - assertThat(response.headers().get(ACCESS_CONTROL_ALLOW_ORIGIN), is(equalTo("*"))); - assertThat(response.headers().get(ACCESS_CONTROL_ALLOW_CREDENTIALS), is(nullValue())); + assertThat(response.headers().get(ACCESS_CONTROL_ALLOW_ORIGIN), is(equalTo("null"))); + assertThat(response.headers().get(ACCESS_CONTROL_ALLOW_CREDENTIALS), is(equalTo("true"))); } @Test 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 01c422bac4..d7731bbad9 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 @@ -80,7 +80,7 @@ public class HttpCorsServerInitializer extends ChannelInitializer @Override public void initChannel(SocketChannel ch) { - CorsConfig corsConfig = CorsConfigBuilder.forAnyOrigin().build(); + CorsConfig corsConfig = CorsConfigBuilder.forAnyOrigin().allowNullOrigin().allowCredentials().build(); ChannelPipeline pipeline = ch.pipeline(); if (sslCtx != null) { pipeline.addLast(sslCtx.newHandler(ch.alloc()));