From 4fbe1ad8de20978b13234b713f8526cbce9c627c Mon Sep 17 00:00:00 2001 From: Ali Beyad Date: Mon, 4 Apr 2016 20:51:08 -0400 Subject: [PATCH] Fixes reading of CORS pre-flight headers and methods CORS headers and methods config parameters must be read as arrays. This commit fixes the issue. It affects http.cors.allow-methods and http.cors.allow-headers. Fixes #17483 --- .../http/netty/NettyHttpServerTransport.java | 4 +- .../http/netty/cors/CorsHandler.java | 8 +- .../netty/NettyHttpServerTransportTests.java | 92 +++++++++++++++++++ 3 files changed, 97 insertions(+), 7 deletions(-) create mode 100644 core/src/test/java/org/elasticsearch/http/netty/NettyHttpServerTransportTests.java diff --git a/core/src/main/java/org/elasticsearch/http/netty/NettyHttpServerTransport.java b/core/src/main/java/org/elasticsearch/http/netty/NettyHttpServerTransport.java index 332380d9fb198..4d95065fa1686 100644 --- a/core/src/main/java/org/elasticsearch/http/netty/NettyHttpServerTransport.java +++ b/core/src/main/java/org/elasticsearch/http/netty/NettyHttpServerTransport.java @@ -392,14 +392,14 @@ private CorsConfig buildCorsConfig(Settings settings) { if (SETTING_CORS_ALLOW_CREDENTIALS.get(settings)) { builder.allowCredentials(); } - String[] strMethods = settings.getAsArray(SETTING_CORS_ALLOW_METHODS.get(settings), new String[0]); + String[] strMethods = settings.getAsArray(SETTING_CORS_ALLOW_METHODS.getKey()); 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.get(settings), new String[0])) + .allowedRequestHeaders(settings.getAsArray(SETTING_CORS_ALLOW_HEADERS.getKey())) .shortCircuit() .build(); } diff --git a/core/src/main/java/org/elasticsearch/http/netty/cors/CorsHandler.java b/core/src/main/java/org/elasticsearch/http/netty/cors/CorsHandler.java index 76e4e67e6cdfa..b04d9013c4f2e 100644 --- a/core/src/main/java/org/elasticsearch/http/netty/cors/CorsHandler.java +++ b/core/src/main/java/org/elasticsearch/http/netty/cors/CorsHandler.java @@ -31,7 +31,6 @@ import org.jboss.netty.handler.codec.http.HttpRequest; import org.jboss.netty.handler.codec.http.HttpResponse; -import java.util.List; import java.util.stream.Collectors; import static org.jboss.netty.handler.codec.http.HttpHeaders.Names.ACCESS_CONTROL_ALLOW_CREDENTIALS; @@ -214,10 +213,9 @@ private static boolean isPreflightRequest(final HttpRequest request) { } private void setAllowMethods(final HttpResponse response) { - response.headers().set(ACCESS_CONTROL_ALLOW_METHODS, - String.join(", ", config.allowedRequestMethods().stream() - .map(HttpMethod::getName) - .collect(Collectors.toList())).trim()); + response.headers().set(ACCESS_CONTROL_ALLOW_METHODS, config.allowedRequestMethods().stream() + .map(m -> m.getName().trim()) + .collect(Collectors.toList())); } private void setAllowHeaders(final HttpResponse response) { diff --git a/core/src/test/java/org/elasticsearch/http/netty/NettyHttpServerTransportTests.java b/core/src/test/java/org/elasticsearch/http/netty/NettyHttpServerTransportTests.java new file mode 100644 index 0000000000000..13ac26b010da2 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/http/netty/NettyHttpServerTransportTests.java @@ -0,0 +1,92 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch 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 org.elasticsearch.http.netty; + +import org.elasticsearch.cache.recycler.MockPageCacheRecycler; +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.network.NetworkService; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.MockBigArrays; +import org.elasticsearch.http.netty.cors.CorsConfig; +import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.threadpool.ThreadPool; +import org.jboss.netty.handler.codec.http.HttpMethod; +import org.junit.After; +import org.junit.Before; + +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; +import java.util.stream.Collectors; + +import static org.elasticsearch.http.HttpTransportSettings.SETTING_CORS_ALLOW_CREDENTIALS; +import static org.elasticsearch.http.HttpTransportSettings.SETTING_CORS_ALLOW_HEADERS; +import static org.elasticsearch.http.HttpTransportSettings.SETTING_CORS_ALLOW_METHODS; +import static org.elasticsearch.http.HttpTransportSettings.SETTING_CORS_ALLOW_ORIGIN; +import static org.elasticsearch.http.HttpTransportSettings.SETTING_CORS_ENABLED; +import static org.hamcrest.Matchers.equalTo; + +/** + * Tests for the {@link NettyHttpServerTransport} class. + */ +public class NettyHttpServerTransportTests extends ESTestCase { + private NetworkService networkService; + private ThreadPool threadPool; + private MockPageCacheRecycler mockPageCacheRecycler; + private MockBigArrays bigArrays; + + @Before + public void setup() throws Exception { + networkService = new NetworkService(Settings.EMPTY); + threadPool = new ThreadPool("test"); + mockPageCacheRecycler = new MockPageCacheRecycler(Settings.EMPTY, threadPool); + bigArrays = new MockBigArrays(mockPageCacheRecycler, new NoneCircuitBreakerService()); + } + + @After + public void shutdown() throws Exception { + if (threadPool != null) { + threadPool.shutdownNow(); + } + threadPool = null; + networkService = null; + mockPageCacheRecycler = null; + bigArrays = null; + } + + 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() + .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 NettyHttpServerTransport transport = new NettyHttpServerTransport(settings, networkService, bigArrays, threadPool); + final CorsConfig corsConfig = transport.getCorsConfig(); + assertThat(corsConfig.isAnyOriginSupported(), equalTo(true)); + assertThat(corsConfig.allowedRequestHeaders(), equalTo(headers)); + assertThat(corsConfig.allowedRequestMethods().stream().map(HttpMethod::getName).collect(Collectors.toSet()), equalTo(methods)); + transport.close(); + } +}