From 2510bd3797b2379dd679f033a94cc556abd96ddc Mon Sep 17 00:00:00 2001 From: Rishabh Budhouliya Date: Fri, 31 Jan 2020 01:54:20 +0530 Subject: [PATCH 01/18] =[JENKINS-56063] added expansion of env variables in refspec in case of honor refspec --- .../plugins/git/extensions/impl/CloneOption.java | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/main/java/hudson/plugins/git/extensions/impl/CloneOption.java b/src/main/java/hudson/plugins/git/extensions/impl/CloneOption.java index be46f4339b..ffd3036965 100644 --- a/src/main/java/hudson/plugins/git/extensions/impl/CloneOption.java +++ b/src/main/java/hudson/plugins/git/extensions/impl/CloneOption.java @@ -14,6 +14,7 @@ import hudson.plugins.git.util.GitUtils; import hudson.slaves.NodeProperty; import java.io.IOException; +import java.util.ArrayList; import java.util.List; import java.util.Objects; import org.eclipse.jgit.transport.RefSpec; @@ -148,7 +149,20 @@ public void decorateCloneCommand(GitSCM scm, Run build, GitClient git, Tas // in a single job definition. RemoteConfig rc = scm.getRepositories().get(0); List refspecs = rc.getFetchRefSpecs(); - cmd.refspecs(refspecs); + List expandedrefSpecs = new ArrayList<>(); + Boolean check = false; + for (RefSpec ref:refspecs) { + if(ref.toString().contains("$")){ + EnvVars env = build.getEnvironment(listener); + expandedrefSpecs.add(new RefSpec(env.expand(ref.toString()))); + check = true; + } + } + if(!check) { + cmd.refspecs(refspecs); + } else{ + cmd.refspecs(expandedrefSpecs); + } } cmd.timeout(timeout); From 7d7f67057c8c79c050d8fcab00535e091deec978 Mon Sep 17 00:00:00 2001 From: Rishabh Budhouliya Date: Fri, 31 Jan 2020 12:55:51 +0530 Subject: [PATCH 02/18] =removed the check flag and changed the implementation --- .../plugins/git/extensions/impl/CloneOption.java | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/src/main/java/hudson/plugins/git/extensions/impl/CloneOption.java b/src/main/java/hudson/plugins/git/extensions/impl/CloneOption.java index ffd3036965..eda5fa9785 100644 --- a/src/main/java/hudson/plugins/git/extensions/impl/CloneOption.java +++ b/src/main/java/hudson/plugins/git/extensions/impl/CloneOption.java @@ -149,20 +149,12 @@ public void decorateCloneCommand(GitSCM scm, Run build, GitClient git, Tas // in a single job definition. RemoteConfig rc = scm.getRepositories().get(0); List refspecs = rc.getFetchRefSpecs(); - List expandedrefSpecs = new ArrayList<>(); - Boolean check = false; + List expandedRefSpecs = new ArrayList<>(); + EnvVars env = build.getEnvironment(listener); for (RefSpec ref:refspecs) { - if(ref.toString().contains("$")){ - EnvVars env = build.getEnvironment(listener); - expandedrefSpecs.add(new RefSpec(env.expand(ref.toString()))); - check = true; - } - } - if(!check) { - cmd.refspecs(refspecs); - } else{ - cmd.refspecs(expandedrefSpecs); + expandedRefSpecs.add(new RefSpec(env.expand(ref.toString()))); } + cmd.refspecs(expandedRefSpecs); } cmd.timeout(timeout); From 58320c4ee6a1db6b432c09511fcfa67c78889560 Mon Sep 17 00:00:00 2001 From: rishabhBudhouliya <31189405+rishabhBudhouliya@users.noreply.github.com> Date: Sat, 1 Feb 2020 12:28:40 +0530 Subject: [PATCH 03/18] Changed minor indentation issues --- .../java/hudson/plugins/git/extensions/impl/CloneOption.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/hudson/plugins/git/extensions/impl/CloneOption.java b/src/main/java/hudson/plugins/git/extensions/impl/CloneOption.java index eda5fa9785..fb1bdf6933 100644 --- a/src/main/java/hudson/plugins/git/extensions/impl/CloneOption.java +++ b/src/main/java/hudson/plugins/git/extensions/impl/CloneOption.java @@ -151,10 +151,10 @@ public void decorateCloneCommand(GitSCM scm, Run build, GitClient git, Tas List refspecs = rc.getFetchRefSpecs(); List expandedRefSpecs = new ArrayList<>(); EnvVars env = build.getEnvironment(listener); - for (RefSpec ref:refspecs) { + for (RefSpec ref : refspecs) { expandedRefSpecs.add(new RefSpec(env.expand(ref.toString()))); } - cmd.refspecs(expandedRefSpecs); + cmd.refspecs(expandedRefSpecs); } cmd.timeout(timeout); From 503e0a3d9fb0053f3b6b3104b75cc3237eed7a44 Mon Sep 17 00:00:00 2001 From: Rishabh Budhouliya Date: Thu, 20 Feb 2020 01:09:37 +0530 Subject: [PATCH 04/18] [JENKINS-48625] Restore binding of doCheckUrl methods and add some initial checks --- pom.xml | 11 +++ .../plugins/git/browser/AssemblaWeb.java | 72 +++++++++++----- .../git/browser/GitBlitRepositoryBrowser.java | 74 ++++++++++++----- .../hudson/plugins/git/browser/Gitiles.java | 73 +++++++++++----- .../plugins/git/browser/ViewGitWeb.java | 83 +++++++++++++------ .../hudson/plugins/git/Messages.properties | 1 + .../browser/AssemblaWebDoCheckURLTest.java | 63 ++++++++++++++ 7 files changed, 284 insertions(+), 93 deletions(-) create mode 100644 src/test/java/hudson/plugins/git/browser/AssemblaWebDoCheckURLTest.java diff --git a/pom.xml b/pom.xml index 5f2b8132e7..535a7575fe 100644 --- a/pom.xml +++ b/pom.xml @@ -255,6 +255,17 @@ + + commons-validator + commons-validator + 1.6 + + + commons-digester + commons-digester + + + diff --git a/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java b/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java index a6627fae84..8bcc70621c 100644 --- a/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java +++ b/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java @@ -1,15 +1,19 @@ package hudson.plugins.git.browser; import hudson.Extension; +import hudson.Util; import hudson.model.Descriptor; +import hudson.model.Item; import hudson.plugins.git.GitChangeSet; import hudson.plugins.git.GitChangeSet.Path; +import hudson.plugins.git.Messages; import hudson.scm.EditType; import hudson.scm.RepositoryBrowser; import hudson.util.FormValidation; import hudson.util.FormValidation.URLCheck; -import jenkins.model.Jenkins; +import org.apache.commons.validator.routines.UrlValidator; import net.sf.json.JSONObject; +import org.kohsuke.stapler.AncestorInPath; import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.interceptor.RequirePOST; import org.kohsuke.stapler.QueryParameter; @@ -18,6 +22,8 @@ import javax.annotation.Nonnull; import javax.servlet.ServletException; import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; import java.net.URL; /** @@ -94,33 +100,55 @@ public AssemblaWeb newInstance(StaplerRequest req, @Nonnull JSONObject jsonObjec } @RequirePOST - public FormValidation doCheckUrl(@QueryParameter(fixEmpty = true) final String url) - throws IOException, ServletException { - if (url == null) // nothing entered yet - { + public FormValidation doCheckRepoUrl(@AncestorInPath Item project, @QueryParameter(fixEmpty = true) final String repoUrl) + throws IOException, ServletException, URISyntaxException { + + String cleanUrl = Util.fixEmptyAndTrim(repoUrl); + + if (cleanUrl == null) { return FormValidation.ok(); } - // Connect to URL and check content only if we have admin permission - if (!Jenkins.get().hasPermission(Jenkins.ADMINISTER)) + + if (project == null || !project.hasPermission(Item.CONFIGURE)) { return FormValidation.ok(); - return new URLCheck() { - protected FormValidation check() throws IOException, ServletException { - String v = url; - if (!v.endsWith("/")) { - v += '/'; - } + } + + if (cleanUrl.contains("$")) { + // set by variable, can't validate + return FormValidation.ok(); + } + FormValidation response; + if (checkURIFormat(cleanUrl)) { + return new URLCheck() { + protected FormValidation check() throws IOException, ServletException { + String v = cleanUrl; + if (!v.endsWith("/")) { + v += '/'; + } - try { - if (findText(open(new URL(v)), "Assembla")) { - return FormValidation.ok(); - } else { - return FormValidation.error("This is a valid URL but it doesn't look like Assembla"); + try { + if (findText(open(new URL(v)), "Assembla")) { + return FormValidation.ok(); + } else { + return FormValidation.error("This is a valid URL but it doesn't look like Assembla"); + } + } catch (IOException e) { + return handleIOException(v, e); } - } catch (IOException e) { - return handleIOException(v, e); } - } - }.check(); + }.check(); + } else { + response = FormValidation.error(Messages.invalidUrl()); + } + return response; + } + + private boolean checkURIFormat(String url) throws URISyntaxException { + //https://app.assembla.com/spaces/git-plugin/git/source + URI uri = new URI(url); + String[] schemes = {"http", "https"}; + UrlValidator urlValidator = new UrlValidator(schemes); + return urlValidator.isValid(uri.toString()) && uri.getHost().contains("assembla"); } } } diff --git a/src/main/java/hudson/plugins/git/browser/GitBlitRepositoryBrowser.java b/src/main/java/hudson/plugins/git/browser/GitBlitRepositoryBrowser.java index bfd8c4331b..2159b560f8 100644 --- a/src/main/java/hudson/plugins/git/browser/GitBlitRepositoryBrowser.java +++ b/src/main/java/hudson/plugins/git/browser/GitBlitRepositoryBrowser.java @@ -1,15 +1,19 @@ package hudson.plugins.git.browser; import hudson.Extension; +import hudson.Util; import hudson.model.Descriptor; +import hudson.model.Item; import hudson.plugins.git.GitChangeSet; import hudson.plugins.git.GitChangeSet.Path; +import hudson.plugins.git.Messages; import hudson.scm.EditType; import hudson.scm.RepositoryBrowser; import hudson.util.FormValidation; import hudson.util.FormValidation.URLCheck; -import jenkins.model.Jenkins; import net.sf.json.JSONObject; +import org.apache.commons.validator.routines.UrlValidator; +import org.kohsuke.stapler.AncestorInPath; import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.interceptor.RequirePOST; import org.kohsuke.stapler.QueryParameter; @@ -19,6 +23,8 @@ import javax.servlet.ServletException; import java.io.IOException; import java.io.UnsupportedEncodingException; +import java.net.URI; +import java.net.URISyntaxException; import java.net.URL; import java.net.URLEncoder; @@ -63,9 +69,10 @@ public String getProjectName() { return projectName; } - private String encodeString(final String s) throws UnsupportedEncodingException { + private String encodeString(final String s) throws UnsupportedEncodingException { return URLEncoder.encode(s, "UTF-8").replaceAll("\\+", "%20"); } + @Extension public static class ViewGitWebDescriptor extends Descriptor> { @Nonnull @@ -80,33 +87,54 @@ public GitBlitRepositoryBrowser newInstance(StaplerRequest req, @Nonnull JSONObj } @RequirePOST - public FormValidation doCheckUrl(@QueryParameter(fixEmpty = true) final String url) - throws IOException, ServletException { - if (url == null) // nothing entered yet - { + public FormValidation doCheckRepoUrl(@AncestorInPath Item project, @QueryParameter(fixEmpty = true) final String repoUrl) + throws IOException, ServletException, URISyntaxException { + + String cleanUrl = Util.fixEmptyAndTrim(repoUrl); + + if (cleanUrl == null) { return FormValidation.ok(); } - // Connect to URL and check content only if we have admin permission - if (!Jenkins.get().hasPermission(Jenkins.ADMINISTER)) + + if (project == null || !project.hasPermission(Item.CONFIGURE)) { return FormValidation.ok(); - return new URLCheck() { - protected FormValidation check() throws IOException, ServletException { - String v = url; - if (!v.endsWith("/")) { - v += '/'; - } + } - try { - if (findText(open(new URL(v)), "Gitblit")) { - return FormValidation.ok(); - } else { - return FormValidation.error("This is a valid URL but it doesn't look like Gitblit"); + if (cleanUrl.contains("$")) { + // set by variable, can't validate + return FormValidation.ok(); + } + FormValidation response; + if (checkURIFormat(cleanUrl)) { + return new URLCheck() { + protected FormValidation check() throws IOException, ServletException { + String v = cleanUrl; + if (!v.endsWith("/")) { + v += '/'; + } + + try { + if (findText(open(new URL(v)), "Gitblit")) { + return FormValidation.ok(); + } else { + return FormValidation.error("This is a valid URL but it doesn't look like Gitblit"); + } + } catch (IOException e) { + return handleIOException(v, e); } - } catch (IOException e) { - return handleIOException(v, e); } - } - }.check(); + }.check(); + } else { + response = FormValidation.error(Messages.invalidUrl()); + } + return response; + } + + private boolean checkURIFormat(String url) throws URISyntaxException { + URI uri = new URI(url); + String[] schemes = {"http", "https"}; + UrlValidator urlValidator = new UrlValidator(schemes); + return urlValidator.isValid(uri.toString()) && uri.getHost().contains("gitblit"); } } } diff --git a/src/main/java/hudson/plugins/git/browser/Gitiles.java b/src/main/java/hudson/plugins/git/browser/Gitiles.java index 9768e4b1f2..d0cf57037e 100644 --- a/src/main/java/hudson/plugins/git/browser/Gitiles.java +++ b/src/main/java/hudson/plugins/git/browser/Gitiles.java @@ -1,16 +1,19 @@ package hudson.plugins.git.browser; import hudson.Extension; +import hudson.Util; import hudson.model.Descriptor; +import hudson.model.Item; import hudson.plugins.git.GitChangeSet; import hudson.plugins.git.GitChangeSet.Path; +import hudson.plugins.git.Messages; import hudson.scm.RepositoryBrowser; import hudson.util.FormValidation; import hudson.util.FormValidation.URLCheck; -import jenkins.model.Jenkins; - import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; import java.net.URL; import javax.annotation.Nonnull; @@ -18,6 +21,8 @@ import net.sf.json.JSONObject; +import org.apache.commons.validator.routines.UrlValidator; +import org.kohsuke.stapler.AncestorInPath; import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.interceptor.RequirePOST; import org.kohsuke.stapler.QueryParameter; @@ -70,30 +75,54 @@ public Gitiles newInstance(StaplerRequest req, @Nonnull JSONObject jsonObject) t } @RequirePOST - public FormValidation doCheckUrl(@QueryParameter(fixEmpty = true) final String url) throws IOException, ServletException { - if (url == null) // nothing entered yet + public FormValidation doCheckRepoUrl(@AncestorInPath Item project, @QueryParameter(fixEmpty = true) final String repoUrl) + throws IOException, ServletException, URISyntaxException { + + String cleanUrl = Util.fixEmptyAndTrim(repoUrl); + + if (cleanUrl == null) { + return FormValidation.ok(); + } + + if (project == null || !project.hasPermission(Item.CONFIGURE)) { return FormValidation.ok(); - // Connect to URL and check content only if we have admin permission - if (!Jenkins.get().hasPermission(Jenkins.ADMINISTER)) + } + + if (cleanUrl.contains("$")) { + // set by variable, can't validate return FormValidation.ok(); - return new URLCheck() { - protected FormValidation check() throws IOException, ServletException { - String v = url; - if (!v.endsWith("/")) - v += '/'; - - try { - // gitiles has a line in main page indicating how to clone the project - if (findText(open(new URL(v)), "git clone")) { - return FormValidation.ok(); - } else { - return FormValidation.error("This is a valid URL but it doesn't look like Gitiles"); + } + FormValidation response; + if (checkURIFormat(cleanUrl)) { + return new URLCheck() { + protected FormValidation check() throws IOException, ServletException { + String v = cleanUrl; + if (!v.endsWith("/")) { + v += '/'; + } + + try { + if (findText(open(new URL(v)), "git clone")) { + return FormValidation.ok(); + } else { + return FormValidation.error("This is a valid URL but it doesn't look like Gitiles"); + } + } catch (IOException e) { + return handleIOException(v, e); } - } catch (IOException e) { - return handleIOException(v, e); } - } - }.check(); + }.check(); + } else { + response = FormValidation.error(Messages.invalidUrl()); + } + return response; + } + + private boolean checkURIFormat(String url) throws URISyntaxException { + URI uri = new URI(url); + String[] schemes = {"http", "https"}; + UrlValidator urlValidator = new UrlValidator(schemes); + return urlValidator.isValid(uri.toString()) && uri.getHost().contains("gerrit"); } } } diff --git a/src/main/java/hudson/plugins/git/browser/ViewGitWeb.java b/src/main/java/hudson/plugins/git/browser/ViewGitWeb.java index ad3c3cafa3..baa38c530c 100644 --- a/src/main/java/hudson/plugins/git/browser/ViewGitWeb.java +++ b/src/main/java/hudson/plugins/git/browser/ViewGitWeb.java @@ -1,16 +1,20 @@ package hudson.plugins.git.browser; import hudson.Extension; +import hudson.Util; import hudson.model.Descriptor; +import hudson.model.Item; import hudson.plugins.git.GitChangeSet; import hudson.plugins.git.GitChangeSet.Path; +import hudson.plugins.git.Messages; import hudson.scm.EditType; import hudson.scm.RepositoryBrowser; import hudson.scm.browsers.QueryBuilder; import hudson.util.FormValidation; import hudson.util.FormValidation.URLCheck; -import jenkins.model.Jenkins; import net.sf.json.JSONObject; +import org.apache.commons.validator.routines.UrlValidator; +import org.kohsuke.stapler.AncestorInPath; import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.interceptor.RequirePOST; import org.kohsuke.stapler.QueryParameter; @@ -20,6 +24,8 @@ import javax.servlet.ServletException; import java.io.IOException; import java.io.UnsupportedEncodingException; +import java.net.URI; +import java.net.URISyntaxException; import java.net.URL; import java.net.URLEncoder; @@ -40,7 +46,7 @@ public URL getDiffLink(Path path) throws IOException { if (path.getEditType() == EditType.EDIT) { URL url = getUrl(); String spec = buildCommitDiffSpec(url, path); - return new URL(url, url.getPath() + spec); + return new URL(url, url.getPath() + spec); } return null; } @@ -52,14 +58,14 @@ public URL getFileLink(Path path) throws IOException { String spec = buildCommitDiffSpec(url, path); return encodeURL(new URL(url, url.getPath() + spec)); } - String spec = param(url).add("p=" + projectName).add("a=viewblob").add("h=" + path.getDst()).add("f=" + path.getPath()).toString(); + String spec = param(url).add("p=" + projectName).add("a=viewblob").add("h=" + path.getDst()).add("f=" + path.getPath()).toString(); return encodeURL(new URL(url, url.getPath() + spec)); } - private String buildCommitDiffSpec(URL url, Path path) - throws UnsupportedEncodingException { - return param(url).add("p=" + projectName).add("a=commitdiff").add("h=" + path.getChangeSet().getId()) + "#" + URLEncoder.encode(path.getPath(),"UTF-8").toString(); - } + private String buildCommitDiffSpec(URL url, Path path) + throws UnsupportedEncodingException { + return param(url).add("p=" + projectName).add("a=commitdiff").add("h=" + path.getChangeSet().getId()) + "#" + URLEncoder.encode(path.getPath(), "UTF-8").toString(); + } @Override public URL getChangeSetLink(GitChangeSet changeSet) throws IOException { @@ -89,29 +95,54 @@ public ViewGitWeb newInstance(StaplerRequest req, @Nonnull JSONObject jsonObject } @RequirePOST - public FormValidation doCheckUrl(@QueryParameter(fixEmpty = true) final String url) throws IOException, ServletException { - if (url == null) // nothing entered yet + public FormValidation doCheckRepoUrl(@AncestorInPath Item project, @QueryParameter(fixEmpty = true) final String repoUrl) + throws IOException, ServletException, URISyntaxException { + + String cleanUrl = Util.fixEmptyAndTrim(repoUrl); + + if (cleanUrl == null) { + return FormValidation.ok(); + } + + if (project == null || !project.hasPermission(Item.CONFIGURE)) { return FormValidation.ok(); - // Connect to URL and check content only if we have admin permission - if (!Jenkins.get().hasPermission(Jenkins.ADMINISTER)) + } + + if (cleanUrl.contains("$")) { + // set by variable, can't validate return FormValidation.ok(); - return new URLCheck() { - protected FormValidation check() throws IOException, ServletException { - String v = url; - if (!v.endsWith("/")) - v += '/'; - - try { - if (findText(open(new URL(v)), "ViewGit")) { - return FormValidation.ok(); - } else { - return FormValidation.error("This is a valid URL but it doesn't look like ViewGit"); + } + FormValidation response; + if (checkURIFormat(cleanUrl)) { + return new URLCheck() { + protected FormValidation check() throws IOException, ServletException { + String v = cleanUrl; + if (!v.endsWith("/")) { + v += '/'; + } + + try { + if (findText(open(new URL(v)), "ViewGit")) { + return FormValidation.ok(); + } else { + return FormValidation.error("This is a valid URL but it doesn't look like ViewGit"); + } + } catch (IOException e) { + return handleIOException(v, e); } - } catch (IOException e) { - return handleIOException(v, e); } - } - }.check(); + }.check(); + } else { + response = FormValidation.error(Messages.invalidUrl()); + } + return response; + } + + private boolean checkURIFormat(String url) throws URISyntaxException { + URI uri = new URI(url); + String[] schemes = {"http", "https"}; + UrlValidator urlValidator = new UrlValidator(schemes); + return urlValidator.isValid(uri.toString()) && uri.getHost().contains("viewgit"); } } } diff --git a/src/main/resources/hudson/plugins/git/Messages.properties b/src/main/resources/hudson/plugins/git/Messages.properties index 11401f4bd1..f64069d344 100644 --- a/src/main/resources/hudson/plugins/git/Messages.properties +++ b/src/main/resources/hudson/plugins/git/Messages.properties @@ -6,6 +6,7 @@ BuildChooser_BuildingLastRevision=No new revisions were found; the most-recently UserRemoteConfig.FailedToConnect=Failed to connect to repository : {0} UserRemoteConfig.CheckUrl.UrlIsNull=Please enter Git repository. UserRemoteConfig.CheckRefSpec.InvalidRefSpec=Specification is invalid. +invalidUrl=Invalid URL GitPublisher.Check.TagName=Tag Name GitPublisher.Check.BranchName=Branch Name diff --git a/src/test/java/hudson/plugins/git/browser/AssemblaWebDoCheckURLTest.java b/src/test/java/hudson/plugins/git/browser/AssemblaWebDoCheckURLTest.java new file mode 100644 index 0000000000..a7702d0e9c --- /dev/null +++ b/src/test/java/hudson/plugins/git/browser/AssemblaWebDoCheckURLTest.java @@ -0,0 +1,63 @@ +package hudson.plugins.git.browser; + +import hudson.model.FreeStyleProject; +import hudson.util.FormValidation; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.jvnet.hudson.test.JenkinsRule; + + +import static org.junit.Assert.assertEquals; + +public class AssemblaWebDoCheckURLTest { + + @Rule + public JenkinsRule r = new JenkinsRule(); + + private FreeStyleProject project; + private AssemblaWeb.AssemblaWebDescriptor assemblaWebDescriptor; + + @Before + public void setProject() throws Exception { + project = r.createFreeStyleProject("p1"); + assemblaWebDescriptor = new AssemblaWeb.AssemblaWebDescriptor(); + } + + @Test + public void testInitialChecksOnRepoUrl() throws Exception { + String url = "https://app.assembla.com/spaces/git-plugin/git/source"; + // Empty url + String url2 = ""; + // URL with env variable + String url3 = "https://www.assembla.com/spaces/$"; + + assertEquals(FormValidation.ok(), assemblaWebDescriptor.doCheckRepoUrl(project, url)); + assertEquals(FormValidation.ok(), assemblaWebDescriptor.doCheckRepoUrl(project, url2)); + assertEquals(FormValidation.ok(), assemblaWebDescriptor.doCheckRepoUrl(project, url3)); + } + + @Test + public void testDomainLevelChecksOnRepoUrl() throws Exception { + // illegal syntax - Earlier it would open connection for such mistakes but now check resolves it beforehand. + String url = "https:/assembla.com"; + String url2 = "http//assmebla"; + + assertEquals("Invalid URL", assemblaWebDescriptor.doCheckRepoUrl(project, url).getLocalizedMessage()); + assertEquals("Invalid URL", assemblaWebDescriptor.doCheckRepoUrl(project, url2).getLocalizedMessage()); + } + + @Test + public void testPathLevelChecksOnRepoUrl() throws Exception { + // Invalid path syntax + String url = "https://assembla.comspaces/git-plugin/git/source"; + // Syntax issue related specific to Assembla + String url2 = "https://app.assembla.com/space/git-plugin/git/source"; + // Any path related errors will not be caught except syntax issues + String url3 = "https://app.assembla.com/spaces"; + + assertEquals("Invalid URL", assemblaWebDescriptor.doCheckRepoUrl(project, url).getLocalizedMessage()); + assertEquals("Unable to connect https://app.assembla.com/space/git-plugin/git/source/", assemblaWebDescriptor.doCheckRepoUrl(project, url2).getLocalizedMessage()); + assertEquals(FormValidation.ok(), assemblaWebDescriptor.doCheckRepoUrl(project, url3)); + } +} From f99b832e5a848714b5fdfc2a3881605c2c0163ba Mon Sep 17 00:00:00 2001 From: Mark Waite Date: Wed, 19 Feb 2020 21:55:04 -0700 Subject: [PATCH 05/18] Proposed AssemblaWebTest changes --- .../plugins/git/browser/AssemblaWeb.java | 2 +- .../browser/AssemblaWebDoCheckURLTest.java | 76 +++++++++++++------ 2 files changed, 55 insertions(+), 23 deletions(-) diff --git a/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java b/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java index 8bcc70621c..5d3a3e22bd 100644 --- a/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java +++ b/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java @@ -130,7 +130,7 @@ protected FormValidation check() throws IOException, ServletException { if (findText(open(new URL(v)), "Assembla")) { return FormValidation.ok(); } else { - return FormValidation.error("This is a valid URL but it doesn't look like Assembla"); + return FormValidation.error("This is a valid URL but it does not look like Assembla"); } } catch (IOException e) { return handleIOException(v, e); diff --git a/src/test/java/hudson/plugins/git/browser/AssemblaWebDoCheckURLTest.java b/src/test/java/hudson/plugins/git/browser/AssemblaWebDoCheckURLTest.java index a7702d0e9c..39760276d1 100644 --- a/src/test/java/hudson/plugins/git/browser/AssemblaWebDoCheckURLTest.java +++ b/src/test/java/hudson/plugins/git/browser/AssemblaWebDoCheckURLTest.java @@ -3,61 +3,93 @@ import hudson.model.FreeStyleProject; import hudson.util.FormValidation; import org.junit.Before; -import org.junit.Rule; +import org.junit.ClassRule; import org.junit.Test; import org.jvnet.hudson.test.JenkinsRule; - -import static org.junit.Assert.assertEquals; +import static org.hamcrest.MatcherAssert.*; +import static org.hamcrest.Matchers.*; public class AssemblaWebDoCheckURLTest { - @Rule - public JenkinsRule r = new JenkinsRule(); + @ClassRule + public static JenkinsRule r = new JenkinsRule(); + private static int counter = 0; private FreeStyleProject project; private AssemblaWeb.AssemblaWebDescriptor assemblaWebDescriptor; @Before public void setProject() throws Exception { - project = r.createFreeStyleProject("p1"); + project = r.createFreeStyleProject("assembla-project-" + counter++); assemblaWebDescriptor = new AssemblaWeb.AssemblaWebDescriptor(); } @Test public void testInitialChecksOnRepoUrl() throws Exception { String url = "https://app.assembla.com/spaces/git-plugin/git/source"; - // Empty url - String url2 = ""; - // URL with env variable - String url3 = "https://www.assembla.com/spaces/$"; + assertThat(assemblaWebDescriptor.doCheckRepoUrl(project, url), is(FormValidation.ok())); + } + + @Test + public void testInitialChecksOnRepoUrlEmpty() throws Exception { + String url = ""; + assertThat(assemblaWebDescriptor.doCheckRepoUrl(project, url), is(FormValidation.ok())); + } - assertEquals(FormValidation.ok(), assemblaWebDescriptor.doCheckRepoUrl(project, url)); - assertEquals(FormValidation.ok(), assemblaWebDescriptor.doCheckRepoUrl(project, url2)); - assertEquals(FormValidation.ok(), assemblaWebDescriptor.doCheckRepoUrl(project, url3)); + @Test + public void testInitialChecksOnRepoUrlWithVariable() throws Exception { + String url = "https://www.assembla.com/spaces/$"; + assertThat(assemblaWebDescriptor.doCheckRepoUrl(project, url), is(FormValidation.ok())); } @Test public void testDomainLevelChecksOnRepoUrl() throws Exception { // illegal syntax - Earlier it would open connection for such mistakes but now check resolves it beforehand. String url = "https:/assembla.com"; - String url2 = "http//assmebla"; + assertThat(assemblaWebDescriptor.doCheckRepoUrl(project, url).getLocalizedMessage(), is("Invalid URL")); + } - assertEquals("Invalid URL", assemblaWebDescriptor.doCheckRepoUrl(project, url).getLocalizedMessage()); - assertEquals("Invalid URL", assemblaWebDescriptor.doCheckRepoUrl(project, url2).getLocalizedMessage()); + @Test + public void testDomainLevelChecksOnRepoUrlInvalidURL() throws Exception { + // illegal syntax - Earlier it would open connection for such mistakes but now check resolves it beforehand. + String url = "http//assmebla"; + assertThat(assemblaWebDescriptor.doCheckRepoUrl(project, url).getLocalizedMessage(), is("Invalid URL")); } @Test - public void testPathLevelChecksOnRepoUrl() throws Exception { + public void testPathLevelChecksOnRepoUrlInvalidPathSyntax() throws Exception { // Invalid path syntax String url = "https://assembla.comspaces/git-plugin/git/source"; + assertThat(assemblaWebDescriptor.doCheckRepoUrl(project, url).getLocalizedMessage(), is("Invalid URL")); + } + + @Test + public void testPathLevelChecksOnRepoUrlValidURL() throws Exception { + // Syntax issue related specific to Assembla + String url = "https://app.assembla.com/space/git-plugin/git/source"; + assertThat(assemblaWebDescriptor.doCheckRepoUrl(null, url), is(FormValidation.ok())); + } + + @Test + public void testPathLevelChecksOnRepoUrlUnableToConnect() throws Exception { // Syntax issue related specific to Assembla - String url2 = "https://app.assembla.com/space/git-plugin/git/source"; + String url = "https://app.assembla.com/space/git-plugin/git/source"; + assertThat(assemblaWebDescriptor.doCheckRepoUrl(project, url).getLocalizedMessage(), + is("Unable to connect https://app.assembla.com/space/git-plugin/git/source/")); + } + + @Test + public void testPathLevelChecksOnRepoUrl() throws Exception { // Any path related errors will not be caught except syntax issues - String url3 = "https://app.assembla.com/spaces"; + String url = "https://app.assembla.com/spaces/"; + assertThat(assemblaWebDescriptor.doCheckRepoUrl(project, url), is(FormValidation.ok())); + } - assertEquals("Invalid URL", assemblaWebDescriptor.doCheckRepoUrl(project, url).getLocalizedMessage()); - assertEquals("Unable to connect https://app.assembla.com/space/git-plugin/git/source/", assemblaWebDescriptor.doCheckRepoUrl(project, url2).getLocalizedMessage()); - assertEquals(FormValidation.ok(), assemblaWebDescriptor.doCheckRepoUrl(project, url3)); + @Test + public void testPathLevelChecksOnRepoUrlSupersetOfAssembla() throws Exception { + String url = "http://assemblagist.com/"; + assertThat(assemblaWebDescriptor.doCheckRepoUrl(project, url).getLocalizedMessage(), + is("This is a valid URL but it does not look like Assembla")); } } From a062e0bd38c3c5f04233c2e17767ee29626c930a Mon Sep 17 00:00:00 2001 From: Mark Waite Date: Thu, 20 Feb 2020 08:48:50 -0700 Subject: [PATCH 06/18] Clarify comments for my benefit I had to read very carefully to detect the syntactic errors in the URL. The comment helps me see more clearly. --- .../plugins/git/browser/AssemblaWebDoCheckURLTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/java/hudson/plugins/git/browser/AssemblaWebDoCheckURLTest.java b/src/test/java/hudson/plugins/git/browser/AssemblaWebDoCheckURLTest.java index 39760276d1..4b4c970794 100644 --- a/src/test/java/hudson/plugins/git/browser/AssemblaWebDoCheckURLTest.java +++ b/src/test/java/hudson/plugins/git/browser/AssemblaWebDoCheckURLTest.java @@ -45,21 +45,21 @@ public void testInitialChecksOnRepoUrlWithVariable() throws Exception { @Test public void testDomainLevelChecksOnRepoUrl() throws Exception { - // illegal syntax - Earlier it would open connection for such mistakes but now check resolves it beforehand. + // Invalid URL, missing '/' character - Earlier it would open connection for such mistakes but now check resolves it beforehand. String url = "https:/assembla.com"; assertThat(assemblaWebDescriptor.doCheckRepoUrl(project, url).getLocalizedMessage(), is("Invalid URL")); } @Test public void testDomainLevelChecksOnRepoUrlInvalidURL() throws Exception { - // illegal syntax - Earlier it would open connection for such mistakes but now check resolves it beforehand. + // Invalid URL, missing ':' character - Earlier it would open connection for such mistakes but now check resolves it beforehand. String url = "http//assmebla"; assertThat(assemblaWebDescriptor.doCheckRepoUrl(project, url).getLocalizedMessage(), is("Invalid URL")); } @Test public void testPathLevelChecksOnRepoUrlInvalidPathSyntax() throws Exception { - // Invalid path syntax + // Invalid hostname in URL String url = "https://assembla.comspaces/git-plugin/git/source"; assertThat(assemblaWebDescriptor.doCheckRepoUrl(project, url).getLocalizedMessage(), is("Invalid URL")); } From c04b4a7289c414ee150621bd4f1fb45aec7193b0 Mon Sep 17 00:00:00 2001 From: Mark Waite Date: Thu, 20 Feb 2020 08:49:39 -0700 Subject: [PATCH 07/18] Extend null project test name to refer to null project --- .../hudson/plugins/git/browser/AssemblaWebDoCheckURLTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/test/java/hudson/plugins/git/browser/AssemblaWebDoCheckURLTest.java b/src/test/java/hudson/plugins/git/browser/AssemblaWebDoCheckURLTest.java index 4b4c970794..e30d4b5a19 100644 --- a/src/test/java/hudson/plugins/git/browser/AssemblaWebDoCheckURLTest.java +++ b/src/test/java/hudson/plugins/git/browser/AssemblaWebDoCheckURLTest.java @@ -65,8 +65,7 @@ public void testPathLevelChecksOnRepoUrlInvalidPathSyntax() throws Exception { } @Test - public void testPathLevelChecksOnRepoUrlValidURL() throws Exception { - // Syntax issue related specific to Assembla + public void testPathLevelChecksOnRepoUrlValidURLNullProject() throws Exception { String url = "https://app.assembla.com/space/git-plugin/git/source"; assertThat(assemblaWebDescriptor.doCheckRepoUrl(null, url), is(FormValidation.ok())); } From 3a8d54917a609a9e8238c18fb5bcc131890187f2 Mon Sep 17 00:00:00 2001 From: Mark Waite Date: Thu, 20 Feb 2020 08:50:10 -0700 Subject: [PATCH 08/18] Test with a random URL that meets criteria Avoid undue load on any one server for this test. --- .../git/browser/AssemblaWebDoCheckURLTest.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/test/java/hudson/plugins/git/browser/AssemblaWebDoCheckURLTest.java b/src/test/java/hudson/plugins/git/browser/AssemblaWebDoCheckURLTest.java index e30d4b5a19..57a3c6e35f 100644 --- a/src/test/java/hudson/plugins/git/browser/AssemblaWebDoCheckURLTest.java +++ b/src/test/java/hudson/plugins/git/browser/AssemblaWebDoCheckURLTest.java @@ -87,7 +87,17 @@ public void testPathLevelChecksOnRepoUrl() throws Exception { @Test public void testPathLevelChecksOnRepoUrlSupersetOfAssembla() throws Exception { - String url = "http://assemblagist.com/"; + Random random = new Random(); + String [] urls = { + "http://assemblage.com/", + "http://assemblage.net/", + "http://assemblage.org/", + "http://assemblages.com/", + "http://assemblages.net/", + "http://assemblages.org/", + "http://assemblagist.com/", + }; + String url = urls[random.nextInt(urls.length)]; // Don't abuse a single web site with tests assertThat(assemblaWebDescriptor.doCheckRepoUrl(project, url).getLocalizedMessage(), is("This is a valid URL but it does not look like Assembla")); } From f29824937dc8a039eb6ca8b76f81d8c8c0812b7d Mon Sep 17 00:00:00 2001 From: Mark Waite Date: Thu, 20 Feb 2020 09:22:55 -0700 Subject: [PATCH 09/18] Fix compile error on use of Random --- .../hudson/plugins/git/browser/AssemblaWebDoCheckURLTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/hudson/plugins/git/browser/AssemblaWebDoCheckURLTest.java b/src/test/java/hudson/plugins/git/browser/AssemblaWebDoCheckURLTest.java index 57a3c6e35f..281bd9868e 100644 --- a/src/test/java/hudson/plugins/git/browser/AssemblaWebDoCheckURLTest.java +++ b/src/test/java/hudson/plugins/git/browser/AssemblaWebDoCheckURLTest.java @@ -87,7 +87,7 @@ public void testPathLevelChecksOnRepoUrl() throws Exception { @Test public void testPathLevelChecksOnRepoUrlSupersetOfAssembla() throws Exception { - Random random = new Random(); + java.util.Random random = new java.util.Random(); String [] urls = { "http://assemblage.com/", "http://assemblage.net/", From 7293a3fa8f1a8f7b71d99b5e5ab4b4b0e26ca976 Mon Sep 17 00:00:00 2001 From: Rishabh Budhouliya Date: Thu, 20 Feb 2020 23:11:42 +0530 Subject: [PATCH 10/18] initial checks and utility function is shifted to GitRepositoryBrowser class --- .../plugins/git/browser/AssemblaWeb.java | 24 ++----------------- .../git/browser/GitBlitRepositoryBrowser.java | 11 +-------- .../git/browser/GitRepositoryBrowser.java | 24 +++++++++++++++++++ .../hudson/plugins/git/browser/Gitiles.java | 23 ++---------------- .../plugins/git/browser/ViewGitWeb.java | 23 ++---------------- .../browser/AssemblaWebDoCheckURLTest.java | 2 +- 6 files changed, 32 insertions(+), 75 deletions(-) diff --git a/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java b/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java index 5d3a3e22bd..4f3765f698 100644 --- a/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java +++ b/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java @@ -11,7 +11,6 @@ import hudson.scm.RepositoryBrowser; import hudson.util.FormValidation; import hudson.util.FormValidation.URLCheck; -import org.apache.commons.validator.routines.UrlValidator; import net.sf.json.JSONObject; import org.kohsuke.stapler.AncestorInPath; import org.kohsuke.stapler.DataBoundConstructor; @@ -22,7 +21,6 @@ import javax.annotation.Nonnull; import javax.servlet.ServletException; import java.io.IOException; -import java.net.URI; import java.net.URISyntaxException; import java.net.URL; @@ -104,21 +102,11 @@ public FormValidation doCheckRepoUrl(@AncestorInPath Item project, @QueryParamet throws IOException, ServletException, URISyntaxException { String cleanUrl = Util.fixEmptyAndTrim(repoUrl); - - if (cleanUrl == null) { - return FormValidation.ok(); - } - - if (project == null || !project.hasPermission(Item.CONFIGURE)) { - return FormValidation.ok(); - } - - if (cleanUrl.contains("$")) { - // set by variable, can't validate + if (initialChecksAndReturnOk(project, cleanUrl)) { return FormValidation.ok(); } FormValidation response; - if (checkURIFormat(cleanUrl)) { + if (checkURIFormat(cleanUrl, "assembla")) { return new URLCheck() { protected FormValidation check() throws IOException, ServletException { String v = cleanUrl; @@ -142,13 +130,5 @@ protected FormValidation check() throws IOException, ServletException { } return response; } - - private boolean checkURIFormat(String url) throws URISyntaxException { - //https://app.assembla.com/spaces/git-plugin/git/source - URI uri = new URI(url); - String[] schemes = {"http", "https"}; - UrlValidator urlValidator = new UrlValidator(schemes); - return urlValidator.isValid(uri.toString()) && uri.getHost().contains("assembla"); - } } } diff --git a/src/main/java/hudson/plugins/git/browser/GitBlitRepositoryBrowser.java b/src/main/java/hudson/plugins/git/browser/GitBlitRepositoryBrowser.java index 2159b560f8..cebc999cc5 100644 --- a/src/main/java/hudson/plugins/git/browser/GitBlitRepositoryBrowser.java +++ b/src/main/java/hudson/plugins/git/browser/GitBlitRepositoryBrowser.java @@ -12,7 +12,6 @@ import hudson.util.FormValidation; import hudson.util.FormValidation.URLCheck; import net.sf.json.JSONObject; -import org.apache.commons.validator.routines.UrlValidator; import org.kohsuke.stapler.AncestorInPath; import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.interceptor.RequirePOST; @@ -23,7 +22,6 @@ import javax.servlet.ServletException; import java.io.IOException; import java.io.UnsupportedEncodingException; -import java.net.URI; import java.net.URISyntaxException; import java.net.URL; import java.net.URLEncoder; @@ -105,7 +103,7 @@ public FormValidation doCheckRepoUrl(@AncestorInPath Item project, @QueryParamet return FormValidation.ok(); } FormValidation response; - if (checkURIFormat(cleanUrl)) { + if (checkURIFormat(cleanUrl, "gitblit")) { return new URLCheck() { protected FormValidation check() throws IOException, ServletException { String v = cleanUrl; @@ -129,12 +127,5 @@ protected FormValidation check() throws IOException, ServletException { } return response; } - - private boolean checkURIFormat(String url) throws URISyntaxException { - URI uri = new URI(url); - String[] schemes = {"http", "https"}; - UrlValidator urlValidator = new UrlValidator(schemes); - return urlValidator.isValid(uri.toString()) && uri.getHost().contains("gitblit"); - } } } diff --git a/src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java b/src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java index e23661756c..ece26ef44f 100644 --- a/src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java +++ b/src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java @@ -1,12 +1,14 @@ package hudson.plugins.git.browser; import hudson.EnvVars; +import hudson.model.Item; import hudson.model.Job; import hudson.model.TaskListener; import hudson.plugins.git.GitChangeSet; import hudson.plugins.git.GitChangeSet.Path; import hudson.scm.RepositoryBrowser; +import org.apache.commons.validator.routines.UrlValidator; import org.kohsuke.stapler.Stapler; import org.kohsuke.stapler.StaplerRequest; @@ -117,5 +119,27 @@ public static URL encodeURL(URL url) throws IOException { } } + public static boolean initialChecksAndReturnOk(Item project, String cleanUrl){ + if (cleanUrl == null) { + return true; + } + if (project == null || !project.hasPermission(Item.CONFIGURE)) { + return true; + } + if (cleanUrl.contains("$")) { + // set by variable, can't validate + return true; + } + return false; + } + + public static boolean checkURIFormat(String url, String browserName) throws URISyntaxException { + URI uri = new URI(url); + String[] schemes = {"http", "https"}; + UrlValidator urlValidator = new UrlValidator(schemes); + browserName = browserName + "."; + return urlValidator.isValid(uri.toString()) && uri.getHost().contains(browserName); + } + private static final long serialVersionUID = 1L; } diff --git a/src/main/java/hudson/plugins/git/browser/Gitiles.java b/src/main/java/hudson/plugins/git/browser/Gitiles.java index d0cf57037e..c8a92fba56 100644 --- a/src/main/java/hudson/plugins/git/browser/Gitiles.java +++ b/src/main/java/hudson/plugins/git/browser/Gitiles.java @@ -12,7 +12,6 @@ import hudson.util.FormValidation.URLCheck; import java.io.IOException; -import java.net.URI; import java.net.URISyntaxException; import java.net.URL; @@ -21,7 +20,6 @@ import net.sf.json.JSONObject; -import org.apache.commons.validator.routines.UrlValidator; import org.kohsuke.stapler.AncestorInPath; import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.interceptor.RequirePOST; @@ -79,21 +77,11 @@ public FormValidation doCheckRepoUrl(@AncestorInPath Item project, @QueryParamet throws IOException, ServletException, URISyntaxException { String cleanUrl = Util.fixEmptyAndTrim(repoUrl); - - if (cleanUrl == null) { - return FormValidation.ok(); - } - - if (project == null || !project.hasPermission(Item.CONFIGURE)) { - return FormValidation.ok(); - } - - if (cleanUrl.contains("$")) { - // set by variable, can't validate + if(initialChecksAndReturnOk(project, cleanUrl)){ return FormValidation.ok(); } FormValidation response; - if (checkURIFormat(cleanUrl)) { + if (checkURIFormat(cleanUrl, "gerrit")) { return new URLCheck() { protected FormValidation check() throws IOException, ServletException { String v = cleanUrl; @@ -117,12 +105,5 @@ protected FormValidation check() throws IOException, ServletException { } return response; } - - private boolean checkURIFormat(String url) throws URISyntaxException { - URI uri = new URI(url); - String[] schemes = {"http", "https"}; - UrlValidator urlValidator = new UrlValidator(schemes); - return urlValidator.isValid(uri.toString()) && uri.getHost().contains("gerrit"); - } } } diff --git a/src/main/java/hudson/plugins/git/browser/ViewGitWeb.java b/src/main/java/hudson/plugins/git/browser/ViewGitWeb.java index baa38c530c..b201810d46 100644 --- a/src/main/java/hudson/plugins/git/browser/ViewGitWeb.java +++ b/src/main/java/hudson/plugins/git/browser/ViewGitWeb.java @@ -13,7 +13,6 @@ import hudson.util.FormValidation; import hudson.util.FormValidation.URLCheck; import net.sf.json.JSONObject; -import org.apache.commons.validator.routines.UrlValidator; import org.kohsuke.stapler.AncestorInPath; import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.interceptor.RequirePOST; @@ -24,7 +23,6 @@ import javax.servlet.ServletException; import java.io.IOException; import java.io.UnsupportedEncodingException; -import java.net.URI; import java.net.URISyntaxException; import java.net.URL; import java.net.URLEncoder; @@ -99,21 +97,11 @@ public FormValidation doCheckRepoUrl(@AncestorInPath Item project, @QueryParamet throws IOException, ServletException, URISyntaxException { String cleanUrl = Util.fixEmptyAndTrim(repoUrl); - - if (cleanUrl == null) { - return FormValidation.ok(); - } - - if (project == null || !project.hasPermission(Item.CONFIGURE)) { - return FormValidation.ok(); - } - - if (cleanUrl.contains("$")) { - // set by variable, can't validate + if(initialChecksAndReturnOk(project, cleanUrl)){ return FormValidation.ok(); } FormValidation response; - if (checkURIFormat(cleanUrl)) { + if (checkURIFormat(cleanUrl, "viewgit")) { return new URLCheck() { protected FormValidation check() throws IOException, ServletException { String v = cleanUrl; @@ -137,12 +125,5 @@ protected FormValidation check() throws IOException, ServletException { } return response; } - - private boolean checkURIFormat(String url) throws URISyntaxException { - URI uri = new URI(url); - String[] schemes = {"http", "https"}; - UrlValidator urlValidator = new UrlValidator(schemes); - return urlValidator.isValid(uri.toString()) && uri.getHost().contains("viewgit"); - } } } diff --git a/src/test/java/hudson/plugins/git/browser/AssemblaWebDoCheckURLTest.java b/src/test/java/hudson/plugins/git/browser/AssemblaWebDoCheckURLTest.java index 281bd9868e..9f51080885 100644 --- a/src/test/java/hudson/plugins/git/browser/AssemblaWebDoCheckURLTest.java +++ b/src/test/java/hudson/plugins/git/browser/AssemblaWebDoCheckURLTest.java @@ -99,6 +99,6 @@ public void testPathLevelChecksOnRepoUrlSupersetOfAssembla() throws Exception { }; String url = urls[random.nextInt(urls.length)]; // Don't abuse a single web site with tests assertThat(assemblaWebDescriptor.doCheckRepoUrl(project, url).getLocalizedMessage(), - is("This is a valid URL but it does not look like Assembla")); + is("Invalid URL")); } } From 141c79f484fb2e18ee560c32c2fd6911ffee2b73 Mon Sep 17 00:00:00 2001 From: Rishabh Budhouliya Date: Thu, 20 Feb 2020 23:51:46 +0530 Subject: [PATCH 11/18] Changed access modifier for utilities in GitRepositoryBrowser --- .../java/hudson/plugins/git/browser/GitRepositoryBrowser.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java b/src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java index ece26ef44f..b3d6d92c86 100644 --- a/src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java +++ b/src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java @@ -119,7 +119,7 @@ public static URL encodeURL(URL url) throws IOException { } } - public static boolean initialChecksAndReturnOk(Item project, String cleanUrl){ + protected static boolean initialChecksAndReturnOk(Item project, String cleanUrl){ if (cleanUrl == null) { return true; } @@ -133,7 +133,7 @@ public static boolean initialChecksAndReturnOk(Item project, String cleanUrl){ return false; } - public static boolean checkURIFormat(String url, String browserName) throws URISyntaxException { + protected static boolean checkURIFormat(String url, String browserName) throws URISyntaxException { URI uri = new URI(url); String[] schemes = {"http", "https"}; UrlValidator urlValidator = new UrlValidator(schemes); From 454397e00aa243e9ccc3d65e38584620be6b5007 Mon Sep 17 00:00:00 2001 From: Rishabh Budhouliya Date: Fri, 21 Feb 2020 10:47:19 +0530 Subject: [PATCH 12/18] change in checkURIFormat --- .../java/hudson/plugins/git/browser/AssemblaWeb.java | 12 +++++++++++- .../plugins/git/browser/GitRepositoryBrowser.java | 5 ++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java b/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java index 4f3765f698..b9d9899ac8 100644 --- a/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java +++ b/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java @@ -12,6 +12,7 @@ import hudson.util.FormValidation; import hudson.util.FormValidation.URLCheck; import net.sf.json.JSONObject; +import org.apache.commons.validator.routines.UrlValidator; import org.kohsuke.stapler.AncestorInPath; import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.interceptor.RequirePOST; @@ -21,6 +22,7 @@ import javax.annotation.Nonnull; import javax.servlet.ServletException; import java.io.IOException; +import java.net.URI; import java.net.URISyntaxException; import java.net.URL; @@ -106,7 +108,7 @@ public FormValidation doCheckRepoUrl(@AncestorInPath Item project, @QueryParamet return FormValidation.ok(); } FormValidation response; - if (checkURIFormat(cleanUrl, "assembla")) { + if (checkURIFormatAndHostName(cleanUrl, "assembla")) { return new URLCheck() { protected FormValidation check() throws IOException, ServletException { String v = cleanUrl; @@ -130,5 +132,13 @@ protected FormValidation check() throws IOException, ServletException { } return response; } + + private boolean checkURIFormatAndHostName(String url, String browserName) throws URISyntaxException { + URI uri = new URI(url); + String[] schemes = {"http", "https"}; + UrlValidator urlValidator = new UrlValidator(schemes); + browserName = browserName + "."; + return urlValidator.isValid(uri.toString()) && uri.getHost().contains(browserName); + } } } diff --git a/src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java b/src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java index b3d6d92c86..d84e555e5d 100644 --- a/src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java +++ b/src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java @@ -133,12 +133,11 @@ protected static boolean initialChecksAndReturnOk(Item project, String cleanUrl) return false; } - protected static boolean checkURIFormat(String url, String browserName) throws URISyntaxException { + protected static boolean checkURIFormat(String url) throws URISyntaxException { URI uri = new URI(url); String[] schemes = {"http", "https"}; UrlValidator urlValidator = new UrlValidator(schemes); - browserName = browserName + "."; - return urlValidator.isValid(uri.toString()) && uri.getHost().contains(browserName); + return urlValidator.isValid(uri.toString()); } private static final long serialVersionUID = 1L; From 6532b346f68650902fcc90fa202a504bf9360ea9 Mon Sep 17 00:00:00 2001 From: Rishabh Budhouliya Date: Fri, 21 Feb 2020 10:58:49 +0530 Subject: [PATCH 13/18] Revert "change in checkURIFormat" compile time errors corrected. This reverts commit 454397e00aa243e9ccc3d65e38584620be6b5007. --- .../java/hudson/plugins/git/browser/AssemblaWeb.java | 12 +----------- .../plugins/git/browser/GitRepositoryBrowser.java | 5 +++-- 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java b/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java index b9d9899ac8..4f3765f698 100644 --- a/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java +++ b/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java @@ -12,7 +12,6 @@ import hudson.util.FormValidation; import hudson.util.FormValidation.URLCheck; import net.sf.json.JSONObject; -import org.apache.commons.validator.routines.UrlValidator; import org.kohsuke.stapler.AncestorInPath; import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.interceptor.RequirePOST; @@ -22,7 +21,6 @@ import javax.annotation.Nonnull; import javax.servlet.ServletException; import java.io.IOException; -import java.net.URI; import java.net.URISyntaxException; import java.net.URL; @@ -108,7 +106,7 @@ public FormValidation doCheckRepoUrl(@AncestorInPath Item project, @QueryParamet return FormValidation.ok(); } FormValidation response; - if (checkURIFormatAndHostName(cleanUrl, "assembla")) { + if (checkURIFormat(cleanUrl, "assembla")) { return new URLCheck() { protected FormValidation check() throws IOException, ServletException { String v = cleanUrl; @@ -132,13 +130,5 @@ protected FormValidation check() throws IOException, ServletException { } return response; } - - private boolean checkURIFormatAndHostName(String url, String browserName) throws URISyntaxException { - URI uri = new URI(url); - String[] schemes = {"http", "https"}; - UrlValidator urlValidator = new UrlValidator(schemes); - browserName = browserName + "."; - return urlValidator.isValid(uri.toString()) && uri.getHost().contains(browserName); - } } } diff --git a/src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java b/src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java index d84e555e5d..b3d6d92c86 100644 --- a/src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java +++ b/src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java @@ -133,11 +133,12 @@ protected static boolean initialChecksAndReturnOk(Item project, String cleanUrl) return false; } - protected static boolean checkURIFormat(String url) throws URISyntaxException { + protected static boolean checkURIFormat(String url, String browserName) throws URISyntaxException { URI uri = new URI(url); String[] schemes = {"http", "https"}; UrlValidator urlValidator = new UrlValidator(schemes); - return urlValidator.isValid(uri.toString()); + browserName = browserName + "."; + return urlValidator.isValid(uri.toString()) && uri.getHost().contains(browserName); } private static final long serialVersionUID = 1L; From 4a0915a59a93402c934bab30a52636346a8a04d7 Mon Sep 17 00:00:00 2001 From: Rishabh Budhouliya Date: Fri, 21 Feb 2020 11:03:57 +0530 Subject: [PATCH 14/18] Change in checkURIFormat and addition of checkURIFormatAndHostname in AssemblaWeb --- .../java/hudson/plugins/git/browser/AssemblaWeb.java | 12 +++++++++++- .../git/browser/GitBlitRepositoryBrowser.java | 2 +- .../plugins/git/browser/GitRepositoryBrowser.java | 5 ++--- .../java/hudson/plugins/git/browser/Gitiles.java | 2 +- .../java/hudson/plugins/git/browser/ViewGitWeb.java | 2 +- 5 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java b/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java index 4f3765f698..b9d9899ac8 100644 --- a/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java +++ b/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java @@ -12,6 +12,7 @@ import hudson.util.FormValidation; import hudson.util.FormValidation.URLCheck; import net.sf.json.JSONObject; +import org.apache.commons.validator.routines.UrlValidator; import org.kohsuke.stapler.AncestorInPath; import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.interceptor.RequirePOST; @@ -21,6 +22,7 @@ import javax.annotation.Nonnull; import javax.servlet.ServletException; import java.io.IOException; +import java.net.URI; import java.net.URISyntaxException; import java.net.URL; @@ -106,7 +108,7 @@ public FormValidation doCheckRepoUrl(@AncestorInPath Item project, @QueryParamet return FormValidation.ok(); } FormValidation response; - if (checkURIFormat(cleanUrl, "assembla")) { + if (checkURIFormatAndHostName(cleanUrl, "assembla")) { return new URLCheck() { protected FormValidation check() throws IOException, ServletException { String v = cleanUrl; @@ -130,5 +132,13 @@ protected FormValidation check() throws IOException, ServletException { } return response; } + + private boolean checkURIFormatAndHostName(String url, String browserName) throws URISyntaxException { + URI uri = new URI(url); + String[] schemes = {"http", "https"}; + UrlValidator urlValidator = new UrlValidator(schemes); + browserName = browserName + "."; + return urlValidator.isValid(uri.toString()) && uri.getHost().contains(browserName); + } } } diff --git a/src/main/java/hudson/plugins/git/browser/GitBlitRepositoryBrowser.java b/src/main/java/hudson/plugins/git/browser/GitBlitRepositoryBrowser.java index cebc999cc5..1e68369233 100644 --- a/src/main/java/hudson/plugins/git/browser/GitBlitRepositoryBrowser.java +++ b/src/main/java/hudson/plugins/git/browser/GitBlitRepositoryBrowser.java @@ -103,7 +103,7 @@ public FormValidation doCheckRepoUrl(@AncestorInPath Item project, @QueryParamet return FormValidation.ok(); } FormValidation response; - if (checkURIFormat(cleanUrl, "gitblit")) { + if (checkURIFormat(cleanUrl)) { return new URLCheck() { protected FormValidation check() throws IOException, ServletException { String v = cleanUrl; diff --git a/src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java b/src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java index b3d6d92c86..d84e555e5d 100644 --- a/src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java +++ b/src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java @@ -133,12 +133,11 @@ protected static boolean initialChecksAndReturnOk(Item project, String cleanUrl) return false; } - protected static boolean checkURIFormat(String url, String browserName) throws URISyntaxException { + protected static boolean checkURIFormat(String url) throws URISyntaxException { URI uri = new URI(url); String[] schemes = {"http", "https"}; UrlValidator urlValidator = new UrlValidator(schemes); - browserName = browserName + "."; - return urlValidator.isValid(uri.toString()) && uri.getHost().contains(browserName); + return urlValidator.isValid(uri.toString()); } private static final long serialVersionUID = 1L; diff --git a/src/main/java/hudson/plugins/git/browser/Gitiles.java b/src/main/java/hudson/plugins/git/browser/Gitiles.java index c8a92fba56..b50eede513 100644 --- a/src/main/java/hudson/plugins/git/browser/Gitiles.java +++ b/src/main/java/hudson/plugins/git/browser/Gitiles.java @@ -81,7 +81,7 @@ public FormValidation doCheckRepoUrl(@AncestorInPath Item project, @QueryParamet return FormValidation.ok(); } FormValidation response; - if (checkURIFormat(cleanUrl, "gerrit")) { + if (checkURIFormat(cleanUrl)) { return new URLCheck() { protected FormValidation check() throws IOException, ServletException { String v = cleanUrl; diff --git a/src/main/java/hudson/plugins/git/browser/ViewGitWeb.java b/src/main/java/hudson/plugins/git/browser/ViewGitWeb.java index b201810d46..971e92a954 100644 --- a/src/main/java/hudson/plugins/git/browser/ViewGitWeb.java +++ b/src/main/java/hudson/plugins/git/browser/ViewGitWeb.java @@ -101,7 +101,7 @@ public FormValidation doCheckRepoUrl(@AncestorInPath Item project, @QueryParamet return FormValidation.ok(); } FormValidation response; - if (checkURIFormat(cleanUrl, "viewgit")) { + if (checkURIFormat(cleanUrl)) { return new URLCheck() { protected FormValidation check() throws IOException, ServletException { String v = cleanUrl; From 5f48308e825580e8f74e5d85997045a74504c081 Mon Sep 17 00:00:00 2001 From: Rishabh Budhouliya Date: Fri, 21 Feb 2020 11:38:19 +0530 Subject: [PATCH 15/18] Removal of fixes of whitespace and temporary variable response --- .../plugins/git/browser/AssemblaWeb.java | 4 +--- .../git/browser/GitBlitRepositoryBrowser.java | 18 +++--------------- .../hudson/plugins/git/browser/Gitiles.java | 4 +--- .../hudson/plugins/git/browser/ViewGitWeb.java | 16 +++++++--------- 4 files changed, 12 insertions(+), 30 deletions(-) diff --git a/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java b/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java index b9d9899ac8..cd0b0f26ef 100644 --- a/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java +++ b/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java @@ -107,7 +107,6 @@ public FormValidation doCheckRepoUrl(@AncestorInPath Item project, @QueryParamet if (initialChecksAndReturnOk(project, cleanUrl)) { return FormValidation.ok(); } - FormValidation response; if (checkURIFormatAndHostName(cleanUrl, "assembla")) { return new URLCheck() { protected FormValidation check() throws IOException, ServletException { @@ -128,9 +127,8 @@ protected FormValidation check() throws IOException, ServletException { } }.check(); } else { - response = FormValidation.error(Messages.invalidUrl()); + return FormValidation.error(Messages.invalidUrl()); } - return response; } private boolean checkURIFormatAndHostName(String url, String browserName) throws URISyntaxException { diff --git a/src/main/java/hudson/plugins/git/browser/GitBlitRepositoryBrowser.java b/src/main/java/hudson/plugins/git/browser/GitBlitRepositoryBrowser.java index 1e68369233..e321dd4266 100644 --- a/src/main/java/hudson/plugins/git/browser/GitBlitRepositoryBrowser.java +++ b/src/main/java/hudson/plugins/git/browser/GitBlitRepositoryBrowser.java @@ -67,7 +67,7 @@ public String getProjectName() { return projectName; } - private String encodeString(final String s) throws UnsupportedEncodingException { + private String encodeString(final String s) throws UnsupportedEncodingException { return URLEncoder.encode(s, "UTF-8").replaceAll("\\+", "%20"); } @@ -89,20 +89,9 @@ public FormValidation doCheckRepoUrl(@AncestorInPath Item project, @QueryParamet throws IOException, ServletException, URISyntaxException { String cleanUrl = Util.fixEmptyAndTrim(repoUrl); - - if (cleanUrl == null) { - return FormValidation.ok(); - } - - if (project == null || !project.hasPermission(Item.CONFIGURE)) { - return FormValidation.ok(); - } - - if (cleanUrl.contains("$")) { - // set by variable, can't validate + if(initialChecksAndReturnOk(project, cleanUrl)){ return FormValidation.ok(); } - FormValidation response; if (checkURIFormat(cleanUrl)) { return new URLCheck() { protected FormValidation check() throws IOException, ServletException { @@ -123,9 +112,8 @@ protected FormValidation check() throws IOException, ServletException { } }.check(); } else { - response = FormValidation.error(Messages.invalidUrl()); + return FormValidation.error(Messages.invalidUrl()); } - return response; } } } diff --git a/src/main/java/hudson/plugins/git/browser/Gitiles.java b/src/main/java/hudson/plugins/git/browser/Gitiles.java index b50eede513..5e48172f24 100644 --- a/src/main/java/hudson/plugins/git/browser/Gitiles.java +++ b/src/main/java/hudson/plugins/git/browser/Gitiles.java @@ -80,7 +80,6 @@ public FormValidation doCheckRepoUrl(@AncestorInPath Item project, @QueryParamet if(initialChecksAndReturnOk(project, cleanUrl)){ return FormValidation.ok(); } - FormValidation response; if (checkURIFormat(cleanUrl)) { return new URLCheck() { protected FormValidation check() throws IOException, ServletException { @@ -101,9 +100,8 @@ protected FormValidation check() throws IOException, ServletException { } }.check(); } else { - response = FormValidation.error(Messages.invalidUrl()); + return FormValidation.error(Messages.invalidUrl()); } - return response; } } } diff --git a/src/main/java/hudson/plugins/git/browser/ViewGitWeb.java b/src/main/java/hudson/plugins/git/browser/ViewGitWeb.java index 971e92a954..3a1ad3011e 100644 --- a/src/main/java/hudson/plugins/git/browser/ViewGitWeb.java +++ b/src/main/java/hudson/plugins/git/browser/ViewGitWeb.java @@ -44,7 +44,7 @@ public URL getDiffLink(Path path) throws IOException { if (path.getEditType() == EditType.EDIT) { URL url = getUrl(); String spec = buildCommitDiffSpec(url, path); - return new URL(url, url.getPath() + spec); + return new URL(url, url.getPath() + spec); } return null; } @@ -56,14 +56,14 @@ public URL getFileLink(Path path) throws IOException { String spec = buildCommitDiffSpec(url, path); return encodeURL(new URL(url, url.getPath() + spec)); } - String spec = param(url).add("p=" + projectName).add("a=viewblob").add("h=" + path.getDst()).add("f=" + path.getPath()).toString(); + String spec = param(url).add("p=" + projectName).add("a=viewblob").add("h=" + path.getDst()).add("f=" + path.getPath()).toString(); return encodeURL(new URL(url, url.getPath() + spec)); } - private String buildCommitDiffSpec(URL url, Path path) - throws UnsupportedEncodingException { - return param(url).add("p=" + projectName).add("a=commitdiff").add("h=" + path.getChangeSet().getId()) + "#" + URLEncoder.encode(path.getPath(), "UTF-8").toString(); - } + private String buildCommitDiffSpec(URL url, Path path) + throws UnsupportedEncodingException { + return param(url).add("p=" + projectName).add("a=commitdiff").add("h=" + path.getChangeSet().getId()) + "#" + URLEncoder.encode(path.getPath(), "UTF-8").toString(); + } @Override public URL getChangeSetLink(GitChangeSet changeSet) throws IOException { @@ -100,7 +100,6 @@ public FormValidation doCheckRepoUrl(@AncestorInPath Item project, @QueryParamet if(initialChecksAndReturnOk(project, cleanUrl)){ return FormValidation.ok(); } - FormValidation response; if (checkURIFormat(cleanUrl)) { return new URLCheck() { protected FormValidation check() throws IOException, ServletException { @@ -121,9 +120,8 @@ protected FormValidation check() throws IOException, ServletException { } }.check(); } else { - response = FormValidation.error(Messages.invalidUrl()); + return FormValidation.error(Messages.invalidUrl()); } - return response; } } } From 8b1433ccfceea85827217f279cb0b06b35307d63 Mon Sep 17 00:00:00 2001 From: Mark Waite Date: Sat, 29 Feb 2020 16:56:22 -0700 Subject: [PATCH 16/18] Revert CloneOptions change from a different branch Refspec expansion is handled in another pull request and should be evaluated separately. --- .../hudson/plugins/git/extensions/impl/CloneOption.java | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/main/java/hudson/plugins/git/extensions/impl/CloneOption.java b/src/main/java/hudson/plugins/git/extensions/impl/CloneOption.java index fb1bdf6933..be46f4339b 100644 --- a/src/main/java/hudson/plugins/git/extensions/impl/CloneOption.java +++ b/src/main/java/hudson/plugins/git/extensions/impl/CloneOption.java @@ -14,7 +14,6 @@ import hudson.plugins.git.util.GitUtils; import hudson.slaves.NodeProperty; import java.io.IOException; -import java.util.ArrayList; import java.util.List; import java.util.Objects; import org.eclipse.jgit.transport.RefSpec; @@ -149,12 +148,7 @@ public void decorateCloneCommand(GitSCM scm, Run build, GitClient git, Tas // in a single job definition. RemoteConfig rc = scm.getRepositories().get(0); List refspecs = rc.getFetchRefSpecs(); - List expandedRefSpecs = new ArrayList<>(); - EnvVars env = build.getEnvironment(listener); - for (RefSpec ref : refspecs) { - expandedRefSpecs.add(new RefSpec(env.expand(ref.toString()))); - } - cmd.refspecs(expandedRefSpecs); + cmd.refspecs(refspecs); } cmd.timeout(timeout); From acebb4689a095337b3196d5451e88ab6a5ec2909 Mon Sep 17 00:00:00 2001 From: Mark Waite Date: Sat, 29 Feb 2020 16:58:27 -0700 Subject: [PATCH 17/18] Reduce whitespace differences from master branch --- .../plugins/git/browser/AssemblaWeb.java | 41 +++++++-------- .../git/browser/GitBlitRepositoryBrowser.java | 41 +++++++-------- .../git/browser/GitRepositoryBrowser.java | 3 +- .../hudson/plugins/git/browser/Gitiles.java | 39 +++++++-------- .../plugins/git/browser/ViewGitWeb.java | 50 +++++++++---------- 5 files changed, 86 insertions(+), 88 deletions(-) diff --git a/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java b/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java index cd0b0f26ef..5c2622e771 100644 --- a/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java +++ b/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java @@ -104,31 +104,32 @@ public FormValidation doCheckRepoUrl(@AncestorInPath Item project, @QueryParamet throws IOException, ServletException, URISyntaxException { String cleanUrl = Util.fixEmptyAndTrim(repoUrl); - if (initialChecksAndReturnOk(project, cleanUrl)) { + if (initialChecksAndReturnOk(project, cleanUrl)) + { return FormValidation.ok(); } - if (checkURIFormatAndHostName(cleanUrl, "assembla")) { - return new URLCheck() { - protected FormValidation check() throws IOException, ServletException { - String v = cleanUrl; - if (!v.endsWith("/")) { - v += '/'; - } + // Connect to URL and check content only if we have permission + if (!checkURIFormatAndHostName(cleanUrl, "assembla")) { + return FormValidation.error(Messages.invalidUrl()); + } + return new URLCheck() { + protected FormValidation check() throws IOException, ServletException { + String v = cleanUrl; + if (!v.endsWith("/")) { + v += '/'; + } - try { - if (findText(open(new URL(v)), "Assembla")) { - return FormValidation.ok(); - } else { - return FormValidation.error("This is a valid URL but it does not look like Assembla"); - } - } catch (IOException e) { - return handleIOException(v, e); + try { + if (findText(open(new URL(v)), "Assembla")) { + return FormValidation.ok(); + } else { + return FormValidation.error("This is a valid URL but it does not look like Assembla"); } + } catch (IOException e) { + return handleIOException(v, e); } - }.check(); - } else { - return FormValidation.error(Messages.invalidUrl()); - } + } + }.check(); } private boolean checkURIFormatAndHostName(String url, String browserName) throws URISyntaxException { diff --git a/src/main/java/hudson/plugins/git/browser/GitBlitRepositoryBrowser.java b/src/main/java/hudson/plugins/git/browser/GitBlitRepositoryBrowser.java index e321dd4266..5a4706a32f 100644 --- a/src/main/java/hudson/plugins/git/browser/GitBlitRepositoryBrowser.java +++ b/src/main/java/hudson/plugins/git/browser/GitBlitRepositoryBrowser.java @@ -89,31 +89,32 @@ public FormValidation doCheckRepoUrl(@AncestorInPath Item project, @QueryParamet throws IOException, ServletException, URISyntaxException { String cleanUrl = Util.fixEmptyAndTrim(repoUrl); - if(initialChecksAndReturnOk(project, cleanUrl)){ + if (initialChecksAndReturnOk(project, cleanUrl)) + { return FormValidation.ok(); } - if (checkURIFormat(cleanUrl)) { - return new URLCheck() { - protected FormValidation check() throws IOException, ServletException { - String v = cleanUrl; - if (!v.endsWith("/")) { - v += '/'; - } + if (!checkURIFormat(cleanUrl)) + { + return FormValidation.error(Messages.invalidUrl()); + } + return new URLCheck() { + protected FormValidation check() throws IOException, ServletException { + String v = cleanUrl; + if (!v.endsWith("/")) { + v += '/'; + } - try { - if (findText(open(new URL(v)), "Gitblit")) { - return FormValidation.ok(); - } else { - return FormValidation.error("This is a valid URL but it doesn't look like Gitblit"); - } - } catch (IOException e) { - return handleIOException(v, e); + try { + if (findText(open(new URL(v)), "Gitblit")) { + return FormValidation.ok(); + } else { + return FormValidation.error("This is a valid URL but it doesn't look like Gitblit"); } + } catch (IOException e) { + return handleIOException(v, e); } - }.check(); - } else { - return FormValidation.error(Messages.invalidUrl()); - } + } + }.check(); } } } diff --git a/src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java b/src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java index d84e555e5d..b5b22d5a5d 100644 --- a/src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java +++ b/src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java @@ -134,10 +134,9 @@ protected static boolean initialChecksAndReturnOk(Item project, String cleanUrl) } protected static boolean checkURIFormat(String url) throws URISyntaxException { - URI uri = new URI(url); String[] schemes = {"http", "https"}; UrlValidator urlValidator = new UrlValidator(schemes); - return urlValidator.isValid(uri.toString()); + return urlValidator.isValid(url); } private static final long serialVersionUID = 1L; diff --git a/src/main/java/hudson/plugins/git/browser/Gitiles.java b/src/main/java/hudson/plugins/git/browser/Gitiles.java index 5e48172f24..acf32dbfa8 100644 --- a/src/main/java/hudson/plugins/git/browser/Gitiles.java +++ b/src/main/java/hudson/plugins/git/browser/Gitiles.java @@ -80,28 +80,27 @@ public FormValidation doCheckRepoUrl(@AncestorInPath Item project, @QueryParamet if(initialChecksAndReturnOk(project, cleanUrl)){ return FormValidation.ok(); } - if (checkURIFormat(cleanUrl)) { - return new URLCheck() { - protected FormValidation check() throws IOException, ServletException { - String v = cleanUrl; - if (!v.endsWith("/")) { - v += '/'; - } - - try { - if (findText(open(new URL(v)), "git clone")) { - return FormValidation.ok(); - } else { - return FormValidation.error("This is a valid URL but it doesn't look like Gitiles"); - } - } catch (IOException e) { - return handleIOException(v, e); - } - } - }.check(); - } else { + if (!checkURIFormat(cleanUrl)) { return FormValidation.error(Messages.invalidUrl()); } + return new URLCheck() { + protected FormValidation check() throws IOException, ServletException { + String v = cleanUrl; + if (!v.endsWith("/")) + v += '/'; + + try { + // gitiles has a line in main page indicating how to clone the project + if (findText(open(new URL(v)), "git clone")) { + return FormValidation.ok(); + } else { + return FormValidation.error("This is a valid URL but it doesn't look like Gitiles"); + } + } catch (IOException e) { + return handleIOException(v, e); + } + } + }.check(); } } } diff --git a/src/main/java/hudson/plugins/git/browser/ViewGitWeb.java b/src/main/java/hudson/plugins/git/browser/ViewGitWeb.java index 3a1ad3011e..74234f8ea0 100644 --- a/src/main/java/hudson/plugins/git/browser/ViewGitWeb.java +++ b/src/main/java/hudson/plugins/git/browser/ViewGitWeb.java @@ -44,7 +44,7 @@ public URL getDiffLink(Path path) throws IOException { if (path.getEditType() == EditType.EDIT) { URL url = getUrl(); String spec = buildCommitDiffSpec(url, path); - return new URL(url, url.getPath() + spec); + return new URL(url, url.getPath() + spec); } return null; } @@ -60,10 +60,10 @@ public URL getFileLink(Path path) throws IOException { return encodeURL(new URL(url, url.getPath() + spec)); } - private String buildCommitDiffSpec(URL url, Path path) - throws UnsupportedEncodingException { - return param(url).add("p=" + projectName).add("a=commitdiff").add("h=" + path.getChangeSet().getId()) + "#" + URLEncoder.encode(path.getPath(), "UTF-8").toString(); - } + private String buildCommitDiffSpec(URL url, Path path) + throws UnsupportedEncodingException { + return param(url).add("p=" + projectName).add("a=commitdiff").add("h=" + path.getChangeSet().getId()) + "#" + URLEncoder.encode(path.getPath(),"UTF-8").toString(); + } @Override public URL getChangeSetLink(GitChangeSet changeSet) throws IOException { @@ -97,31 +97,29 @@ public FormValidation doCheckRepoUrl(@AncestorInPath Item project, @QueryParamet throws IOException, ServletException, URISyntaxException { String cleanUrl = Util.fixEmptyAndTrim(repoUrl); - if(initialChecksAndReturnOk(project, cleanUrl)){ + // Connect to URL and check content only if we have admin permission + if (initialChecksAndReturnOk(project, cleanUrl)) return FormValidation.ok(); + if (!checkURIFormat(cleanUrl)) { + return FormValidation.error(Messages.invalidUrl()); } - if (checkURIFormat(cleanUrl)) { - return new URLCheck() { - protected FormValidation check() throws IOException, ServletException { - String v = cleanUrl; - if (!v.endsWith("/")) { - v += '/'; - } - - try { - if (findText(open(new URL(v)), "ViewGit")) { - return FormValidation.ok(); - } else { - return FormValidation.error("This is a valid URL but it doesn't look like ViewGit"); - } - } catch (IOException e) { - return handleIOException(v, e); + return new URLCheck() { + protected FormValidation check() throws IOException, ServletException { + String v = cleanUrl; + if (!v.endsWith("/")) + v += '/'; + + try { + if (findText(open(new URL(v)), "ViewGit")) { + return FormValidation.ok(); + } else { + return FormValidation.error("This is a valid URL but it doesn't look like ViewGit"); } + } catch (IOException e) { + return handleIOException(v, e); } - }.check(); - } else { - return FormValidation.error(Messages.invalidUrl()); - } + } + }.check(); } } } From d9fe71b6657ec3ab20fc5eaa7cf7896b035ba154 Mon Sep 17 00:00:00 2001 From: Mark Waite Date: Sat, 29 Feb 2020 17:05:22 -0700 Subject: [PATCH 18/18] Better variable name in Assembla checker --- src/main/java/hudson/plugins/git/browser/AssemblaWeb.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java b/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java index 5c2622e771..aa05edf63b 100644 --- a/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java +++ b/src/main/java/hudson/plugins/git/browser/AssemblaWeb.java @@ -132,12 +132,12 @@ protected FormValidation check() throws IOException, ServletException { }.check(); } - private boolean checkURIFormatAndHostName(String url, String browserName) throws URISyntaxException { + private boolean checkURIFormatAndHostName(String url, String hostNameFragment) throws URISyntaxException { URI uri = new URI(url); String[] schemes = {"http", "https"}; UrlValidator urlValidator = new UrlValidator(schemes); - browserName = browserName + "."; - return urlValidator.isValid(uri.toString()) && uri.getHost().contains(browserName); + hostNameFragment = hostNameFragment + "."; + return urlValidator.isValid(uri.toString()) && uri.getHost().contains(hostNameFragment); } } }