honeymoose/OpenSearch
1
0
Fork 0
mirror of https://github.com/honeymoose/OpenSearch.git synced 2025-03-27 18:38:41 +00:00
Code Issues Packages Projects Releases Wiki Activity

Bad regex in CORS settings should throw a nicer error (#34035)

Browse Source 
Currently a bad regex in CORS settings throws a PatternSyntaxException, which
then bubbles up through the bootstrap code, meaning users have to parse a
stack trace to work out where the problem is.  We should instead catch this
exception and rethrow with a more useful error message.
This commit is contained in:
Alan Woodward 2018-09-27 10:02:29 +01:00 committed by GitHub
parent 53d10bb3b2
commit 1cc53b5988
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 48 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpServerTransport.java
View File

@ -48,6 +48,7 @@ import org.elasticsearch.common.network.NetworkService;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Setting.Property;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.settings.SettingsException;
import org.elasticsearch.common.unit.ByteSizeUnit;
import org.elasticsearch.common.unit.ByteSizeValue;
import org.elasticsearch.common.util.BigArrays;
@ -68,6 +69,7 @@ import java.net.InetSocketAddress;
import java.util.Arrays;
import java.util.concurrent.TimeUnit;
import java.util.regex.Pattern;
import java.util.regex.PatternSyntaxException;
import static org.elasticsearch.common.util.concurrent.EsExecutors.daemonThreadFactory;
import static org.elasticsearch.http.HttpTransportSettings.SETTING_CORS_ALLOW_CREDENTIALS;
@ -241,11 +243,15 @@ public class Netty4HttpServerTransport extends AbstractHttpServerTransport {
} else if (origin.equals(ANY_ORIGIN)) {
builder = Netty4CorsConfigBuilder.forAnyOrigin();
} else {
Pattern p = RestUtils.checkCorsSettingForRegex(origin);
if (p == null) {
builder = Netty4CorsConfigBuilder.forOrigins(RestUtils.corsSettingAsArray(origin));
} else {
builder = Netty4CorsConfigBuilder.forPattern(p);
try {
Pattern p = RestUtils.checkCorsSettingForRegex(origin);
if (p == null) {
builder = Netty4CorsConfigBuilder.forOrigins(RestUtils.corsSettingAsArray(origin));
} else {
builder = Netty4CorsConfigBuilder.forPattern(p);
}
} catch (PatternSyntaxException e) {
throw new SettingsException("Bad regex in [" + SETTING_CORS_ALLOW_ORIGIN.getKey() + "]: [" + origin + "]", e);
}
}
if (SETTING_CORS_ALLOW_CREDENTIALS.get(settings)) {

13
modules/transport-netty4/src/test/java/org/elasticsearch/http/netty4/Netty4HttpServerTransportTests.java
View File

@ -44,6 +44,7 @@ import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.network.NetworkService;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.settings.SettingsException;
import org.elasticsearch.common.transport.TransportAddress;
import org.elasticsearch.common.unit.ByteSizeValue;
import org.elasticsearch.common.unit.TimeValue;
@ -75,6 +76,7 @@ import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
import java.util.regex.PatternSyntaxException;
import java.util.stream.Collectors;
import static org.elasticsearch.common.Strings.collectionToDelimitedString;
@ -148,6 +150,17 @@ public class Netty4HttpServerTransportTests extends ESTestCase {
assertFalse(corsConfig.isCredentialsAllowed());
}
public void testCorsConfigWithBadRegex() {
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();
SettingsException e = expectThrows(SettingsException.class, () -> Netty4HttpServerTransport.buildCorsConfig(settings));
assertThat(e.getMessage(), containsString("Bad regex in [http.cors.allow-origin]: [/[*/]"));
assertThat(e.getCause(), instanceOf(PatternSyntaxException.class));
}
/**
* Test that {@link Netty4HttpServerTransport} supports the "Expect: 100-continue" HTTP header
* @throws InterruptedException if the client communication with the server is interrupted

16
plugins/transport-nio/src/main/java/org/elasticsearch/http/nio/NioHttpServerTransport.java
View File

@ -27,6 +27,7 @@ import org.elasticsearch.common.network.NetworkService;
import org.elasticsearch.common.recycler.Recycler;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.settings.SettingsException;
import org.elasticsearch.common.unit.ByteSizeValue;
import org.elasticsearch.common.util.BigArrays;
import org.elasticsearch.common.util.PageCacheRecycler;
@ -57,6 +58,7 @@ import java.nio.channels.SocketChannel;
import java.util.Arrays;
import java.util.function.Consumer;
import java.util.regex.Pattern;
import java.util.regex.PatternSyntaxException;
import static org.elasticsearch.common.settings.Setting.intSetting;
import static org.elasticsearch.common.util.concurrent.EsExecutors.daemonThreadFactory;
@ -176,11 +178,15 @@ public class NioHttpServerTransport extends AbstractHttpServerTransport {
} else if (origin.equals(ANY_ORIGIN)) {
builder = NioCorsConfigBuilder.forAnyOrigin();
} else {
Pattern p = RestUtils.checkCorsSettingForRegex(origin);
if (p == null) {
builder = NioCorsConfigBuilder.forOrigins(RestUtils.corsSettingAsArray(origin));
} else {
builder = NioCorsConfigBuilder.forPattern(p);
try {
Pattern p = RestUtils.checkCorsSettingForRegex(origin);
if (p == null) {
builder = NioCorsConfigBuilder.forOrigins(RestUtils.corsSettingAsArray(origin));
} else {
builder = NioCorsConfigBuilder.forPattern(p);
}
} catch (PatternSyntaxException e) {
throw new SettingsException("Bad regex in [" + SETTING_CORS_ALLOW_ORIGIN.getKey() + "]: [" + origin + "]", e);
}
}
if (SETTING_CORS_ALLOW_CREDENTIALS.get(settings)) {

13
plugins/transport-nio/src/test/java/org/elasticsearch/http/nio/NioHttpServerTransportTests.java
View File

@ -37,6 +37,7 @@ import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.network.NetworkService;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.settings.SettingsException;
import org.elasticsearch.common.transport.TransportAddress;
import org.elasticsearch.common.unit.ByteSizeValue;
import org.elasticsearch.common.util.MockBigArrays;
@ -65,6 +66,7 @@ import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
import java.util.concurrent.atomic.AtomicReference;
import java.util.regex.PatternSyntaxException;
import java.util.stream.Collectors;
import static org.elasticsearch.http.HttpTransportSettings.SETTING_CORS_ALLOW_CREDENTIALS;
@ -139,6 +141,17 @@ public class NioHttpServerTransportTests extends ESTestCase {
assertFalse(corsConfig.isCredentialsAllowed());
}
public void testCorsConfigWithBadRegex() {
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();
SettingsException e = expectThrows(SettingsException.class, () -> NioHttpServerTransport.buildCorsConfig(settings));
assertThat(e.getMessage(), containsString("Bad regex in [http.cors.allow-origin]: [/[*/]"));
assertThat(e.getCause(), instanceOf(PatternSyntaxException.class));
}
/**
* Test that {@link NioHttpServerTransport} supports the "Expect: 100-continue" HTTP header
* @throws InterruptedException if the client communication with the server is interrupted