From 0c46701287ebfa1719c66dcdbcbba57f36c2ac59 Mon Sep 17 00:00:00 2001 From: Vojtech Juranek Date: Tue, 5 Mar 2013 20:54:26 +0100 Subject: [PATCH 1/5] [FIXED JENKINS-17084] Changelog should produce output even if some changelog annotator fails --- .../main/java/hudson/scm/ChangeLogSet.java | 4 +- .../java/hudson/scm/ChangeLogSetTest.java | 47 +++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 test/src/test/java/hudson/scm/ChangeLogSetTest.java diff --git a/core/src/main/java/hudson/scm/ChangeLogSet.java b/core/src/main/java/hudson/scm/ChangeLogSet.java index 5b0f342b35..a16b6130e2 100644 --- a/core/src/main/java/hudson/scm/ChangeLogSet.java +++ b/core/src/main/java/hudson/scm/ChangeLogSet.java @@ -211,7 +211,9 @@ public abstract class ChangeLogSet 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); diff --git a/test/src/test/java/hudson/scm/ChangeLogSetTest.java b/test/src/test/java/hudson/scm/ChangeLogSetTest.java new file mode 100644 index 0000000000..c17d87d3eb --- /dev/null +++ b/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(); + } + + } +} From 42734675f4825e4856f67945981ce44d2190dbd6 Mon Sep 17 00:00:00 2001 From: Vojtech Juranek Date: Fri, 8 Mar 2013 14:04:25 +0100 Subject: [PATCH 2/5] [FIXED JENKINS-17128] Unquote labels before processing --- core/src/main/java/jenkins/model/Jenkins.java | 1 + .../test/java/hudson/model/labels/LabelExpressionTest.java | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java index 8ba08f4cae..1c0b3e339d 100755 --- a/core/src/main/java/jenkins/model/Jenkins.java +++ b/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) diff --git a/test/src/test/java/hudson/model/labels/LabelExpressionTest.java b/test/src/test/java/hudson/model/labels/LabelExpressionTest.java index 4239b75ba7..60c85d80a8 100644 --- a/test/src/test/java/hudson/model/labels/LabelExpressionTest.java +++ b/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()); } /** From 8284013b6072d88bf6928b7a2994cbf2a447983a Mon Sep 17 00:00:00 2001 From: Vojtech Juranek Date: Fri, 22 Mar 2013 12:22:39 +0100 Subject: [PATCH 3/5] Add changelog recored for pull #729 and #730 --- changelog.html | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/changelog.html b/changelog.html index 26893c6868..b7b7b7556b 100644 --- a/changelog.html +++ b/changelog.html @@ -70,6 +70,12 @@ Upcoming changes
  • Performance improvement in master/slave communication throughput (issue 7813) +
  • + Quoted label expression can result into dead executors (throwing exception) + (issue 17128) +
  • + ChangeLog should produce some output even if some (plugin) annotator fails + (issue 17084) From d8b29df3558724090efaf18326937075c25ba7f3 Mon Sep 17 00:00:00 2001 From: Seiji Sogabe Date: Sat, 23 Mar 2013 03:40:43 +0900 Subject: [PATCH 4/5] [FIXED JENKINS-16608] View name should not allow "..". --- changelog.html | 3 + core/src/main/java/jenkins/model/Jenkins.java | 2 + .../hudson/model/Messages.properties | 2 + .../hudson/model/Messages_ja.properties | 1 + test/src/test/java/hudson/model/ViewTest.java | 55 ++++++++++++------- 5 files changed, 44 insertions(+), 19 deletions(-) diff --git a/changelog.html b/changelog.html index b7b7b7556b..660ede45c6 100644 --- a/changelog.html +++ b/changelog.html @@ -76,6 +76,9 @@ Upcoming changes
  • ChangeLog should produce some output even if some (plugin) annotator fails (issue 17084) +
  • + View name should not allow "..". + (issue 16608) diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java index 8528206628..8d0802736d 100755 --- a/core/src/main/java/jenkins/model/Jenkins.java +++ b/core/src/main/java/jenkins/model/Jenkins.java @@ -2922,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 Date: Fri, 22 Mar 2013 15:42:23 -0400 Subject: [PATCH 5/5] Test & I18N for checkSanity. --- core/src/main/java/hudson/scheduler/CronTab.java | 3 +-- .../resources/hudson/scheduler/Messages.properties | 3 ++- core/src/test/java/hudson/scheduler/CronTabTest.java | 11 +++++++++++ 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/hudson/scheduler/CronTab.java b/core/src/main/java/hudson/scheduler/CronTab.java index 52e7d602fe..bc99770a30 100644 --- a/core/src/main/java/hudson/scheduler/CronTab.java +++ b/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; } diff --git a/core/src/main/resources/hudson/scheduler/Messages.properties b/core/src/main/resources/hudson/scheduler/Messages.properties index c49b377bb4..bd23244270 100644 --- a/core/src/main/resources/hudson/scheduler/Messages.properties +++ b/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} \ No newline at end of file +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} diff --git a/core/src/test/java/hudson/scheduler/CronTabTest.java b/core/src/test/java/hudson/scheduler/CronTabTest.java index 5265bc79c2..21ecc523bd 100644 --- a/core/src/test/java/hudson/scheduler/CronTabTest.java +++ b/core/src/test/java/hudson/scheduler/CronTabTest.java @@ -178,6 +178,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}. */