Fix URL validation

Author: AntoxaAntoxic

extra/pom.xml

@@ -39,6 +39,7 @@
         <commons.collections.version>4.4</commons.collections.version>
         <commons.compress.version>1.27.1</commons.compress.version>
         <commons-math3.version>3.6.1</commons-math3.version>
+        <commons-validator.version>1.10.0</commons-validator.version>
         <scram.version>2.1</scram.version>
         <httpclient.version>4.5.14</httpclient.version>
         <ipaddress.version>5.5.1</ipaddress.version>
@@ -135,6 +136,11 @@
                 <artifactId>commons-math3</artifactId>
                 <version>${commons-math3.version}</version>
             </dependency>
+            <dependency>
+                <groupId>commons-validator</groupId>
+                <artifactId>commons-validator</artifactId>
+                <version>${commons-validator.version}</version>
+            </dependency>
             <!-- TODO: refactor code to replace URIBuilder with something else so that this dep can be removed -->
             <dependency>
                 <groupId>org.apache.httpcomponents</groupId>

pom.xml

@@ -114,6 +114,10 @@
             <groupId>org.apache.httpcomponents</groupId>
             <artifactId>httpclient</artifactId>
         </dependency>
+        <dependency>
+            <groupId>commons-validator</groupId>
+            <artifactId>commons-validator</artifactId>
+        </dependency>
         <dependency>
             <groupId>com.github.seancfoley</groupId>
             <artifactId>ipaddress</artifactId>

src/main/java/org/prebid/server/settings/HttpApplicationSettings.java

@@ -94,16 +94,15 @@ public HttpApplicationSettings(HttpClient httpClient,
 
         this.httpClient = Objects.requireNonNull(httpClient);
         this.mapper = Objects.requireNonNull(mapper);
-        this.endpoint = HttpUtil.validateUrl(Objects.requireNonNull(endpoint));
-        this.ampEndpoint = HttpUtil.validateUrl(Objects.requireNonNull(ampEndpoint));
-        this.videoEndpoint = HttpUtil.validateUrl(Objects.requireNonNull(videoEndpoint));
-        this.categoryEndpoint = HttpUtil.validateUrl(Objects.requireNonNull(categoryEndpoint));
+        this.endpoint = HttpUtil.validateUrlSyntax(Objects.requireNonNull(endpoint));
+        this.ampEndpoint = HttpUtil.validateUrlSyntax(Objects.requireNonNull(ampEndpoint));
+        this.videoEndpoint = HttpUtil.validateUrlSyntax(Objects.requireNonNull(videoEndpoint));
+        this.categoryEndpoint = HttpUtil.validateUrlSyntax(Objects.requireNonNull(categoryEndpoint));
         this.isRfc3986Compatible = isRfc3986Compatible;
     }
 
     @Override
     public Future<Account> getAccountById(String accountId, Timeout timeout) {
-
         return fetchAccountsByIds(Collections.singleton(accountId), timeout)
                 .map(accounts -> accounts.stream()
                         .findFirst()

src/main/java/org/prebid/server/util/HttpUtil.java

@@ -8,6 +8,7 @@
 import io.vertx.core.http.HttpServerResponse;
 import io.vertx.ext.web.RoutingContext;
 import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.validator.routines.UrlValidator;
 import org.prebid.server.log.ConditionalLogger;
 import org.prebid.server.log.Logger;
 import org.prebid.server.log.LoggerFactory;
@@ -78,12 +79,15 @@ public final class HttpUtil {
     public static final String MACROS_OPEN = "{{";
     public static final String MACROS_CLOSE = "}}";
 
+    private static final UrlValidator URL_VALIDAROR = UrlValidator.getInstance();
+
     private HttpUtil() {
     }
 
     /**
      * Checks the input string for using as URL.
      */
+    @Deprecated
     public static String validateUrl(String url) {
         if (containsMacrosses(url)) {
             return url;
@@ -96,6 +100,14 @@ public static String validateUrl(String url) {
         }
     }
 
+    public static String validateUrlSyntax(String url) {
+        if (containsMacrosses(url) || URL_VALIDAROR.isValid(url)) {
+            return url;
+        }
+
+        throw new IllegalArgumentException("URL supplied is not valid: " + url);
+    }
+
     // TODO: We need our own way to work with url macrosses
     private static boolean containsMacrosses(String url) {
         return StringUtils.contains(url, MACROS_OPEN) && StringUtils.contains(url, MACROS_CLOSE);

src/test/java/org/prebid/server/settings/HttpApplicationSettingsTest.java

@@ -283,14 +283,14 @@ public void getStoredDataShouldSendHttpRequestWithExpectedAppendedParams() {
         // given
         givenHttpClientReturnsResponse(200, null);
         httpApplicationSettings = new HttpApplicationSettings(httpClient, jacksonMapper,
-                "http://some-domain?param1=value1", AMP_ENDPOINT, VIDEO_ENDPOINT, CATEGORY_ENDPOINT, false);
+                "http://some-domain.com?param1=value1", AMP_ENDPOINT, VIDEO_ENDPOINT, CATEGORY_ENDPOINT, false);
 
         // when
         httpApplicationSettings.getStoredData(null, singleton("id1"), singleton("id2"), timeout);
 
         // then
         verify(httpClient).get(
-                eq("http://some-domain?param1=value1&request-ids=%5B%22id1%22%5D&imp-ids=%5B%22id2%22%5D"),
+                eq("http://some-domain.com?param1=value1&request-ids=%5B%22id1%22%5D&imp-ids=%5B%22id2%22%5D"),
                 any(),
                 anyLong());
     }