[FIXED JENKINS-19493] Use form validation to alert users of invalid r…

…epository browser URLs before saving.
jenkinsci · Sep 8, 2013 · 776baa3 · 776baa3
1 parent 6a60763
commit 776baa3
Show file tree

Hide file tree

Showing 9 changed files with 105 additions and 32 deletions.
diff --git a/src/main/java/hudson/plugins/mercurial/browser/BitBucket.java b/src/main/java/hudson/plugins/mercurial/browser/BitBucket.java
@@ -1,9 +1,7 @@
 package hudson.plugins.mercurial.browser;
 
 import hudson.Extension;
-import hudson.model.Descriptor;
 import hudson.plugins.mercurial.MercurialChangeSet;
-import hudson.scm.RepositoryBrowser;
 import hudson.util.FormValidation;
 
 import java.io.IOException;
@@ -63,7 +61,7 @@ public URL getDiffLink(String path) throws MalformedURLException {
     }
 
     @Extension
-    public static class DescriptorImpl extends Descriptor<RepositoryBrowser<?>> {
+    public static class DescriptorImpl extends HgBrowserDescriptor {
         public String getDisplayName() {
             return "bitbucket";
         }
@@ -72,10 +70,12 @@ public String getDisplayName() {
             return req.bindParameters(BitBucket.class,"bitbucket.");
         }
 
-        public FormValidation doCheckUrl(@QueryParameter String url) {
-            if (url.length() == 0) {
-                return FormValidation.error("Must enter URL");
-            } else if (url.matches("https?://bitbucket[.]org/[^/]+/[^/]+/")) {
+        @Override public FormValidation doCheckUrl(@QueryParameter String url) {
+            return _doCheckUrl(url);
+        }
+
+        @Override protected FormValidation check(URL url) {
+            if (url.toString().matches("https?://bitbucket[.]org/[^/]+/[^/]+/")) {
                 return FormValidation.ok();
             } else {
                 return FormValidation.warning("Possibly incorrect root URL; expected http://bitbucket.org/USERNAME/REPOS/");

diff --git a/src/main/java/hudson/plugins/mercurial/browser/FishEye.java b/src/main/java/hudson/plugins/mercurial/browser/FishEye.java
@@ -1,9 +1,8 @@
 package hudson.plugins.mercurial.browser;
 
 import hudson.Extension;
-import hudson.model.Descriptor;
 import hudson.plugins.mercurial.MercurialChangeSet;
-import hudson.scm.RepositoryBrowser;
+import hudson.util.FormValidation;
 
 import java.io.IOException;
 import java.net.MalformedURLException;
@@ -12,6 +11,7 @@
 import net.sf.json.JSONObject;
 
 import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.QueryParameter;
 import org.kohsuke.stapler.StaplerRequest;
 
 /**
@@ -71,14 +71,19 @@ public URL getDiffLink(String path) throws MalformedURLException {
     }
 
     @Extension
-    public static class DescriptorImpl extends Descriptor<RepositoryBrowser<?>> {
+    public static class DescriptorImpl extends HgBrowserDescriptor {
         public String getDisplayName() {
             return "fisheye";
         }
 
         public @Override FishEye newInstance(StaplerRequest req, JSONObject json) throws FormException {
             return req.bindParameters(FishEye.class,"fisheye.");
         }
+
+        @Override public FormValidation doCheckUrl(@QueryParameter String url) {
+            return _doCheckUrl(url);
+        }
+
     }
 
     private static final long serialVersionUID = 1L;

diff --git a/src/main/java/hudson/plugins/mercurial/browser/GoogleCode.java b/src/main/java/hudson/plugins/mercurial/browser/GoogleCode.java
@@ -1,9 +1,8 @@
 package hudson.plugins.mercurial.browser;
 
 import hudson.Extension;
-import hudson.model.Descriptor;
 import hudson.plugins.mercurial.MercurialChangeSet;
-import hudson.scm.RepositoryBrowser;
+import hudson.util.FormValidation;
 
 import java.io.IOException;
 import java.io.UnsupportedEncodingException;
@@ -14,6 +13,7 @@
 import net.sf.json.JSONObject;
 
 import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.QueryParameter;
 import org.kohsuke.stapler.StaplerRequest;
 
 /**
@@ -72,14 +72,19 @@ public URL getDiffLink(String path) throws MalformedURLException {
     }
 
     @Extension
-    public static class DescriptorImpl extends Descriptor<RepositoryBrowser<?>> {
+    public static class DescriptorImpl extends HgBrowserDescriptor {
         public String getDisplayName() {
             return "googlecode";
         }
 
         public @Override GoogleCode newInstance(StaplerRequest req, JSONObject json) throws FormException {
             return req.bindParameters(GoogleCode.class,"googlecode.");
         }
+
+        @Override public FormValidation doCheckUrl(@QueryParameter String url) {
+            return _doCheckUrl(url);
+        }
+
     }
 
     private static final long serialVersionUID = 1L;

diff --git a/src/main/java/hudson/plugins/mercurial/browser/HgBrowser.java b/src/main/java/hudson/plugins/mercurial/browser/HgBrowser.java
@@ -1,18 +1,22 @@
 package hudson.plugins.mercurial.browser;
 
+import hudson.Util;
+import hudson.model.Descriptor;
 import hudson.plugins.mercurial.MercurialChangeSet;
 import hudson.scm.RepositoryBrowser;
+import hudson.util.FormValidation;
 
 import java.io.IOException;
 import java.net.MalformedURLException;
 import java.net.URL;
+import org.kohsuke.stapler.QueryParameter;
 
 /**
  * Parent class, as there is more than one browser.
  * {@link HgBrowser#readResolve()} will return the old default {@link HgWeb}.
  * Direct calls on this class will always throw {@link UnsupportedOperationException}s.
  */
-public class HgBrowser extends RepositoryBrowser<MercurialChangeSet> {
+public abstract class HgBrowser extends RepositoryBrowser<MercurialChangeSet> {
 
     transient MercurialChangeSet current;
 
@@ -84,4 +88,37 @@ public Object readResolve() {
             throw new RuntimeException(e);
         }
     }
+
+    protected static abstract class HgBrowserDescriptor extends Descriptor<RepositoryBrowser<?>> {
+
+        /**
+         * Stapler does not seem to (reliably) find the web method on subclasses, so each needs to implement this to call {@link #_doCheckUrl}.
+         * {@link #check} may be used for browser-specific checks.
+         */
+        public abstract FormValidation doCheckUrl(@QueryParameter String url);
+
+        protected final FormValidation _doCheckUrl(@QueryParameter String url) {
+            url = Util.fixNull(url);
+            if (url.length() == 0) {
+                return FormValidation.error("URL is mandatory");
+            }
+            try {
+                URL u = new URL(url);
+                if (u.getPath().endsWith("/")) {
+                    return check(u);
+                } else {
+                    return FormValidation.warning("Browser URL should end with a slash (/)");
+                }
+            } catch (MalformedURLException x) {
+                return FormValidation.error(x, "Invalid URL");
+            }
+        }
+
+        /** Perform optional additional checks on URL, such as that it uses a particular host. */
+        protected FormValidation check(URL url) {
+            return FormValidation.ok();
+        }
+
+    }
+
 }
diff --git a/src/main/java/hudson/plugins/mercurial/browser/HgWeb.java b/src/main/java/hudson/plugins/mercurial/browser/HgWeb.java
@@ -1,9 +1,8 @@
 package hudson.plugins.mercurial.browser;
 
 import hudson.Extension;
-import hudson.model.Descriptor;
 import hudson.plugins.mercurial.MercurialChangeSet;
-import hudson.scm.RepositoryBrowser;
+import hudson.util.FormValidation;
 
 import java.io.IOException;
 import java.net.MalformedURLException;
@@ -12,6 +11,7 @@
 import net.sf.json.JSONObject;
 
 import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.QueryParameter;
 import org.kohsuke.stapler.StaplerRequest;
 
 /**
@@ -63,14 +63,19 @@ public URL getDiffLink(String path) throws MalformedURLException {
     }
 
     @Extension
-    public static class DescriptorImpl extends Descriptor<RepositoryBrowser<?>> {
+    public static class DescriptorImpl extends HgBrowserDescriptor {
         public String getDisplayName() {
             return "hgweb";
         }
 
         public @Override HgWeb newInstance(StaplerRequest req, JSONObject json) throws FormException {
             return req.bindParameters(HgWeb.class,"hgweb.");
         }
+
+        @Override public FormValidation doCheckUrl(@QueryParameter String url) {
+            return _doCheckUrl(url);
+        }
+
     }
 
     private static final long serialVersionUID = 1L;

diff --git a/src/main/java/hudson/plugins/mercurial/browser/KilnHG.java b/src/main/java/hudson/plugins/mercurial/browser/KilnHG.java
@@ -1,9 +1,8 @@
 package hudson.plugins.mercurial.browser;
 
 import hudson.Extension;
-import hudson.model.Descriptor;
 import hudson.plugins.mercurial.MercurialChangeSet;
-import hudson.scm.RepositoryBrowser;
+import hudson.util.FormValidation;
 
 import java.io.IOException;
 import java.net.MalformedURLException;
@@ -12,6 +11,7 @@
 import net.sf.json.JSONObject;
 
 import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.QueryParameter;
 import org.kohsuke.stapler.StaplerRequest;
 
 /**
@@ -63,14 +63,19 @@ public URL getDiffLink(String path) throws MalformedURLException {
     }
 
     @Extension
-    public static class DescriptorImpl extends Descriptor<RepositoryBrowser<?>> {
+    public static class DescriptorImpl extends HgBrowserDescriptor {
         public String getDisplayName() {
             return "kilnhg";
         }
 
         public @Override KilnHG newInstance(StaplerRequest req, JSONObject json) throws FormException {
             return req.bindParameters(KilnHG.class,"kilnhg.");
         }
+
+        @Override public FormValidation doCheckUrl(@QueryParameter String url) {
+            return _doCheckUrl(url);
+        }
+
     }
 
     private static final long serialVersionUID = 1L;

diff --git a/src/main/java/hudson/plugins/mercurial/browser/RhodeCode.java b/src/main/java/hudson/plugins/mercurial/browser/RhodeCode.java
@@ -1,9 +1,8 @@
 package hudson.plugins.mercurial.browser;
 
 import hudson.Extension;
-import hudson.model.Descriptor;
 import hudson.plugins.mercurial.MercurialChangeSet;
-import hudson.scm.RepositoryBrowser;
+import hudson.util.FormValidation;
 
 import java.io.IOException;
 import java.net.MalformedURLException;
@@ -12,6 +11,7 @@
 import net.sf.json.JSONObject;
 
 import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.QueryParameter;
 import org.kohsuke.stapler.StaplerRequest;
 
 /**
@@ -70,7 +70,7 @@ public URL getDiffLink(String path) throws MalformedURLException {
     }
 
     @Extension
-    public static class DescriptorImpl extends Descriptor<RepositoryBrowser<?>> {
+    public static class DescriptorImpl extends HgBrowserDescriptor {
         public String getDisplayName() {
             return "rhodecode";
         }
@@ -80,6 +80,11 @@ RhodeCode newInstance(StaplerRequest req, JSONObject json)
                 throws FormException {
             return req.bindParameters(RhodeCode.class, "rhodecode.");
         }
+
+        @Override public FormValidation doCheckUrl(@QueryParameter String url) {
+            return _doCheckUrl(url);
+        }
+
     }
 
     private static final long serialVersionUID = 1L;

diff --git a/src/main/java/hudson/plugins/mercurial/browser/RhodeCodeLegacy.java b/src/main/java/hudson/plugins/mercurial/browser/RhodeCodeLegacy.java
@@ -1,15 +1,15 @@
 package hudson.plugins.mercurial.browser;
 
 import hudson.Extension;
-import hudson.model.Descriptor;
-import hudson.scm.RepositoryBrowser;
+import hudson.util.FormValidation;
 
 import java.net.MalformedURLException;
 import java.net.URL;
 
 import net.sf.json.JSONObject;
 
 import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.QueryParameter;
 import org.kohsuke.stapler.StaplerRequest;
 
 /**
@@ -46,7 +46,7 @@ public URL getDiffLink(String path) throws MalformedURLException {
     }
 
     @Extension
-    public static class DescriptorImpl extends Descriptor<RepositoryBrowser<?>> {
+    public static class DescriptorImpl extends HgBrowserDescriptor {
         public String getDisplayName() {
             return "rhodecode (pre-1.2.0)";
         }
@@ -57,6 +57,11 @@ RhodeCodeLegacy newInstance(StaplerRequest req, JSONObject json)
             return req.bindParameters(RhodeCodeLegacy.class,
                     "rhodecode-legacy.");
         }
+
+        @Override public FormValidation doCheckUrl(@QueryParameter String url) {
+            return _doCheckUrl(url);
+        }
+
     }
 
     private static final long serialVersionUID = 1L;

diff --git a/src/test/java/hudson/plugins/mercurial/browser/HgBrowserTest.java b/src/test/java/hudson/plugins/mercurial/browser/HgBrowserTest.java
@@ -11,28 +11,34 @@ public class HgBrowserTest {
 
     @Test(expected=UnsupportedOperationException.class)
     public final void testGetChangeSetLinkMercurialChangeSet() throws IOException {
-        new HgBrowser("http://abc/").getChangeSetLink(null);
+        new MockBrowser("http://abc/").getChangeSetLink(null);
     }
 
     @Test(expected=UnsupportedOperationException.class)
     public final void getFileLink() throws IOException {
-        new HgBrowser("http://abc/").getFileLink("");
+        new MockBrowser("http://abc/").getFileLink("");
     }
 
     @Test(expected=UnsupportedOperationException.class)
     public final void testGetDiffLink() throws IOException {
-        new HgBrowser("http://abc/").getDiffLink("");
+        new MockBrowser("http://abc/").getDiffLink("");
     }
 
     @Test
     public final void testGetUrl() throws MalformedURLException {
-        assertEquals("http://abc/", new HgBrowser("http://abc").getUrl().toExternalForm());
+        assertEquals("http://abc/", new MockBrowser("http://abc").getUrl().toExternalForm());
     }
 
     @Test
     public final void testResolveObject() throws MalformedURLException {
-        final Object browser = new HgBrowser("http://abc").readResolve();
-        assertEquals(HgWeb.class, browser.getClass());
+        final Object browser = new MockBrowser("http://abc").readResolve();
+        assertEquals(MockBrowser.class, browser.getClass());
+    }
+
+    public static final class MockBrowser extends HgBrowser {
+        public MockBrowser(String url) throws MalformedURLException {
+            super(url);
+        }
     }
 
 }