Fixing validation for checkout of tag and branch names, tidying up

Messages and fixing JENKINS-9045
jenkinsci · Jan 2, 2012 · 31ec23f · 31ec23f
1 parent 66a21fc
commit 31ec23f
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 32 deletions.
diff --git a/src/main/java/hudson/scm/CVSSCM.java b/src/main/java/hudson/scm/CVSSCM.java
@@ -1007,7 +1007,11 @@ public FormValidation doCheckBranchName(@QueryParameter final String value) {
             String v = fixNull(value);
 
             if (v.equals("HEAD")) {
-                return FormValidation.error(Messages.CVSSCM_HeadIsNotBranch());
+                return FormValidation.error(Messages.CVSSCM_HeadIsNotTag(Messages.CVSSCM_Branch()));
+            }
+
+            if (!v.equals(v.trim())) {
+                return FormValidation.error(Messages.CVSSCM_TagNameInvalid(Messages.CVSSCM_Branch()));
             }
 
             return FormValidation.ok();
@@ -1020,7 +1024,11 @@ public FormValidation doCheckTagName(@QueryParameter final String value) {
             String v = fixNull(value);
 
             if (v.equals("HEAD")) {
-                return FormValidation.error(Messages.CVSSCM_HeadIsNotTag());
+                return FormValidation.error(Messages.CVSSCM_HeadIsNotTag(Messages.CVSSCM_Tag()));
+            }
+
+            if (!v.equals(v.trim())) {
+                return FormValidation.error(Messages.CVSSCM_TagNameInvalid(Messages.CVSSCM_Tag()));
             }
 
             return FormValidation.ok();

diff --git a/src/main/java/hudson/scm/cvstagging/CvsTagAction.java b/src/main/java/hudson/scm/cvstagging/CvsTagAction.java
@@ -51,13 +51,11 @@
 import org.kohsuke.stapler.export.ExportedBean;
 
 @ExportedBean
-public class CvsTagAction extends AbstractScmTagAction implements
-                Describable<CvsTagAction> {
+public class CvsTagAction extends AbstractScmTagAction implements Describable<CvsTagAction> {
 
     private final List<String> tagNames = new ArrayList<String>();
 
-    public CvsTagAction(final AbstractBuild<?, ?> build,
-                    final CvsRepository[] repositories) {
+    public CvsTagAction(final AbstractBuild<?, ?> build, final CvsRepository[] repositories) {
         super(build);
     }
 
@@ -97,31 +95,25 @@ public String[] getTagNames() {
         return tagNames.toArray(new String[tagNames.size()]);
     }
 
-    public synchronized void doSubmit(final StaplerRequest request,
-                    final StaplerResponse response) throws IOException,
+    public synchronized void doSubmit(final StaplerRequest request, final StaplerResponse response) throws IOException,
                     ServletException {
         // check the user is allowed to tag
         getBuild().checkPermission(getPermission());
 
         final String tagName = fixNull(request.getParameter("name")).trim();
         if (!checkTagName(tagName)) {
-            sendError(Messages.CVSSCM_TagNameInvalid());
+            sendError(Messages.CVSSCM_TagNameInvalid(Messages.CVSSCM_Tag()));
         }
 
         if (null != request.getParameter("upstream")) {
             @SuppressWarnings("unchecked")
-            Map<AbstractProject<?, ?>, Integer> upstream = getBuild()
-                            .getTransitiveUpstreamBuilds();
+            Map<AbstractProject<?, ?>, Integer> upstream = getBuild().getTransitiveUpstreamBuilds();
             for (AbstractProject<?, ?> project : upstream.keySet()) {
-                String upstreamTagName = fixNull(
-                                request.getParameter("upstream."
-                                                + project.getName())).trim();
+                String upstreamTagName = fixNull(request.getParameter("upstream." + project.getName())).trim();
                 if (!checkTagName(upstreamTagName)) {
-                    sendError(Messages.CVSSCM_TagNameInvalid());
+                    sendError(Messages.CVSSCM_TagNameInvalid(Messages.CVSSCM_Tag()));
                 }
-                CvsTagAction action = project.getBuildByNumber(
-                                upstream.get(project)).getAction(
-                                CvsTagAction.class);
+                CvsTagAction action = project.getBuildByNumber(upstream.get(project)).getAction(CvsTagAction.class);
                 if (null != action) {
                     action.perform(upstreamTagName);
                 }
@@ -152,10 +144,18 @@ public FormValidation doCheckTag(@QueryParameter final String value) {
             return FormValidation.ok();
         }
 
-        return FormValidation.error(Messages.CVSSCM_TagNameInvalid());
+        return FormValidation.error(Messages.CVSSCM_TagNameInvalid(Messages.CVSSCM_Tag()));
     }
 
     public boolean checkTagName(final String tagName) {
+        /*
+         * we can improve this:
+         * YouÕve probably noticed that no periods or spaces were used in the
+         * tag names. CVS is rather strict about what constitutes a valid tag
+         * name. The rules are that it must start with a letter and contain
+         * letters, digits, hyphens (Ò-Ó), and underscores (Ò_Ó). No spaces,
+         * periods, colons, commas, or any other symbols may be used.
+         */
         if (fixNull(tagName).length() == 0) {
             return false;
         }

diff --git a/src/main/java/hudson/scm/cvstagging/LegacyTagAction.java b/src/main/java/hudson/scm/cvstagging/LegacyTagAction.java
@@ -251,17 +251,17 @@ private String isInvalidTag(final String name) {
         // error (1, 0, "tag `%s' must start with a letter", tag);
         // }
         if (name == null || name.length() == 0) {
-            return Messages.CVSSCM_TagIsEmpty();
+            return Messages.CVSSCM_TagNameInvalid(Messages.CVSSCM_Tag());
         }
 
         char ch = name.charAt(0);
         if (!(('A' <= ch && ch <= 'Z') || ('a' <= ch && ch <= 'z'))) {
-            return Messages.CVSSCM_TagNeedsToStartWithAlphabet();
+            return Messages.CVSSCM_TagNameInvalid(Messages.CVSSCM_Tag());
         }
 
         for (char invalid : "$,.:;@".toCharArray()) {
             if (name.indexOf(invalid) >= 0) {
-                return Messages.CVSSCM_TagContainsIllegalChar(invalid);
+                return Messages.CVSSCM_TagNameInvalid(Messages.CVSSCM_Tag());
             }
         }
 

diff --git a/src/main/resources/hudson/scm/CVSSCM/config.jelly b/src/main/resources/hudson/scm/CVSSCM/config.jelly
@@ -42,17 +42,17 @@ THE SOFTWARE.
                     <f:block>
                         <f:radioBlock name="moduleLocation" value="head" title="${%Head}" checked="${module.moduleLocation.locationType == 'head' || loc.repositoryLocation.locationType == null}" />
                         <f:radioBlock name="moduleLocation" value="branch" title="${%Branch}" checked="${module.moduleLocation.locationType == 'branch'}">
-                            <f:entry title="${%Branch Name}">
-                                <f:textbox type="text" name="branchName" value="${module.moduleLocation.branchName}" />
+                            <f:entry title="${%Branch Name}" field="branchName">
+                                <f:textbox type="text" value="${module.moduleLocation.branchName}" />
                             </f:entry>
                             <f:entry>
                                 <f:checkbox checked="${module.moduleLocation.useHeadIfBranchNotFound != false}" name="useHeadIfBranchNotFound" />
                                 ${%Use HEAD revision if branch not found}
                             </f:entry>
                         </f:radioBlock>
                         <f:radioBlock name="moduleLocation" value="tag" title="${%Tag}" checked="${module.moduleLocation.locationType == 'tag'}">
-                            <f:entry title="${%Tag Name}">
-                                <f:textbox type="text" name="tagName" value="${module.moduleLocation.tagName}" />
+                            <f:entry title="${%Tag Name}" field="tagName">
+                                <f:textbox type="text" value="${module.moduleLocation.tagName}" />
                             </f:entry>
                             <f:entry>
                                 <f:checkbox checked="${module.moduleLocation.useHeadIfTagNotFound != false}" name="useHeadIfTagNotFound" />

diff --git a/src/main/resources/hudson/scm/cvs/Messages.properties b/src/main/resources/hudson/scm/cvs/Messages.properties
@@ -26,22 +26,20 @@
 CVSSCM.TagginXasY=Tagging {0} as {1}
 CVSSCM.FailedToMarkForKeep=Failed to mark {0} for keep
 CVSSCM.ExpandingWorkspaceArchive=expanding the workspace archive into {0}
-CVSSCM.HeadIsNotBranch=Technically, HEAD is not a branch in CVS. Select the 'Head' radio button to build the trunk.
-CVSSCM.HeadIsNotTag=Technically, HEAD is not a tag in CVS. Select the 'Head' radio button to build the trunk.
+CVSSCM.HeadIsNotTag=Technically, HEAD is not a {0} in CVS. Select the 'Head' radio button to build the trunk.
 CVSSCM.InvalidCvsroot=Invalid CVSROOT string
 CVSSCM.MissingCvsroot=CVSROOT is mandatory
 CVSSCM.NoSuchJobExists=No such job exists: {0}
 CVSSCM.NoUpstreamBuildFound=Upstream build not found for: {0}
 CVSSCM.NoValidTagNameGivenFor=No valid tag name given for {0} : {1}
 CVSSCM.PasswordNotSet=It doesn''t look like this CVSROOT has its password set.
-CVSSCM.TagContainsIllegalChar=Tag contains illegal ''{0}'' character
-CVSSCM.TagIsEmpty=Tag is empty
-CVSSCM.TagNeedsToStartWithAlphabet=Tag needs to start with alphabet
 CVSSCM.TagThisBuild=Tag this build
 CVSSCM.TaggingFailed=tagging failed
 CVSSCM.TaggingWorkspace=tagging the workspace
 CVSSCM.DisplayName2=CVS tags
 CVSSCM.DisplayName1=CVS tag
 CVSSCM.WorkspaceInconsistent=Workspace is inconsistent with configuration. Scheduling a new build: {0}
 CVSSCM.MissingRemoteName=Remote Name is mandatory
-CVSSCM.TagNameInvalid=Tag Name is Invalid. Tag name cannot be blank, must start with a letter (A-Z) and cannot contain any of '$,.:;@'
+CVSSCM.TagNameInvalid={0} Name is Invalid. {0} name cannot be blank, must start with a letter (A-Z) and cannot contain any of spaces or '$,.:;@'
+CVSSCM.Tag=Tag
+CVSSCM.Branch=Branch