Merged revisions 34000-34001,34005,34007,34038 via svnmerge from

https://www.dev.java.net/svn/hudson/branches/rc

........
  r34000 | kohsuke | 2010-08-20 14:16:13 -0700 (Fri, 20 Aug 2010) | 2 lines
  
  [FIXED HUDSON-7216]
  Fixed a 1.372 regression in handling whitespace and other characters in label names.
........
  r34001 | kohsuke | 2010-08-20 14:26:55 -0700 (Fri, 20 Aug 2010) | 1 line
  
  foo*bar parses just like foo-bar parses OK.
........
  r34005 | kohsuke | 2010-08-20 20:08:26 -0700 (Fri, 20 Aug 2010) | 1 line
  
  [maven-release-plugin] prepare release hudson-1_373
........
  r34007 | kohsuke | 2010-08-20 20:08:40 -0700 (Fri, 20 Aug 2010) | 1 line
  
  [maven-release-plugin] prepare for next development iteration
........
  r34038 | kohsuke | 2010-08-23 11:02:55 -0700 (Mon, 23 Aug 2010) | 1 line
  
  updated changelog as a part of the release
........


git-svn-id: https://hudson.dev.java.net/svn/hudson/trunk/hudson/main@34118 71c3de6d-444a-0410-be80-ed276b4c234a

cli/pom.xml

@@ -4,7 +4,7 @@
   <parent>
     <artifactId>pom</artifactId>
     <groupId>org.jvnet.hudson.main</groupId>
-    <version>1.373-SNAPSHOT</version>
+    <version>1.374-SNAPSHOT</version>
   </parent>
   <artifactId>cli</artifactId>
   <name>Hudson CLI</name>

core/pom.xml

@@ -28,7 +28,7 @@ THE SOFTWARE.
   <parent>
     <groupId>org.jvnet.hudson.main</groupId>
     <artifactId>pom</artifactId>
-    <version>1.373-SNAPSHOT</version>
+    <version>1.374-SNAPSHOT</version>
     <relativePath>../pom.xml</relativePath>
   </parent>
 

core/src/main/grammar/labelExpr.g

@@ -76,8 +76,9 @@ options { generateAmbigWarnings=false; }
   : LPAREN l=term1 RPAREN
     { l=l.paren(); }
   | a:ATOM
-    { LabelAtom.isValidName(a.getText()) }?
     { l=LabelAtom.get(a.getText()); }
+  | s:STRINGLITERAL
+    { l=LabelAtom.get(hudson.util.QuotedStringTokenizer.unquote(s.getText())); }
   ;
 
 class LabelExpressionLexer extends Lexer;
@@ -92,7 +93,7 @@ RPAREN: ")";
 
 protected
 IDENTIFIER_PART
-    :   ~( '&' | '|' | '!' | '<' | '>' | '(' | ')' | ' ' | '\t')
+    :   ~( '&' | '|' | '!' | '<' | '>' | '(' | ')' | ' ' | '\t' | '\"' | '\'' )
     ;
 
 ATOM
@@ -104,3 +105,11 @@ WS
   : (' '|'\t')+
     { $setType(Token.SKIP); }
   ;
+
+STRINGLITERAL
+    :   '"'
+        ( '\\' ( 'b' | 't' | 'n' | 'f' | 'r' | '\"' | '\'' | '\\' )   /* escape */
+        |  ~( '\\' | '"' | '\r' | '\n' )
+        )*
+        '"'
+    ;

core/src/main/java/hudson/model/AbstractProject.java

@@ -43,6 +43,8 @@ import hudson.model.Fingerprint.RangeSet;
 import hudson.model.Queue.Executable;
 import hudson.model.Queue.WaitingItem;
 import hudson.model.RunMap.Constructor;
+import hudson.model.labels.LabelAtom;
+import hudson.model.labels.LabelExpression;
 import hudson.model.queue.CauseOfBlockage;
 import hudson.scm.ChangeLogSet;
 import hudson.scm.ChangeLogSet.Entry;
