Merge branch 'master' into cron-hash-JENKINS-17311

changelog.html

@@ -70,6 +70,15 @@ Upcoming changes</a>
   <li class='major rfe'>
     Performance improvement in master/slave communication throughput
     (<a href="https://issues.jenkins-ci.org/browse/JENKINS-7813">issue 7813</a>)
+  <li class=bug>
+    Quoted label expression can result into dead executors (throwing exception)
+    (<a href="https://issues.jenkins-ci.org/browse/JENKINS-17128">issue 17128</a>)
+  <li class=bug>
+    ChangeLog should produce some output even if some (plugin) annotator fails
+    (<a href="https://issues.jenkins-ci.org/browse/JENKINS-17084">issue 17084</a>)
+  <li class=bug>
+    View name should not allow "..".    
+    (<a href="https://issues.jenkins-ci.org/browse/JENKINS-16608">issue 16608</a>)
 </ul>
 </div><!--=TRUNK-END=-->
 

core/src/main/java/hudson/scheduler/CronTab.java

@@ -420,8 +420,7 @@ public final class CronTab {
                     // this rank has a sparse entry.
                     // if we have a sparse rank, one of them better be the left-most.
                     if(i>0)
-                        return "Do you really mean \"every minute\" when you say \""+spec+"\"? "+
-                                "Perhaps you meant \"0 "+spec.substring(spec.indexOf(' ')+1)+"\"";
+                        return Messages.CronTab_do_you_really_mean_every_minute_when_you(spec, "0 " + spec.substring(spec.indexOf(' ')+1));
                     // once we find a sparse rank, upper ranks don't matter
                     return null;
                 }

core/src/main/java/hudson/scm/ChangeLogSet.java

@@ -211,7 +211,9 @@ public abstract class ChangeLogSet<T extends ChangeLogSet.Entry> implements Iter
                 try {
                     a.annotate(parent.build,this,markup);
                 } catch(Exception e) {
-                    LOGGER.fine("ChangeLogAnnotator " + a.toString() + " failed to annotate message '" + getMsg() + "'; " + e.getMessage());
+                    LOGGER.info("ChangeLogAnnotator " + a.toString() + " failed to annotate message '" + getMsg() + "'; " + e.getMessage());
+                } catch(Error e) {
+                    LOGGER.severe("ChangeLogAnnotator " + a.toString() + " failed to annotate message '" + getMsg() + "'; " + e.getMessage());
                 }
 
             return markup.toString(false);

core/src/main/java/jenkins/model/Jenkins.java

