Internal: only have one CORS allow origin setting string

We currently have two, which is confusing when you  read the code (especially if one is used with a null default and the other with '*')

Note: this is not a real bug, just a  clean up. We do the right thing...

Closes #13988
This commit is contained in:
Boaz Leskes 2015-10-07 10:49:28 +02:00
parent de7d3cf752
commit fbe9c49e8f
3 changed files with 12 additions and 15 deletions

View File

@ -40,7 +40,7 @@ public class HttpRequestHandler extends SimpleChannelUpstreamHandler {
public HttpRequestHandler(NettyHttpServerTransport serverTransport, boolean detailedErrorsEnabled) {
this.serverTransport = serverTransport;
this.corsPattern = RestUtils.getCorsSettingRegex(serverTransport.settings());
this.corsPattern = RestUtils.checkCorsSettingForRegex(serverTransport.settings().get(NettyHttpServerTransport.SETTING_CORS_ALLOW_ORIGIN));
this.httpPipeliningEnabled = serverTransport.pipelining;
this.detailedErrorsEnabled = detailedErrorsEnabled;
}

View File

@ -19,12 +19,11 @@
package org.elasticsearch.rest.support;
import java.nio.charset.StandardCharsets;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.path.PathTrie;
import org.elasticsearch.common.settings.Settings;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.util.Map;
import java.util.regex.Pattern;
@ -39,7 +38,6 @@ public class RestUtils {
return RestUtils.decodeComponent(value);
}
};
public static final String HTTP_CORS_ALLOW_ORIGIN_SETTING = "http.cors.allow-origin";
public static boolean isBrowser(@Nullable String userAgent) {
if (userAgent == null) {
@ -224,9 +222,13 @@ public class RestUtils {
/**
* Determine if CORS setting is a regex
*
* @return a corresponding {@link Pattern} if so and o.w. null.
*/
public static Pattern getCorsSettingRegex(Settings settings) {
String corsSetting = settings.get(HTTP_CORS_ALLOW_ORIGIN_SETTING, "*");
public static Pattern checkCorsSettingForRegex(String corsSetting) {
if (corsSetting == null) {
return null;
}
int len = corsSetting.length();
boolean isRegex = len > 2 && corsSetting.startsWith("/") && corsSetting.endsWith("/");

View File

@ -19,7 +19,6 @@
package org.elasticsearch.rest.util;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.rest.support.RestUtils;
import org.elasticsearch.test.ESTestCase;
import org.junit.Test;
@ -29,10 +28,7 @@ import java.util.Locale;
import java.util.Map;
import java.util.regex.Pattern;
import static org.elasticsearch.common.settings.Settings.settingsBuilder;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;
import static org.hamcrest.Matchers.*;
/**
*
@ -139,7 +135,6 @@ public class RestUtilsTests extends ESTestCase {
assertCorsSettingRegexIsNull("/foo");
assertCorsSettingRegexIsNull("foo");
assertCorsSettingRegexIsNull("");
assertThat(RestUtils.getCorsSettingRegex(Settings.EMPTY), is(nullValue()));
}
public void testCrazyURL() {
@ -153,15 +148,15 @@ public class RestUtilsTests extends ESTestCase {
}
private void assertCorsSettingRegexIsNull(String settingsValue) {
assertThat(RestUtils.getCorsSettingRegex(settingsBuilder().put("http.cors.allow-origin", settingsValue).build()), is(nullValue()));
assertThat(RestUtils.checkCorsSettingForRegex(settingsValue), is(nullValue()));
}
private void assertCorsSettingRegex(String settingsValue, Pattern pattern) {
assertThat(RestUtils.getCorsSettingRegex(settingsBuilder().put("http.cors.allow-origin", settingsValue).build()).toString(), is(pattern.toString()));
assertThat(RestUtils.checkCorsSettingForRegex(settingsValue).toString(), is(pattern.toString()));
}
private void assertCorsSettingRegexMatches(String settingsValue, boolean expectMatch, String ... candidates) {
Pattern pattern = RestUtils.getCorsSettingRegex(settingsBuilder().put("http.cors.allow-origin", settingsValue).build());
Pattern pattern = RestUtils.checkCorsSettingForRegex(settingsValue);
for (String candidate : candidates) {
assertThat(String.format(Locale.ROOT, "Expected pattern %s to match against %s: %s", settingsValue, candidate, expectMatch),
pattern.matcher(candidate).matches(), is(expectMatch));