Fixes CORS handling so that it uses the defaults

core/src/main/java/org/elasticsearch/common/Strings.java

@@ -33,6 +33,7 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedList;
@@ -509,7 +510,19 @@ public static String[] splitStringByCommaToArray(final String s) {
         else return s.split(",");
     }
 
+    /**
+     * A convenience method for splitting a delimited string into
+     * a set and trimming leading and trailing whitespace from all
+     * split strings.
+     *
+     * @param s the string to split
+     * @param c the delimiter to split on
+     * @return the set of split strings
+     */
     public static Set<String> splitStringToSet(final String s, final char c) {
+        if (s == null || s.isEmpty()) {
+            return Collections.emptySet();
+        }
         final char[] chars = s.toCharArray();
         int count = 1;
         for (final char x : chars) {
@@ -521,16 +534,25 @@ public static Set<String> splitStringToSet(final String s, final char c) {
         final int len = chars.length;
         int start = 0;  // starting index in chars of the current substring.
         int pos = 0;    // current index in chars.
+        int end = 0; // the position of the end of the current token
         for (; pos < len; pos++) {
             if (chars[pos] == c) {
-                int size = pos - start;
+                int size = end - start;
                 if (size > 0) { // only add non empty strings
                     result.add(new String(chars, start, size));
                 }
                 start = pos + 1;
+                end = start;
+            } else if (Character.isWhitespace(chars[pos])) {
+                if (start == pos) {
+                    // skip over preceding whitespace
+                    start++;
+                }
+            } else {
+                end = pos + 1;
             }
         }
-        int size = pos - start;
+        int size = end - start;
         if (size > 0) {
             result.add(new String(chars, start, size));
         }

core/src/main/java/org/elasticsearch/http/HttpTransportSettings.java

@@ -40,9 +40,9 @@ public final class HttpTransportSettings {
     public static final Setting<Integer> SETTING_CORS_MAX_AGE =
         Setting.intSetting("http.cors.max-age", 1728000, Property.NodeScope);
     public static final Setting<String> SETTING_CORS_ALLOW_METHODS =
-        new Setting<>("http.cors.allow-methods", "OPTIONS, HEAD, GET, POST, PUT, DELETE", (value) -> value, Property.NodeScope);
+        new Setting<>("http.cors.allow-methods", "OPTIONS,HEAD,GET,POST,PUT,DELETE", (value) -> value, Property.NodeScope);
     public static final Setting<String> SETTING_CORS_ALLOW_HEADERS =
-        new Setting<>("http.cors.allow-headers", "X-Requested-With, Content-Type, Content-Length", (value) -> value, Property.NodeScope);
+        new Setting<>("http.cors.allow-headers", "X-Requested-With,Content-Type,Content-Length", (value) -> value, Property.NodeScope);
     public static final Setting<Boolean> SETTING_CORS_ALLOW_CREDENTIALS =
         Setting.boolSetting("http.cors.allow-credentials", false, Property.NodeScope);
     public static final Setting<Boolean> SETTING_PIPELINING =

core/src/test/java/org/elasticsearch/common/StringsTests.java

@@ -19,6 +19,7 @@
 
 package org.elasticsearch.common;
 
+import org.elasticsearch.common.util.set.Sets;
 import org.elasticsearch.common.xcontent.ToXContent;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.test.ESTestCase;
@@ -73,4 +74,32 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
         assertThat(toString, containsString("\"ok\":\"here\""));
         assertThat(toString, containsString("\"catastrophe\":\"\""));
     }
+
+    public void testSplitStringToSet() {
+        assertEquals(Strings.splitStringByCommaToSet(null), Sets.newHashSet());
+        assertEquals(Strings.splitStringByCommaToSet(""), Sets.newHashSet());
+        assertEquals(Strings.splitStringByCommaToSet("a,b,c"), Sets.newHashSet("a","b","c"));
+        assertEquals(Strings.splitStringByCommaToSet("a, b, c"), Sets.newHashSet("a","b","c"));
+        assertEquals(Strings.splitStringByCommaToSet(" a ,  b, c  "), Sets.newHashSet("a","b","c"));
+        assertEquals(Strings.splitStringByCommaToSet("aa, bb, cc"), Sets.newHashSet("aa","bb","cc"));
+        assertEquals(Strings.splitStringByCommaToSet(" a "), Sets.newHashSet("a"));
+        assertEquals(Strings.splitStringByCommaToSet("   a   "), Sets.newHashSet("a"));
+        assertEquals(Strings.splitStringByCommaToSet("   aa   "), Sets.newHashSet("aa"));
+        assertEquals(Strings.splitStringByCommaToSet("   "), Sets.newHashSet());
+
+        assertEquals(Strings.splitStringToSet(null, ' '), Sets.newHashSet());
+        assertEquals(Strings.splitStringToSet("", ' '), Sets.newHashSet());
+        assertEquals(Strings.splitStringToSet("a b c", ' '), Sets.newHashSet("a","b","c"));
+        assertEquals(Strings.splitStringToSet("a, b, c", ' '), Sets.newHashSet("a,","b,","c"));
+        assertEquals(Strings.splitStringToSet(" a   b c  ", ' '), Sets.newHashSet("a","b","c"));
+        assertEquals(Strings.splitStringToSet("  a   b   c  ", ' '), Sets.newHashSet("a","b","c"));
+        assertEquals(Strings.splitStringToSet("aa bb cc", ' '), Sets.newHashSet("aa","bb","cc"));
+        assertEquals(Strings.splitStringToSet(" a ", ' '), Sets.newHashSet("a"));
+        assertEquals(Strings.splitStringToSet("    a    ", ' '), Sets.newHashSet("a"));
+        assertEquals(Strings.splitStringToSet(" a   ", ' '), Sets.newHashSet("a"));
+        assertEquals(Strings.splitStringToSet("a   ", ' '), Sets.newHashSet("a"));
+        assertEquals(Strings.splitStringToSet("   aa   ", ' '), Sets.newHashSet("aa"));
+        assertEquals(Strings.splitStringToSet("aa   ", ' '), Sets.newHashSet("aa"));
+        assertEquals(Strings.splitStringToSet("   ", ' '), Sets.newHashSet());
+    }
 }

modules/transport-netty3/src/main/java/org/elasticsearch/http/netty3/Netty3HttpServerTransport.java

@@ -81,9 +81,11 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Set;
 import java.util.concurrent.Executors;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.regex.Pattern;
+import java.util.stream.Collectors;
 
 import static org.elasticsearch.common.settings.Setting.boolSetting;
 import static org.elasticsearch.common.settings.Setting.byteSizeSetting;
@@ -390,14 +392,10 @@ private Netty3CorsConfig buildCorsConfig(Settings settings) {
         if (SETTING_CORS_ALLOW_CREDENTIALS.get(settings)) {
             builder.allowCredentials();
         }
-        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)
+        Set<String> strMethods = Strings.splitStringByCommaToSet(SETTING_CORS_ALLOW_METHODS.get(settings));
+        return builder.allowedRequestMethods(strMethods.stream().map(HttpMethod::valueOf).collect(Collectors.toSet()))
                       .maxAge(SETTING_CORS_MAX_AGE.get(settings))
-                      .allowedRequestHeaders(settings.getAsArray(SETTING_CORS_ALLOW_HEADERS.getKey()))
+                      .allowedRequestHeaders(Strings.splitStringByCommaToSet(SETTING_CORS_ALLOW_HEADERS.get(settings)))
                       .shortCircuit()
                       .build();
     }

modules/transport-netty3/src/main/java/org/elasticsearch/http/netty3/cors/Netty3CorsConfigBuilder.java

@@ -193,8 +193,8 @@ public Netty3CorsConfigBuilder maxAge(final long max) {
      * @param methods the {@link HttpMethod}s that should be allowed.
      * @return {@link Netty3CorsConfigBuilder} to support method chaining.
      */
-    public Netty3CorsConfigBuilder allowedRequestMethods(final HttpMethod... methods) {
-        requestMethods.addAll(Arrays.asList(methods));
+    public Netty3CorsConfigBuilder allowedRequestMethods(final Set<HttpMethod> methods) {
+        requestMethods.addAll(methods);
         return this;
     }
 
@@ -214,8 +214,8 @@ public Netty3CorsConfigBuilder allowedRequestMethods(final HttpMethod... methods
      * @param headers the headers to be added to the preflight 'Access-Control-Allow-Headers' response header.
      * @return {@link Netty3CorsConfigBuilder} to support method chaining.
      */
-    public Netty3CorsConfigBuilder allowedRequestHeaders(final String... headers) {
-        requestHeaders.addAll(Arrays.asList(headers));
+    public Netty3CorsConfigBuilder allowedRequestHeaders(final Set<String> headers) {
+        requestHeaders.addAll(headers);
         return this;
     }
 

modules/transport-netty3/src/test/java/org/elasticsearch/http/netty3/Netty3HttpServerTransportTests.java

@@ -23,6 +23,7 @@
 import org.elasticsearch.common.network.NetworkService;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.MockBigArrays;
+import org.elasticsearch.common.util.set.Sets;
 import org.elasticsearch.http.netty3.cors.Netty3CorsConfig;
 import org.elasticsearch.indices.breaker.NoneCircuitBreakerService;
 import org.elasticsearch.test.ESTestCase;
@@ -86,4 +87,19 @@ public void testCorsConfig() {
         assertThat(corsConfig.allowedRequestMethods().stream().map(HttpMethod::getName).collect(Collectors.toSet()), equalTo(methods));
         transport.close();
     }
+
+    public void testCorsConfigDefaults() {
+        final Set<String> headers = Sets.newHashSet("X-Requested-With", "Content-Type", "Content-Length");
+        final Set<String> methods = Sets.newHashSet("OPTIONS", "HEAD", "GET", "POST", "PUT", "DELETE");
+        final Settings settings = Settings.builder()
+                                      .put(SETTING_CORS_ENABLED.getKey(), true)
+                                      .put(SETTING_CORS_ALLOW_ORIGIN.getKey(), "*")
+                                      .put(SETTING_CORS_ALLOW_CREDENTIALS.getKey(), true)
+                                      .build();
+        final Netty3HttpServerTransport transport = new Netty3HttpServerTransport(settings, networkService, bigArrays, threadPool);
+        final Netty3CorsConfig corsConfig = transport.getCorsConfig();
+        assertThat(corsConfig.allowedRequestHeaders(), equalTo(headers));
+        assertThat(corsConfig.allowedRequestMethods().stream().map(HttpMethod::getName).collect(Collectors.toSet()), equalTo(methods));
+        transport.close();
+    }
 }