@@ -1570,6 +1570,7 @@ public class Jenkins extends AbstractCIBase implements ModifiableTopLevelItemGro
      */
     public Label getLabel(String expr) {
         if(expr==null)  return null;
+        expr = hudson.util.QuotedStringTokenizer.unquote(expr);
         while(true) {
             Label l = labels.get(expr);
             if(l!=null)
@@ -2921,6 +2922,8 @@ public class Jenkins extends AbstractCIBase implements ModifiableTopLevelItemGro
         if(name==null || name.length()==0)
             throw new Failure(Messages.Hudson_NoName());
 
+        if("..".equals(name.trim())) 
+            throw new Failure(Messages.Jenkins_NotAllowedName(".."));
         for( int i=0; i<name.length(); i++ ) {
             char ch = name.charAt(i);
             if(Character.isISOControl(ch)) {

core/src/main/resources/hudson/model/Messages.properties

@@ -344,3 +344,5 @@ BuildAuthorizationToken.InvalidTokenProvided=Invalid token provided.
 
 Jenkins.CheckDisplayName.NameNotUniqueWarning=The display name, "{0}", is used as a name by a job and could cause confusing search results.
 Jenkins.CheckDisplayName.DisplayNameNotUniqueWarning=The display name, "{0}", is already in use by another job and could cause confusion and delay.
+
+Jenkins.NotAllowedName="{0}" is not allowed name

core/src/main/resources/hudson/model/Messages_ja.properties

@@ -333,3 +333,4 @@ ManageJenkinsAction.DisplayName=Jenkins\u306e\u7ba1\u7406
 Jenkins.CheckDisplayName.NameNotUniqueWarning=\u8868\u793a\u7528\u30d7\u30ed\u30b8\u30a7\u30af\u30c8\u540d "{0}" \u306f\u30d7\u30ed\u30b8\u30a7\u30af\u30c8\u540d\u3068\u3057\u3066\u65e2\u306b\u4f7f\u7528\u3055\u308c\u3066\u3044\u308b\u305f\u3081\u3001\u691c\u7d22\u7d50\u679c\u3067\u533a\u5225\u3067\u304d\u306a\u3044\u3053\u3068\u304c\u3042\u308a\u307e\u3059\u3002
 Jenkins.CheckDisplayName.DisplayNameNotUniqueWarning=\u8868\u793a\u7528\u30d7\u30ed\u30b8\u30a7\u30af\u30c8\u540d "{0}" \u306f\u4ed6\u306e\u30d7\u30ed\u30b8\u30a7\u30af\u30c8\u3067\u4f7f\u7528\u3055\u308c\u3066\u3044\u308b\u305f\u3081\u3001 \u533a\u5225\u3067\u304d\u306a\u3044\u3053\u3068\u304c\u3042\u308a\u307e\u3059\u3002
 
+Jenkins.NotAllowedName="{0}" \u306f\u8a31\u53ef\u3055\u308c\u306a\u3044\u540d\u524d\u3067\u3059\u3002

core/src/main/resources/hudson/scheduler/Messages.properties

@@ -23,4 +23,5 @@
 BaseParser.StartEndReversed=You mean {0}-{1}?
 BaseParser.MustBePositive=step must be positive, but found {0}
 BaseParser.OutOfRange={0} is an invalid value. Must be within {1} and {2}
-CronTabList.InvalidInput=Invalid input: "{0}": {1}
+CronTab.do_you_really_mean_every_minute_when_you=Do you really mean "every minute" when you say "{0}"? Perhaps you meant "{1}"
+CronTabList.InvalidInput=Invalid input: "{0}": {1}

core/src/test/java/hudson/scheduler/CronTabTest.java

@@ -173,6 +173,17 @@ public class CronTabTest {
         compare(answer,x.floor(c));
     }
 
+    @Test public void checkSanity() throws Exception {
+        assertEquals(Messages.CronTab_do_you_really_mean_every_minute_when_you("* * * * *", "0 * * * *"), new CronTab("* * * * *").checkSanity());
+        assertEquals(null, new CronTab("0 * * * *").checkSanity());
+        assertEquals(null, new CronTab("0 3 * * *").checkSanity());
+        assertEquals(null, new CronTab("H H(0-2) * * *", Hash.from("stuff")).checkSanity());
+        assertEquals(Messages.CronTab_do_you_really_mean_every_minute_when_you("* 0 * * *", "0 0 * * *"), new CronTab("* 0 * * *").checkSanity());
+        assertEquals(Messages.CronTab_do_you_really_mean_every_minute_when_you("* 6,18 * * *", "0 6,18 * * *"), new CronTab("* 6,18 * * *").checkSanity());
+        // dubious; could be improved:
+        assertEquals(Messages.CronTab_do_you_really_mean_every_minute_when_you("* * 3 * *", "0 * 3 * *"), new CronTab("* * 3 * *").checkSanity());
+    }
+
     /**
      * Humans can't easily see difference in two {@link Calendar}s, do help the diagnosis by using {@link DateFormat}. 
      */

test/src/test/java/hudson/model/ViewTest.java

@@ -35,6 +35,9 @@ import org.jvnet.hudson.test.HudsonTestCase;
 import org.w3c.dom.Text;
 
 import static hudson.model.Messages.Hudson_ViewName;
+import static junit.framework.TestCase.assertEquals;
+import static junit.framework.TestCase.assertNotNull;
+import static junit.framework.TestCase.fail;
 
 /**
  * @author Kohsuke Kawaguchi
@@ -46,7 +49,7 @@ public class ViewTest extends HudsonTestCase {
         assertNotNull(new WebClient().goTo("/").getWebResponse().getResponseHeaderValue("X-Hudson"));
     }
 
-	/**
+    /**
      * Creating two views with the same name.
      */
     @Email("http://d.hatena.ne.jp/ssogabe/20090101/1230744150")
@@ -64,7 +67,7 @@ public class ViewTest extends HudsonTestCase {
             submit(form);
             fail("shouldn't be allowed to create two views of the same name.");
         } catch (FailingHttpStatusCodeException e) {
-            assertEquals(400,e.getStatusCode());
+            assertEquals(400, e.getStatusCode());
         }
     }
 
@@ -100,24 +103,24 @@ public class ViewTest extends HudsonTestCase {
         assertEquals(((ProxyView) proxyView).getProxiedViewName(), "listView");
         assertEquals(((ProxyView) proxyView).getProxiedView(), listView);
     }
-    
-    public void testDeleteView() throws Exception {
-    	WebClient wc = new WebClient();
 
-    	ListView v = new ListView("list", jenkins);
-		jenkins.addView(v);
-    	HtmlPage delete = wc.getPage(v, "delete");
-    	submit(delete.getFormByName("delete"));
-    	assertNull(jenkins.getView("list"));
-    	
-    	User user = User.get("user", true);
-    	MyViewsProperty p = user.getProperty(MyViewsProperty.class);
-    	v = new ListView("list", p);
-		p.addView(v);
-    	delete = wc.getPage(v, "delete");
-    	submit(delete.getFormByName("delete"));
-    	assertNull(p.getView("list"));
-    	
+    public void testDeleteView() throws Exception {
+        WebClient wc = new WebClient();
+
+        ListView v = new ListView("list", jenkins);
+        jenkins.addView(v);
+        HtmlPage delete = wc.getPage(v, "delete");
+        submit(delete.getFormByName("delete"));
+        assertNull(jenkins.getView("list"));
+
+        User user = User.get("user", true);
+        MyViewsProperty p = user.getProperty(MyViewsProperty.class);
+        v = new ListView("list", p);
+        p.addView(v);
+        delete = wc.getPage(v, "delete");
+        submit(delete.getFormByName("delete"));
+        assertNull(p.getView("list"));
+
     }
 
     @Bug(9367)
@@ -137,4 +140,18 @@ public class ViewTest extends HudsonTestCase {
         webClient.setJavaScriptEnabled(false);
         assertAllImageLoadSuccessfully(webClient.goTo("asynchPeople"));
     }
+
+    @Bug(16608)
+    public void testNotAlloedName() throws Exception {
+        HtmlForm form = new WebClient().goTo("newView").getFormByName("createItem");
+        form.getInputByName("name").setValueAttribute("..");
+        form.getRadioButtonsByName("mode").get(0).setChecked(true);
+
+        try {
+            submit(form);
+            fail("\"..\" should not be allowed.");
+        } catch (FailingHttpStatusCodeException e) {
+            assertEquals(400, e.getStatusCode());
+        }
+    }
 }

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

@@ -196,6 +196,10 @@ public class LabelExpressionTest extends HudsonTestCase {
     public void testQuote() {
         Label l = jenkins.getLabel("\"abc\\\\\\\"def\"");
         assertEquals("abc\\\"def",l.getName());
+        
+        l = jenkins.getLabel("label1||label2"); // create label expression
+        l = jenkins.getLabel("\"label1||label2\"");
+        assertEquals("label1||label2",l.getName());
     }
 
     /**

test/src/test/java/hudson/scm/ChangeLogSetTest.java

@@ -0,0 +1,47 @@
+package hudson.scm;
+
+import hudson.Extension;
+import hudson.MarkupText;
+import hudson.model.AbstractBuild;
+import hudson.scm.ChangeLogSet.Entry;
+
+import org.jvnet.hudson.test.Bug;
+import org.jvnet.hudson.test.FakeChangeLogSCM.EntryImpl;
+import org.jvnet.hudson.test.HudsonTestCase;
+
+public class ChangeLogSetTest extends HudsonTestCase {
+
+    @Bug(17084)
+    public void testCatchingExceptionDuringAnnotation() throws Exception {
+        EntryImpl change = new EntryImpl();
+        change.setParent(ChangeLogSet.createEmpty(null)); // otherwise test would actually test only NPE thrown when accessing paret.build 
+        boolean notCaught = false;
+        try {
+            change.getMsgAnnotated();
+        } catch (Throwable t) {
+            notCaught = true;
+        }
+        assertEquals((new EntryImpl()).getMsg(), change.getMsg());
+        assertEquals(false, notCaught);
+    }
+    
+    @Extension
+    public static final class ThrowExceptionChangeLogAnnotator extends ChangeLogAnnotator {
+        
+        @Override
+        public void annotate(AbstractBuild<?,?> build, Entry change, MarkupText text ) {
+            throw new RuntimeException();
+        }
+        
+    }
+    
+    @Extension
+    public static final class ThrowErrorChangeLogAnnotator extends ChangeLogAnnotator {
+        
+        @Override
+        public void annotate(AbstractBuild<?,?> build, Entry change, MarkupText text ) {
+            throw new Error();
+        }
+        
+    }
+}