From 2ce74d16c402a47fad731d0a683a8e58e584a7e4 Mon Sep 17 00:00:00 2001 From: Ali Beyad Date: Mon, 8 Aug 2016 13:30:15 -0500 Subject: [PATCH 1/2] Fixes netty4 CORS config to use defaults for `http.cors.allow-methods` and `http.cors.allow-headers` when none are specified. --- .../netty4/Netty4HttpServerTransport.java | 5 ++-- .../Netty4HttpServerTransportTests.java | 25 +++++++++++++++---- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpServerTransport.java b/modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpServerTransport.java index dd6c816ce655e..5cc251a9add54 100644 --- a/modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpServerTransport.java +++ b/modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpServerTransport.java @@ -33,7 +33,6 @@ import io.netty.channel.RecvByteBufAllocator; import io.netty.channel.nio.NioEventLoopGroup; import io.netty.channel.oio.OioEventLoopGroup; -import io.netty.channel.socket.SocketChannel; import io.netty.channel.socket.nio.NioServerSocketChannel; import io.netty.channel.socket.oio.OioServerSocketChannel; import io.netty.handler.codec.ByteToMessageDecoder; @@ -403,14 +402,14 @@ private Netty4CorsConfig buildCorsConfig(Settings settings) { if (SETTING_CORS_ALLOW_CREDENTIALS.get(settings)) { builder.allowCredentials(); } - String[] strMethods = settings.getAsArray(SETTING_CORS_ALLOW_METHODS.getKey()); + String[] strMethods = Strings.splitStringByCommaToArray(SETTING_CORS_ALLOW_METHODS.get(settings)); HttpMethod[] methods = Arrays.asList(strMethods) .stream() .map(HttpMethod::valueOf) .toArray(size -> new HttpMethod[size]); return builder.allowedRequestMethods(methods) .maxAge(SETTING_CORS_MAX_AGE.get(settings)) - .allowedRequestHeaders(settings.getAsArray(SETTING_CORS_ALLOW_HEADERS.getKey())) + .allowedRequestHeaders(Strings.splitStringByCommaToArray(SETTING_CORS_ALLOW_HEADERS.get(settings))) .shortCircuit() .build(); } diff --git a/modules/transport-netty4/src/test/java/org/elasticsearch/http/netty4/Netty4HttpServerTransportTests.java b/modules/transport-netty4/src/test/java/org/elasticsearch/http/netty4/Netty4HttpServerTransportTests.java index 96618b685f28e..e6e3a29d9d897 100644 --- a/modules/transport-netty4/src/test/java/org/elasticsearch/http/netty4/Netty4HttpServerTransportTests.java +++ b/modules/transport-netty4/src/test/java/org/elasticsearch/http/netty4/Netty4HttpServerTransportTests.java @@ -86,17 +86,32 @@ public void shutdown() throws Exception { } public void testCorsConfig() { - final Set methods = new HashSet<>(Arrays.asList("get", "options", "post")); - final Set headers = new HashSet<>(Arrays.asList("Content-Type", "Content-Length")); - final Settings settings = Settings.builder() + Set methods = new HashSet<>(Arrays.asList("get", "options", "post")); + Set headers = new HashSet<>(Arrays.asList("Content-Type", "Content-Length")); + Settings settings = Settings.builder() .put(SETTING_CORS_ENABLED.getKey(), true) .put(SETTING_CORS_ALLOW_ORIGIN.getKey(), "*") .put(SETTING_CORS_ALLOW_METHODS.getKey(), Strings.collectionToCommaDelimitedString(methods)) .put(SETTING_CORS_ALLOW_HEADERS.getKey(), Strings.collectionToCommaDelimitedString(headers)) .put(SETTING_CORS_ALLOW_CREDENTIALS.getKey(), true) .build(); - final Netty4HttpServerTransport transport = new Netty4HttpServerTransport(settings, networkService, bigArrays, threadPool); - final Netty4CorsConfig corsConfig = transport.getCorsConfig(); + Netty4HttpServerTransport transport = new Netty4HttpServerTransport(settings, networkService, bigArrays, threadPool); + Netty4CorsConfig corsConfig = transport.getCorsConfig(); + assertThat(corsConfig.isAnyOriginSupported(), equalTo(true)); + assertThat(corsConfig.allowedRequestHeaders(), equalTo(headers)); + assertThat(corsConfig.allowedRequestMethods().stream().map(HttpMethod::name).collect(Collectors.toSet()), equalTo(methods)); + transport.close(); + + // test with default values + methods = Strings.commaDelimitedListToSet(SETTING_CORS_ALLOW_METHODS.getDefault(Settings.EMPTY)); + headers = Strings.commaDelimitedListToSet(SETTING_CORS_ALLOW_HEADERS.getDefault(Settings.EMPTY)); + settings = Settings.builder() + .put(SETTING_CORS_ENABLED.getKey(), true) + .put(SETTING_CORS_ALLOW_ORIGIN.getKey(), "*") + .put(SETTING_CORS_ALLOW_CREDENTIALS.getKey(), true) + .build(); + transport = new Netty4HttpServerTransport(settings, networkService, bigArrays, threadPool); + corsConfig = transport.getCorsConfig(); assertThat(corsConfig.isAnyOriginSupported(), equalTo(true)); assertThat(corsConfig.allowedRequestHeaders(), equalTo(headers)); assertThat(corsConfig.allowedRequestMethods().stream().map(HttpMethod::name).collect(Collectors.toSet()), equalTo(methods)); From 7a9a7a163b8a887e18d7782c39a4557e1d006a80 Mon Sep 17 00:00:00 2001 From: Ali Beyad Date: Wed, 10 Aug 2016 16:37:39 -0400 Subject: [PATCH 2/2] Splits cors config tests into two and simplifies them --- .../netty4/Netty4HttpServerTransport.java | 3 +- .../Netty4HttpServerTransportTests.java | 57 +++++++++---------- 2 files changed, 29 insertions(+), 31 deletions(-) diff --git a/modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpServerTransport.java b/modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpServerTransport.java index 5cc251a9add54..68727db27c1ec 100644 --- a/modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpServerTransport.java +++ b/modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpServerTransport.java @@ -381,7 +381,8 @@ static int resolvePublishPort(Settings settings, List methods = new HashSet<>(Arrays.asList("get", "options", "post")); - Set headers = new HashSet<>(Arrays.asList("Content-Type", "Content-Length")); - Settings settings = Settings.builder() - .put(SETTING_CORS_ENABLED.getKey(), true) - .put(SETTING_CORS_ALLOW_ORIGIN.getKey(), "*") - .put(SETTING_CORS_ALLOW_METHODS.getKey(), Strings.collectionToCommaDelimitedString(methods)) - .put(SETTING_CORS_ALLOW_HEADERS.getKey(), Strings.collectionToCommaDelimitedString(headers)) - .put(SETTING_CORS_ALLOW_CREDENTIALS.getKey(), true) - .build(); - Netty4HttpServerTransport transport = new Netty4HttpServerTransport(settings, networkService, bigArrays, threadPool); - Netty4CorsConfig corsConfig = transport.getCorsConfig(); - assertThat(corsConfig.isAnyOriginSupported(), equalTo(true)); - assertThat(corsConfig.allowedRequestHeaders(), equalTo(headers)); - assertThat(corsConfig.allowedRequestMethods().stream().map(HttpMethod::name).collect(Collectors.toSet()), equalTo(methods)); - transport.close(); + final Set methods = new HashSet<>(Arrays.asList("get", "options", "post")); + final Set headers = new HashSet<>(Arrays.asList("Content-Type", "Content-Length")); + final Settings settings = Settings.builder() + .put(SETTING_CORS_ENABLED.getKey(), true) + .put(SETTING_CORS_ALLOW_ORIGIN.getKey(), "*") + .put(SETTING_CORS_ALLOW_METHODS.getKey(), Strings.collectionToCommaDelimitedString(methods)) + .put(SETTING_CORS_ALLOW_HEADERS.getKey(), Strings.collectionToCommaDelimitedString(headers)) + .put(SETTING_CORS_ALLOW_CREDENTIALS.getKey(), true) + .build(); + final Netty4CorsConfig corsConfig = Netty4HttpServerTransport.buildCorsConfig(settings); + assertTrue(corsConfig.isAnyOriginSupported()); + assertEquals(headers, corsConfig.allowedRequestHeaders()); + assertEquals(methods, corsConfig.allowedRequestMethods().stream().map(HttpMethod::name).collect(Collectors.toSet())); + } - // test with default values - methods = Strings.commaDelimitedListToSet(SETTING_CORS_ALLOW_METHODS.getDefault(Settings.EMPTY)); - headers = Strings.commaDelimitedListToSet(SETTING_CORS_ALLOW_HEADERS.getDefault(Settings.EMPTY)); - settings = Settings.builder() - .put(SETTING_CORS_ENABLED.getKey(), true) - .put(SETTING_CORS_ALLOW_ORIGIN.getKey(), "*") - .put(SETTING_CORS_ALLOW_CREDENTIALS.getKey(), true) - .build(); - transport = new Netty4HttpServerTransport(settings, networkService, bigArrays, threadPool); - corsConfig = transport.getCorsConfig(); - assertThat(corsConfig.isAnyOriginSupported(), equalTo(true)); - assertThat(corsConfig.allowedRequestHeaders(), equalTo(headers)); - assertThat(corsConfig.allowedRequestMethods().stream().map(HttpMethod::name).collect(Collectors.toSet()), equalTo(methods)); - transport.close(); + public void testCorsConfigWithDefaults() { + final Set methods = Strings.commaDelimitedListToSet(SETTING_CORS_ALLOW_METHODS.getDefault(Settings.EMPTY)); + final Set headers = Strings.commaDelimitedListToSet(SETTING_CORS_ALLOW_HEADERS.getDefault(Settings.EMPTY)); + final long maxAge = SETTING_CORS_MAX_AGE.getDefault(Settings.EMPTY); + final Settings settings = Settings.builder().put(SETTING_CORS_ENABLED.getKey(), true).build(); + final Netty4CorsConfig corsConfig = Netty4HttpServerTransport.buildCorsConfig(settings); + assertFalse(corsConfig.isAnyOriginSupported()); + assertEquals(Collections.emptySet(), corsConfig.origins().get()); + assertEquals(headers, corsConfig.allowedRequestHeaders()); + assertEquals(methods, corsConfig.allowedRequestMethods().stream().map(HttpMethod::name).collect(Collectors.toSet())); + assertEquals(maxAge, corsConfig.maxAge()); + assertFalse(corsConfig.isCredentialsAllowed()); } /**