From 21259cb6994325d5e9b7453fdf220699220a4be5 Mon Sep 17 00:00:00 2001 From: Jihoon Son Date: Fri, 22 Feb 2019 13:11:54 -0800 Subject: [PATCH 1/2] Fix exception when the scheme is missing in endpointUrl for S3 --- .../apache/druid/java/util/common/URIs.java | 39 +++++++++++ .../druid/java/util/common/URIsTest.java | 70 +++++++++++++++++++ .../storage/s3/S3StorageDruidModule.java | 4 +- 3 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 core/src/main/java/org/apache/druid/java/util/common/URIs.java create mode 100644 core/src/test/java/org/apache/druid/java/util/common/URIsTest.java diff --git a/core/src/main/java/org/apache/druid/java/util/common/URIs.java b/core/src/main/java/org/apache/druid/java/util/common/URIs.java new file mode 100644 index 000000000000..b0191f7bbc5f --- /dev/null +++ b/core/src/main/java/org/apache/druid/java/util/common/URIs.java @@ -0,0 +1,39 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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.apache.druid.java.util.common; + +import java.net.URI; + +public final class URIs +{ + public static URI parse(String strUri, String defaultScheme) + { + final String[] tokens = strUri.split("://"); + if (tokens.length == 1) { + return URI.create(StringUtils.format("%s://%s", defaultScheme, strUri)); + } else { + return URI.create(strUri); + } + } + + private URIs() + { + } +} diff --git a/core/src/test/java/org/apache/druid/java/util/common/URIsTest.java b/core/src/test/java/org/apache/druid/java/util/common/URIsTest.java new file mode 100644 index 000000000000..bb65e8e59817 --- /dev/null +++ b/core/src/test/java/org/apache/druid/java/util/common/URIsTest.java @@ -0,0 +1,70 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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.apache.druid.java.util.common; + +import org.junit.Assert; +import org.junit.Test; + +import java.net.URI; + +public class URIsTest +{ + @Test + public void testFullUri() + { + final String strUri = "https://test-user@127.0.0.1:8000/test/path?test-query#test-fragment"; + final URI uri = URIs.parse(strUri, "http"); + + Assert.assertEquals("https", uri.getScheme()); + Assert.assertEquals("test-user", uri.getUserInfo()); + Assert.assertEquals("127.0.0.1", uri.getHost()); + Assert.assertEquals(8000, uri.getPort()); + Assert.assertEquals("/test/path", uri.getPath()); + Assert.assertEquals("test-query", uri.getQuery()); + Assert.assertEquals("test-fragment", uri.getFragment()); + } + + @Test + public void testWithoutScheme() + { + final String strUri = "test-user@127.0.0.1:8000/test/path?test-query#test-fragment"; + final URI uri = URIs.parse(strUri, "http"); + + Assert.assertEquals("http", uri.getScheme()); + Assert.assertEquals("test-user", uri.getUserInfo()); + Assert.assertEquals("127.0.0.1", uri.getHost()); + Assert.assertEquals(8000, uri.getPort()); + Assert.assertEquals("/test/path", uri.getPath()); + Assert.assertEquals("test-query", uri.getQuery()); + Assert.assertEquals("test-fragment", uri.getFragment()); + } + + @Test + public void testSimpleUri() + { + final String strUri = "127.0.0.1:8000"; + final URI uri = URIs.parse(strUri, "https"); + + Assert.assertEquals("https", uri.getScheme()); + Assert.assertNull(uri.getUserInfo()); + Assert.assertEquals("127.0.0.1", uri.getHost()); + Assert.assertEquals(8000, uri.getPort()); + } +} diff --git a/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3StorageDruidModule.java b/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3StorageDruidModule.java index 2bac01896b8a..61d145d591e8 100644 --- a/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3StorageDruidModule.java +++ b/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3StorageDruidModule.java @@ -43,6 +43,7 @@ import org.apache.druid.guice.LazySingleton; import org.apache.druid.initialization.DruidModule; import org.apache.druid.java.util.common.IAE; +import org.apache.druid.java.util.common.URIs; import org.apache.druid.java.util.common.logger.Logger; import javax.annotation.Nullable; @@ -184,7 +185,8 @@ private static Protocol determineProtocol(AWSClientConfig clientConfig, AWSEndpo final Protocol protocolFromClientConfig = parseProtocol(clientConfig.getProtocol()); final String endpointUrl = endpointConfig.getUrl(); if (StringUtils.isNotEmpty(endpointUrl)) { - final URI uri = URI.create(endpointUrl); + //noinspection ConstantConditions + final URI uri = URIs.parse(endpointUrl, protocolFromClientConfig.toString()); final Protocol protocol = parseProtocol(uri.getScheme()); if (protocol != null && (protocol != protocolFromClientConfig)) { log.warn("[%s] protocol will be used for endpoint [%s]", protocol, endpointUrl); From 40fa31a713e36708ae26b76810be1a278ab3f444 Mon Sep 17 00:00:00 2001 From: Jihoon Son Date: Fri, 22 Feb 2019 13:13:29 -0800 Subject: [PATCH 2/2] add null check --- .../src/main/java/org/apache/druid/java/util/common/URIs.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/src/main/java/org/apache/druid/java/util/common/URIs.java b/core/src/main/java/org/apache/druid/java/util/common/URIs.java index b0191f7bbc5f..f2476e71f094 100644 --- a/core/src/main/java/org/apache/druid/java/util/common/URIs.java +++ b/core/src/main/java/org/apache/druid/java/util/common/URIs.java @@ -19,12 +19,16 @@ package org.apache.druid.java.util.common; +import com.google.common.base.Preconditions; + import java.net.URI; public final class URIs { public static URI parse(String strUri, String defaultScheme) { + Preconditions.checkNotNull(strUri, "strUri"); + Preconditions.checkNotNull(defaultScheme, "defaultScheme"); final String[] tokens = strUri.split("://"); if (tokens.length == 1) { return URI.create(StringUtils.format("%s://%s", defaultScheme, strUri));