Fixes reading of CORS pre-flight headers and methods

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<HttpSer
     public static final int DEFAULT_SETTING_PIPELINING_MAX_EVENTS = 10000;
     public static final String DEFAULT_PORT_RANGE = "9200-9300";
 
-    private static final String DEFAULT_CORS_METHODS = "OPTIONS, HEAD, GET, POST, PUT, DELETE";
-    private static final String DEFAULT_CORS_HEADERS = "X-Requested-With, Content-Type, Content-Length";
+    private static final String[] DEFAULT_CORS_METHODS = { "OPTIONS", "HEAD", "GET", "POST", "PUT", "DELETE" };
+    private static final String[] DEFAULT_CORS_HEADERS = { "X-Requested-With", "Content-Type", "Content-Length" };
     private static final int DEFAULT_CORS_MAX_AGE = 1728000;
 
     protected final NetworkService networkService;
@@ -353,14 +354,14 @@ private CorsConfig buildCorsConfig(Settings settings) {
         if (settings.getAsBoolean(SETTING_CORS_ALLOW_CREDENTIALS, false)) {
             builder.allowCredentials();
         }
-        String[] strMethods = settings.getAsArray(settings.get(SETTING_CORS_ALLOW_METHODS, DEFAULT_CORS_METHODS), new String[0]);
+        String[] strMethods = settings.getAsArray(SETTING_CORS_ALLOW_METHODS, DEFAULT_CORS_METHODS);
         HttpMethod[] methods = new HttpMethod[strMethods.length];
         for (int i = 0; i < methods.length; i++) {
             methods[i] = HttpMethod.valueOf(strMethods[i]);
         }
         return builder.allowedRequestMethods(methods)
                       .maxAge(settings.getAsInt(SETTING_CORS_MAX_AGE, DEFAULT_CORS_MAX_AGE))
-                      .allowedRequestHeaders(settings.getAsArray(settings.get(SETTING_CORS_ALLOW_HEADERS, DEFAULT_CORS_HEADERS), new String[0]))
+                      .allowedRequestHeaders(settings.getAsArray(SETTING_CORS_ALLOW_HEADERS, DEFAULT_CORS_HEADERS))
                       .shortCircuit()
                       .build();
     }

core/src/main/java/org/elasticsearch/http/netty/cors/CorsHandler.java

@@ -31,6 +31,7 @@
 import org.jboss.netty.handler.codec.http.HttpRequest;
 import org.jboss.netty.handler.codec.http.HttpResponse;
 
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.Set;
 
@@ -214,18 +215,11 @@ private static boolean isPreflightRequest(final HttpRequest request) {
     }
 
     private void setAllowMethods(final HttpResponse response) {
-        Set<HttpMethod> methods = config.allowedRequestMethods();
-        Iterator<HttpMethod> 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<String> 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) {

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<String> methods = new HashSet<>(Arrays.asList("get", "options", "post"));
+        final Set<String> 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<String> allowedRequestMethods = new HashSet<>();
+        for (HttpMethod method : corsConfig.allowedRequestMethods()) {
+            allowedRequestMethods.add(method.getName());
+        }
+        assertThat(allowedRequestMethods, equalTo(methods));
+        transport.close();
+    }
+}