[JENKINS-51064] Lenient URL validator for Root URL (#3415)

* [JENKINS-51064] Lenient URL validator for Root URL - especially with the TLDs - more test cases * add blank line * Adjust comment * Correction typo
jenkinsci · May 4, 2018 · e7eec1c · steele · May 5, 2018 · oleg-nenashev
1 parent e229f37
commit e7eec1c
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 24 deletions.
diff --git a/core/src/main/java/jenkins/org/apache/commons/validator/routines/DomainValidator.java b/core/src/main/java/jenkins/org/apache/commons/validator/routines/DomainValidator.java
@@ -2002,6 +2002,9 @@ public static synchronized void updateTLDOverride(DomainValidator.ArrayType tabl
      * @return converted input, or original input if conversion fails
      */
     // Needed by UrlValidator
+    //[PATCH]
+    public
+    // end of [PATCH]
     static String unicodeToASCII(String input) {
         if (isOnlyASCII(input)) { // skip possibly expensive processing
             return input;

diff --git a/core/src/main/java/jenkins/util/UrlHelper.java b/core/src/main/java/jenkins/util/UrlHelper.java
@@ -23,36 +23,42 @@
  */
 package jenkins.util;
 
+import jenkins.org.apache.commons.validator.routines.DomainValidator;
 import jenkins.org.apache.commons.validator.routines.UrlValidator;
 import org.kohsuke.accmod.Restricted;
 import org.kohsuke.accmod.restrictions.NoExternalUse;
 
-import java.net.MalformedURLException;
-import java.net.URL;
-
 @Restricted(NoExternalUse.class)
 public class UrlHelper {
+    /**
+     * Authorize the {@code _} and {@code -} characters in domain
+     * <p>
+     * Avoid {@code -} and {@code .} to be first or last
+     */
+    private static String DOMAIN_REGEX = System.getProperty(
+            UrlHelper.class.getName() + ".DOMAIN_REGEX", 
+            "(\\w(\\.?-?\\w+)*)(:\\d{1,5})?"
+    );
+
     public static boolean isValidRootUrl(String url) {
-        String[] schemes = {"http", "https"};
-        // option to accept url like http://localhost or http://SERVER_JENKINS that are often used inside company's network
-        UrlValidator validator = new UrlValidator(schemes, UrlValidator.ALLOW_LOCAL_URLS + UrlValidator.NO_FRAGMENTS);
-        boolean isValid = validator.isValid(url);
-        if (!isValid) {
-            // potentially it contains _'s in hostname which seems accepted but not by the UrlValidator
-            // https://issues.apache.org/jira/browse/VALIDATOR-358
-            try {
-                URL urlObject = new URL(url);
-                String host = urlObject.getHost();
-                if (host.contains("_")) {
-                    String hostWithoutUnderscore = host.replace("_", "");
-                    String modifiedUrl = url.replace(host, hostWithoutUnderscore);
-                    isValid = validator.isValid(modifiedUrl);
-                }
-            } catch (MalformedURLException e) {
+        UrlValidator validator = new CustomUrlValidator();
+        return validator.isValid(url);
+    }
+
+    private static class CustomUrlValidator extends UrlValidator {
+        private CustomUrlValidator() {
+            super(new String[]{"http", "https"}, UrlValidator.ALLOW_LOCAL_URLS + UrlValidator.NO_FRAGMENTS);
+        }
+
+        @Override 
+        protected boolean isValidAuthority(String authority) {
+            // to support ipv6
+            boolean superResult = super.isValidAuthority(authority);
+            if(!superResult && authority == null){
                 return false;
             }
+            String authorityASCII = DomainValidator.unicodeToASCII(authority);
+            return authorityASCII.matches(DOMAIN_REGEX);
         }
-
-        return isValid;
     }
 }
diff --git a/core/src/test/java/jenkins/util/UrlHelperTest.java b/core/src/test/java/jenkins/util/UrlHelperTest.java
@@ -6,17 +6,19 @@
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
-@Issue("JENKINS-31661")
 public class UrlHelperTest {
 
     @Test
+    @Issue("JENKINS-31661")
     public void isValid() {
         assertTrue(UrlHelper.isValidRootUrl("http://www.google.com"));
+        assertTrue(UrlHelper.isValidRootUrl("http://www.google.com/"));
         assertTrue(UrlHelper.isValidRootUrl("http://www.google.com/jenkins"));
         assertTrue(UrlHelper.isValidRootUrl("http://www.google.com:8080"));
         assertTrue(UrlHelper.isValidRootUrl("http://www.google.com:8080/jenkins"));
         assertTrue(UrlHelper.isValidRootUrl("https://www.google.com:8080/jenkins"));
         assertTrue(UrlHelper.isValidRootUrl("http://localhost:8080/jenkins"));
+        assertTrue(UrlHelper.isValidRootUrl("http://localhost:8080/jenkins/"));
         assertTrue(UrlHelper.isValidRootUrl("http://my_server:8080/jenkins"));
         assertTrue(UrlHelper.isValidRootUrl("http://MY_SERVER_IN_PRIVATE_NETWORK:8080/jenkins"));
         assertTrue(UrlHelper.isValidRootUrl("http://jenkins"));
@@ -40,5 +42,24 @@ public void isValid() {
         // this url will be used as a root url and so will be concatenated with other part, fragments are not allowed
         assertFalse(UrlHelper.isValidRootUrl("http://jenkins#fragment"));
     }
-
-}
+
+    @Test
+    @Issue("JENKINS-51064")
+    public void isAlsoValid() {
+        assertTrue(UrlHelper.isValidRootUrl("http://my-server:8080/jenkins"));
+        assertTrue(UrlHelper.isValidRootUrl("http://jenkins.internal/"));
+        assertTrue(UrlHelper.isValidRootUrl("http://jenkins.otherDomain/"));
+        assertTrue(UrlHelper.isValidRootUrl("http://my-server.domain:8080/jenkins"));
+        assertTrue(UrlHelper.isValidRootUrl("http://my-ser_ver.do_m-ain:8080/jenkins"));
+        assertTrue(UrlHelper.isValidRootUrl("http://my-ser_ver.do_m-ain:8080/jenkins"));
+
+        // forbidden to start or end domain with - or .
+        assertFalse(UrlHelper.isValidRootUrl("http://-jenkins.com"));
+        assertFalse(UrlHelper.isValidRootUrl("http://jenkins.com-"));
+        assertFalse(UrlHelper.isValidRootUrl("http://.jenkins.com"));
+        assertFalse(UrlHelper.isValidRootUrl("http://jenkins.com."));
+
+        // forbidden to have multiple dots in chain
+        assertFalse(UrlHelper.isValidRootUrl("http://jen..kins.com"));
+    }
+}