Enable IPv6 URIs in reindex from remote (#36874)

Reindex from remote was using a custom regex to dermine what URIs were
valid. This commit removes the custom regex and uses the java.net.URI
class instead, allowing IPv6 support without changing the existing
validation around a URI in reindex from remote.
This commit is contained in:
Michael Basnight 2018-12-20 13:48:35 -06:00 committed by GitHub
parent d00780d00c
commit a64fea10e2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 37 additions and 10 deletions

View File

@ -40,10 +40,10 @@ import org.elasticsearch.script.Script;
import java.io.IOException; import java.io.IOException;
import java.io.InputStream; import java.io.InputStream;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import static java.util.Collections.emptyMap; import static java.util.Collections.emptyMap;
import static java.util.Objects.requireNonNull; import static java.util.Objects.requireNonNull;
@ -56,7 +56,6 @@ import static org.elasticsearch.rest.RestRequest.Method.POST;
*/ */
public class RestReindexAction extends AbstractBaseReindexRestHandler<ReindexRequest, ReindexAction> { public class RestReindexAction extends AbstractBaseReindexRestHandler<ReindexRequest, ReindexAction> {
static final ObjectParser<ReindexRequest, Void> PARSER = new ObjectParser<>("reindex"); static final ObjectParser<ReindexRequest, Void> PARSER = new ObjectParser<>("reindex");
private static final Pattern HOST_PATTERN = Pattern.compile("(?<scheme>[^:]+)://(?<host>[^:]+):(?<port>\\d+)(?<pathPrefix>/.*)?");
static { static {
ObjectParser.Parser<ReindexRequest, Void> sourceParser = (parser, request, context) -> { ObjectParser.Parser<ReindexRequest, Void> sourceParser = (parser, request, context) -> {
@ -136,15 +135,27 @@ public class RestReindexAction extends AbstractBaseReindexRestHandler<ReindexReq
String username = extractString(remote, "username"); String username = extractString(remote, "username");
String password = extractString(remote, "password"); String password = extractString(remote, "password");
String hostInRequest = requireNonNull(extractString(remote, "host"), "[host] must be specified to reindex from a remote cluster"); String hostInRequest = requireNonNull(extractString(remote, "host"), "[host] must be specified to reindex from a remote cluster");
Matcher hostMatcher = HOST_PATTERN.matcher(hostInRequest); URI uri;
if (false == hostMatcher.matches()) { try {
throw new IllegalArgumentException("[host] must be of the form [scheme]://[host]:[port](/[pathPrefix])? but was [" uri = new URI(hostInRequest);
+ hostInRequest + "]"); // URI has less stringent URL parsing than our code. We want to fail if all values are not provided.
if (uri.getPort() == -1) {
throw new URISyntaxException(hostInRequest, "The port was not defined in the [host]");
} }
String scheme = hostMatcher.group("scheme"); } catch (URISyntaxException ex) {
String host = hostMatcher.group("host"); throw new IllegalArgumentException("[host] must be of the form [scheme]://[host]:[port](/[pathPrefix])? but was ["
String pathPrefix = hostMatcher.group("pathPrefix"); + hostInRequest + "]", ex);
int port = Integer.parseInt(hostMatcher.group("port")); }
String scheme = uri.getScheme();
String host = uri.getHost();
int port = uri.getPort();
String pathPrefix = null;
if (uri.getPath().isEmpty() == false) {
pathPrefix = uri.getPath();
}
Map<String, String> headers = extractStringStringMap(remote, "headers"); Map<String, String> headers = extractStringStringMap(remote, "headers");
TimeValue socketTimeout = extractTimeValue(remote, "socket_timeout", RemoteInfo.DEFAULT_SOCKET_TIMEOUT); TimeValue socketTimeout = extractTimeValue(remote, "socket_timeout", RemoteInfo.DEFAULT_SOCKET_TIMEOUT);
TimeValue connectTimeout = extractTimeValue(remote, "connect_timeout", RemoteInfo.DEFAULT_CONNECT_TIMEOUT); TimeValue connectTimeout = extractTimeValue(remote, "connect_timeout", RemoteInfo.DEFAULT_CONNECT_TIMEOUT);

View File

@ -102,6 +102,12 @@ public class ReindexFromRemoteWhitelistTests extends ESTestCase {
assertMatchesTooMuch(random); assertMatchesTooMuch(random);
} }
public void testIPv6Address() {
List<String> whitelist = randomWhitelist();
whitelist.add("[::1]:*");
checkRemoteWhitelist(buildRemoteWhitelist(whitelist), newRemoteInfo("[::1]", 9200));
}
private void assertMatchesTooMuch(List<String> whitelist) { private void assertMatchesTooMuch(List<String> whitelist) {
Exception e = expectThrows(IllegalArgumentException.class, () -> buildRemoteWhitelist(whitelist)); Exception e = expectThrows(IllegalArgumentException.class, () -> buildRemoteWhitelist(whitelist));
assertEquals("Refusing to start because whitelist " + whitelist + " accepts all addresses. " assertEquals("Refusing to start because whitelist " + whitelist + " accepts all addresses. "

View File

@ -78,6 +78,8 @@ public class RestReindexActionTests extends ESTestCase {
public void testBuildRemoteInfoWithoutAllParts() throws IOException { public void testBuildRemoteInfoWithoutAllParts() throws IOException {
expectThrows(IllegalArgumentException.class, () -> buildRemoteInfoHostTestCase("example.com")); expectThrows(IllegalArgumentException.class, () -> buildRemoteInfoHostTestCase("example.com"));
expectThrows(IllegalArgumentException.class, () -> buildRemoteInfoHostTestCase(":9200"));
expectThrows(IllegalArgumentException.class, () -> buildRemoteInfoHostTestCase("http://:9200"));
expectThrows(IllegalArgumentException.class, () -> buildRemoteInfoHostTestCase("example.com:9200")); expectThrows(IllegalArgumentException.class, () -> buildRemoteInfoHostTestCase("example.com:9200"));
expectThrows(IllegalArgumentException.class, () -> buildRemoteInfoHostTestCase("http://example.com")); expectThrows(IllegalArgumentException.class, () -> buildRemoteInfoHostTestCase("http://example.com"));
} }
@ -99,6 +101,14 @@ public class RestReindexActionTests extends ESTestCase {
assertEquals(RemoteInfo.DEFAULT_SOCKET_TIMEOUT, info.getSocketTimeout()); assertEquals(RemoteInfo.DEFAULT_SOCKET_TIMEOUT, info.getSocketTimeout());
assertEquals(RemoteInfo.DEFAULT_CONNECT_TIMEOUT, info.getConnectTimeout()); assertEquals(RemoteInfo.DEFAULT_CONNECT_TIMEOUT, info.getConnectTimeout());
info = buildRemoteInfoHostTestCase("https://[::1]:9201");
assertEquals("https", info.getScheme());
assertEquals("[::1]", info.getHost());
assertEquals(9201, info.getPort());
assertNull(info.getPathPrefix());
assertEquals(RemoteInfo.DEFAULT_SOCKET_TIMEOUT, info.getSocketTimeout());
assertEquals(RemoteInfo.DEFAULT_CONNECT_TIMEOUT, info.getConnectTimeout());
info = buildRemoteInfoHostTestCase("https://other.example.com:9201/"); info = buildRemoteInfoHostTestCase("https://other.example.com:9201/");
assertEquals("https", info.getScheme()); assertEquals("https", info.getScheme());
assertEquals("other.example.com", info.getHost()); assertEquals("other.example.com", info.getHost());