From 1ecc62778e2ae03937d6b498026652ec098f4fa2 Mon Sep 17 00:00:00 2001 From: Ali Beyad Date: Mon, 4 Apr 2016 22:27:01 -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. Closes #17524 Backports #17523 --- .../http/netty/NettyHttpServerTransport.java | 9 +- .../http/netty/cors/CorsHandler.java | 16 +-- .../netty/NettyHttpServerTransportTests.java | 97 +++++++++++++++++++ 3 files changed, 107 insertions(+), 15 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 c4169cfbe020e..65415eb423b4d 100644 --- a/core/src/main/java/org/elasticsearch/http/netty/NettyHttpServerTransport.java +++ b/core/src/main/java/org/elasticsearch/http/netty/NettyHttpServerTransport.java @@ -59,6 +59,7 @@ import org.jboss.netty.handler.codec.http.HttpMethod; import org.jboss.netty.handler.codec.http.HttpRequestDecoder; import org.jboss.netty.handler.timeout.ReadTimeoutException; +import sun.security.util.Length; import java.io.IOException; import java.net.InetAddress; @@ -105,8 +106,8 @@ public class NettyHttpServerTransport extends AbstractLifecycleComponent methods = config.allowedRequestMethods(); - Iterator iter = methods.iterator(); - final int size = methods.size(); - int count = 0; - StringBuilder buf = new StringBuilder(); - while (iter.hasNext()) { - buf.append(iter.next().getName().trim()); - if (++count < size) { - buf.append(", "); - } + Set strMethods = new HashSet<>(); + for (HttpMethod method : config.allowedRequestMethods()) { + strMethods.add(method.getName().trim()); } - response.headers().set(ACCESS_CONTROL_ALLOW_METHODS, buf.toString()); + response.headers().set(ACCESS_CONTROL_ALLOW_METHODS, strMethods); } 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..18704498317d2 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/http/netty/NettyHttpServerTransportTests.java @@ -0,0 +1,97 @@ +/* + * 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 org.junit.Test; + +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; + +import static org.elasticsearch.http.netty.NettyHttpServerTransport.SETTING_CORS_ALLOW_CREDENTIALS; +import static org.elasticsearch.http.netty.NettyHttpServerTransport.SETTING_CORS_ALLOW_HEADERS; +import static org.elasticsearch.http.netty.NettyHttpServerTransport.SETTING_CORS_ALLOW_METHODS; +import static org.elasticsearch.http.netty.NettyHttpServerTransport.SETTING_CORS_ALLOW_ORIGIN; +import static org.elasticsearch.http.netty.NettyHttpServerTransport.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; + } + + @Test + public void testCorsConfig() throws Exception { + 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, true) + .put(SETTING_CORS_ALLOW_ORIGIN, "*") + .put(SETTING_CORS_ALLOW_METHODS, Strings.collectionToCommaDelimitedString(methods)) + .put(SETTING_CORS_ALLOW_HEADERS, Strings.collectionToCommaDelimitedString(headers)) + .put(SETTING_CORS_ALLOW_CREDENTIALS, true) + .build(); + final NettyHttpServerTransport transport = new NettyHttpServerTransport(settings, networkService, bigArrays); + final CorsConfig corsConfig = transport.getCorsConfig(); + assertThat(corsConfig.isAnyOriginSupported(), equalTo(true)); + assertThat(corsConfig.allowedRequestHeaders(), equalTo(headers)); + final Set allowedRequestMethods = new HashSet<>(); + for (HttpMethod method : corsConfig.allowedRequestMethods()) { + allowedRequestMethods.add(method.getName()); + } + assertThat(allowedRequestMethods, equalTo(methods)); + transport.close(); + } +}