@@ -299,8 +301,14 @@ public abstract class AbstractProject<P extends AbstractProject<P,R>,R extends A
      * Gets the textual representation of the assigned label as it was entered by the user.
      */
     public String getAssignedLabelString() {
-        if (canRoam)    return null;
-        return assignedNode;
+        if (canRoam || assignedNode==null)    return null;
+        try {
+            LabelExpression.parseExpression(assignedNode);
+            return assignedNode;
+        } catch (ANTLRException e) {
+            // must be old label or host name that includes whitespace or other unsafe chars
+            return LabelAtom.escape(assignedNode);
+        }
     }
 
     /**
@@ -313,7 +321,7 @@ public abstract class AbstractProject<P extends AbstractProject<P,R>,R extends A
         } else {
             canRoam = false;
             if(l==Hudson.getInstance().getSelfLabel())  assignedNode = null;
-            else                                        assignedNode = l.getName();
+            else                                        assignedNode = l.getExpression();
         }
         save();
     }

core/src/main/java/hudson/model/Hudson.java

@@ -76,7 +76,6 @@ import hudson.remoting.LocalChannel;
 import hudson.remoting.VirtualChannel;
 import hudson.scm.RepositoryBrowser;
 import hudson.scm.SCM;
-import hudson.scm.SCMDescriptor;
 import hudson.search.CollectionSearchIndex;
 import hudson.search.SearchIndexBuilder;
 import hudson.security.ACL;
@@ -105,8 +104,6 @@ import hudson.tasks.BuildWrapper;
 import hudson.tasks.Builder;
 import hudson.tasks.Mailer;
 import hudson.tasks.Publisher;
-import hudson.tools.ToolDescriptor;
-import hudson.tools.ToolInstallation;
 import hudson.triggers.Trigger;
 import hudson.triggers.TriggerDescriptor;
 import hudson.util.AdministrativeError;
@@ -1378,7 +1375,8 @@ public final class Hudson extends Node implements ItemGroup<TopLevelItem>, Stapl
             try {
                 labels.putIfAbsent(expr,Label.parseExpression(expr));
             } catch (ANTLRException e) {
-                throw new IllegalArgumentException("Invalid label expression: "+expr,e);
+                // laxly accept it as a single label atom for backward compatibility
+                return getLabelAtom(expr);
             }
         }
     }
@@ -1388,8 +1386,6 @@ public final class Hudson extends Node implements ItemGroup<TopLevelItem>, Stapl
      */
     public LabelAtom getLabelAtom(String name) {
         if (name==null)  return null;
-        if (!LabelAtom.isValidName(name))
-            throw new IllegalArgumentException("Illegal character in a token: "+name);
 
         while(true) {
             Label l = labels.get(name);

core/src/main/java/hudson/model/Label.java

@@ -34,6 +34,7 @@ import hudson.model.labels.LabelExpressionParser;
 import hudson.model.labels.LabelOperatorPrecedence;
 import hudson.slaves.NodeProvisioner;
 import hudson.slaves.Cloud;
+import hudson.util.QuotedStringTokenizer;
 import hudson.util.VariableResolver;
 import org.kohsuke.stapler.export.Exported;
 import org.kohsuke.stapler.export.ExportedBean;
@@ -62,6 +63,9 @@ import com.thoughtworks.xstream.io.HierarchicalStreamReader;
  */
 @ExportedBean
 public abstract class Label extends Actionable implements Comparable<Label>, ModelObject {
+    /**
+     * Display name of this label.
+     */
     protected transient final String name;
     private transient volatile Set<Node> nodes;
     private transient volatile Set<Cloud> clouds;
@@ -92,15 +96,26 @@ public abstract class Label extends Actionable implements Comparable<Label>, Mod
         this.nodeProvisioner = new NodeProvisioner(this, loadStatistics);
     }
 
+    /**
+     * Alias for {@link #getDisplayName()}.
+     */
     @Exported
-    public String getName() {
-        return name;
+    public final String getName() {
+        return getDisplayName();
     }
 
+    /**
+     * Returns a human-readable text that represents this label.
+     */
     public String getDisplayName() {
         return name;
     }
 
+    /**
+     * Returns a label expression that represents this label.
+     */
+    public abstract String getExpression();
+
     /**
      * Relative URL from the context path, that ends with '/'.
      */
@@ -425,7 +440,7 @@ public abstract class Label extends Actionable implements Comparable<Label>, Mod
 
         public void marshal(Object source, HierarchicalStreamWriter writer, MarshallingContext context) {
             Label src = (Label) source;
-            writer.setValue(src.getName());
+            writer.setValue(src.getExpression());
         }
 
         public Object unmarshal(HierarchicalStreamReader reader, final UnmarshallingContext context) {
@@ -447,7 +462,7 @@ public abstract class Label extends Actionable implements Comparable<Label>, Mod
         Set<LabelAtom> r = new TreeSet<LabelAtom>();
         labels = fixNull(labels);
         if(labels.length()>0)
-            for( String l : labels.split(" +"))
+            for( String l : new QuotedStringTokenizer(labels).toArray())
                 r.add(Hudson.getInstance().getLabelAtom(l));
         return r;
     }

core/src/main/java/hudson/model/labels/LabelAtom.java

@@ -31,9 +31,11 @@ import hudson.model.Descriptor.FormException;
 import hudson.model.Failure;
 import hudson.model.Hudson;
 import hudson.model.Label;
+import hudson.model.Messages;
 import hudson.model.Saveable;
 import hudson.model.listeners.SaveableListener;
 import hudson.util.DescribableList;
+import hudson.util.QuotedStringTokenizer;
 import hudson.util.VariableResolver;
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.StaplerResponse;
@@ -66,6 +68,14 @@ public class LabelAtom extends Label implements Saveable {
         load();
     }
 
+    /**
+     * If the label contains 'unsafe' chars, escape them.
+     */
+    @Override
+    public String getExpression() {
+        return escape(name);
+    }
+
     /**
      * {@inheritDoc}
      *
@@ -189,14 +199,26 @@ public class LabelAtom extends Label implements Saveable {
         return Hudson.getInstance().getLabelAtom(l);
     }
 
-    public static boolean isValidName(String name) {
+    public static boolean needsEscape(String name) {
         try {
             Hudson.checkGoodName(name);
-            return true;
-        } catch (Failure failure) {
+            // additional restricted chars
+            for( int i=0; i<name.length(); i++ ) {
+                char ch = name.charAt(i);
+                if(" ()\t\n".indexOf(ch)!=-1)
+                    return true;
+            }
             return false;
+        } catch (Failure failure) {
+            return true;
         }
     }
 
+    public static String escape(String name) {
+        if (needsEscape(name))
+            return QuotedStringTokenizer.quote(name);
+        return name;
+    }
+
     private static final Logger LOGGER = Logger.getLogger(LabelAtom.class.getName());
 }

core/src/main/java/hudson/model/labels/LabelExpression.java

@@ -37,6 +37,11 @@ public abstract class LabelExpression extends Label {
         super(name);
     }
 
+    @Override
+    public String getExpression() {
+        return getDisplayName();
+    }
+
     public static class Not extends LabelExpression {
         private final Label base;
 
@@ -63,7 +68,7 @@ public abstract class LabelExpression extends Label {
         private final Label base;
 
         public Paren(Label base) {
-            super('('+base.getName()+')');
+            super('('+base.getExpression()+')');
             this.base = base;
         }
 
@@ -83,8 +88,8 @@ public abstract class LabelExpression extends Label {
      */
     static String paren(LabelOperatorPrecedence op, Label l) {
         if (op.compareTo(l.precedence())<0)
-            return '('+l.getName()+')';
-        return l.getName();
+            return '('+l.getExpression()+')';
+        return l.getExpression();
     }
 
     public static abstract class Binary extends LabelExpression {

core/src/main/resources/hudson/model/AbstractProject/help-assignedLabelString.html

@@ -45,4 +45,8 @@
     <p>
     All operators are left-associative (i.e., a->b->c &lt;-> (a->b)->c )
     An expression can contain whitespace for better readability, and it'll be ignored.
+
+    <p>
+    Label names or slave names can be quoted if they contain unsafe characters. For example,
+    <tt>"hudson-solaris (Solaris)" || "Windows 2008"</tt>
 </div>
\ No newline at end of file

debian/debian/changelog

+hudson (1.373) unstable; urgency=low
+
+  * See http://hudson.dev.java.net/changelog.html for more details.
+
+ -- Kohsuke Kawaguchi <kk@kohsuke.org>  Mon, 23 Aug 2010 11:01:23 -0700
+
 hudson (1.372) unstable; urgency=low
 
   * See http://hudson.dev.java.net/changelog.html for more details.

maven-agent/pom.xml

@@ -27,7 +27,7 @@ THE SOFTWARE.
   <parent>
     <groupId>org.jvnet.hudson.main</groupId>
     <artifactId>pom</artifactId>
-    <version>1.373-SNAPSHOT</version>
+    <version>1.374-SNAPSHOT</version>
     <relativePath>../pom.xml</relativePath>
   </parent>
   

maven-interceptor/pom.xml

@@ -27,7 +27,7 @@ THE SOFTWARE.
   <parent>
     <groupId>org.jvnet.hudson.main</groupId>
     <artifactId>pom</artifactId>
-    <version>1.373-SNAPSHOT</version>
+    <version>1.374-SNAPSHOT</version>
     <relativePath>../pom.xml</relativePath>
   </parent>
   

maven-plugin/pom.xml

@@ -27,7 +27,7 @@ THE SOFTWARE.
   <parent>
     <groupId>org.jvnet.hudson.main</groupId>
     <artifactId>pom</artifactId>
-    <version>1.373-SNAPSHOT</version>
+    <version>1.374-SNAPSHOT</version>
   </parent>
 
   <artifactId>maven-plugin</artifactId>

pom.xml

@@ -33,7 +33,7 @@ THE SOFTWARE.
   
   <groupId>org.jvnet.hudson.main</groupId>
   <artifactId>pom</artifactId>
-  <version>1.373-SNAPSHOT</version>
+  <version>1.374-SNAPSHOT</version>
   <packaging>pom</packaging>
   
   <name>Hudson main module</name>

remoting/pom.xml

@@ -27,7 +27,7 @@ THE SOFTWARE.
   <parent>
     <groupId>org.jvnet.hudson.main</groupId>
     <artifactId>pom</artifactId>
-    <version>1.373-SNAPSHOT</version>
+    <version>1.374-SNAPSHOT</version>
     <relativePath>../pom.xml</relativePath>
   </parent>
   

test/pom.xml

@@ -27,7 +27,7 @@ THE SOFTWARE.
   <parent>
     <artifactId>pom</artifactId>
     <groupId>org.jvnet.hudson.main</groupId>
-    <version>1.373-SNAPSHOT</version>
+    <version>1.374-SNAPSHOT</version>
   </parent>
   <modelVersion>4.0.0</modelVersion>
   <groupId>org.jvnet.hudson.main</groupId>

test/src/main/java/org/jvnet/hudson/test/HudsonTestCase.java

@@ -554,7 +554,7 @@ public abstract class HudsonTestCase extends TestCase implements RootAction {
     }
 
     public DumbSlave createSlave(Label l, EnvVars env) throws Exception {
-        return createSlave(l==null ? null : l.getName(), env);
+        return createSlave(l==null ? null : l.getExpression(), env);
     }
 
     /**

test/src/test/java/hudson/model/labels/LabelExpressionTest.java

@@ -26,15 +26,21 @@ package hudson.model.labels;
 import antlr.ANTLRException;
 import hudson.Launcher;
 import hudson.model.AbstractBuild;
+import hudson.model.AbstractProject;
 import hudson.model.BuildListener;
 import hudson.model.FreeStyleBuild;
 import hudson.model.FreeStyleProject;
+import hudson.model.Label;
+import hudson.model.Node.Mode;
 import hudson.slaves.DumbSlave;
+import hudson.slaves.RetentionStrategy;
 import org.jvnet.hudson.test.HudsonTestCase;
 import org.jvnet.hudson.test.SequenceLock;
 import org.jvnet.hudson.test.TestBuilder;
 
 import java.io.IOException;
+import java.lang.reflect.Field;
+import java.util.Collections;
 import java.util.concurrent.Future;
 
 /**
@@ -135,7 +141,40 @@ public class LabelExpressionTest extends HudsonTestCase {
 
     public void testParserError() throws Exception {
         parseShouldFail("foo bar");
-        parseShouldFail("foo*bar");
+        parseShouldFail("foo (bar)");
+    }
+
+    public void testLaxParsing() {
+        // this should parse as an atom
+        LabelAtom l = (LabelAtom)hudson.getLabel("lucene.zones.apache.org (Solaris 10)");
+        assertEquals(l.getName(),"lucene.zones.apache.org (Solaris 10)");
+        assertEquals(l.getExpression(),"\"lucene.zones.apache.org (Solaris 10)\"");
+    }
+
+    public void testDataCompatibilityWithHostNameWithWhitespace() throws Exception {
+        DumbSlave slave = new DumbSlave("abc def (xyz) : test", "dummy",
+                createTmpDir().getPath(), "1", Mode.NORMAL, "", createComputerLauncher(null), RetentionStrategy.NOOP, Collections.EMPTY_LIST);
+        hudson.addNode(slave);
+
+
+        FreeStyleProject p = createFreeStyleProject();
+        p.setAssignedLabel(hudson.getLabel("abc def"));
+        assertEquals("abc def",p.getAssignedLabel().getName());
+        assertEquals("\"abc def\"",p.getAssignedLabel().getExpression());
+
+        // expression should be persisted, not the name
+        Field f = AbstractProject.class.getDeclaredField("assignedNode");
+        f.setAccessible(true);
+        assertEquals("\"abc def\"",f.get(p));
+
+        // but if the name is set, we'd still like to parse it
+        f.set(p,"a:b c");
+        assertEquals("a:b c",p.getAssignedLabel().getName());
+    }
+
+    public void testQuote() {
+        Label l = hudson.getLabel("\"abc\\\\\\\"def\"");
+        assertEquals("abc\\\"def",l.getName());
     }
 
     /**
@@ -155,7 +194,7 @@ public class LabelExpressionTest extends HudsonTestCase {
     private void parseShouldFail(String expr) {
         try {
             LabelExpression.parseExpression(expr);
-            fail();
+            fail(expr + " should fail to parse");
         } catch (ANTLRException e) {
             // expected
         }

ui-samples-plugin/pom.xml

@@ -27,7 +27,7 @@ THE SOFTWARE.
   <parent>
     <groupId>org.jvnet.hudson.main</groupId>
     <artifactId>pom</artifactId>
-    <version>1.373-SNAPSHOT</version>
+    <version>1.374-SNAPSHOT</version>
   </parent>
 
   <artifactId>ui-samples-plugin</artifactId>

war/pom.xml

@@ -27,7 +27,7 @@ THE SOFTWARE.
   <parent>
     <groupId>org.jvnet.hudson.main</groupId>
     <artifactId>pom</artifactId>
-    <version>1.373-SNAPSHOT</version>
+    <version>1.374-SNAPSHOT</version>
     <relativePath>../pom.xml</relativePath>
   </parent>