From 7c4f6b7ef433f4a3d85707096f9e5f8b366f232a Mon Sep 17 00:00:00 2001 From: Oleg Nenashev Date: Sun, 28 Sep 2014 18:11:57 +0400 Subject: [PATCH 01/40] [FIXED JENKINS-19017] - FileParameter - Handle non-null file parameters as different values The change prevents the issue when Jenkins merges builds with different files coming from one source (It may happen!). Signed-off-by: Oleg Nenashev --- .../java/hudson/model/FileParameterValue.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/hudson/model/FileParameterValue.java b/core/src/main/java/hudson/model/FileParameterValue.java index 090a50aa9f..80c374fcad 100644 --- a/core/src/main/java/hudson/model/FileParameterValue.java +++ b/core/src/main/java/hudson/model/FileParameterValue.java @@ -160,7 +160,8 @@ public class FileParameterValue extends ParameterValue { } /** - * In practice this will always be false, since location should be unique. + * Compares file parameters (existing files will be considered as different). + * Function has been modified in order to avoid JENKINS-19017 issue (wrong merge of builds in the queue). */ @Override public boolean equals(Object obj) { @@ -171,12 +172,13 @@ public class FileParameterValue extends ParameterValue { if (getClass() != obj.getClass()) return false; FileParameterValue other = (FileParameterValue) obj; - if (location == null) { - if (other.location != null) - return false; - } else if (!location.equals(other.location)) - return false; - return true; + + if (location == null && other.location == null) + return true; // Consider null parameters as equal + + //TODO: check fingerprints or checksums to improve the behavior + // Return false even if files are equal + return false; } @Override From 0bd2ea1808c52f82b14a33e61ed70adfb28a8816 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Sun, 12 Oct 2014 20:00:16 -0700 Subject: [PATCH 02/40] the trunk is toward 1.586-SNAPSHOT --- cli/pom.xml | 2 +- core/pom.xml | 2 +- plugins/pom.xml | 8 ++++---- pom.xml | 2 +- test/pom.xml | 2 +- war/pom.xml | 2 +- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/cli/pom.xml b/cli/pom.xml index 756ddb787a..a100512fc7 100644 --- a/cli/pom.xml +++ b/cli/pom.xml @@ -5,7 +5,7 @@ pom org.jenkins-ci.main - 1.585-SNAPSHOT + 1.586-SNAPSHOT cli diff --git a/core/pom.xml b/core/pom.xml index 35378ce66e..e13b04ba6b 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -29,7 +29,7 @@ THE SOFTWARE. org.jenkins-ci.main pom - 1.585-SNAPSHOT + 1.586-SNAPSHOT ../pom.xml diff --git a/plugins/pom.xml b/plugins/pom.xml index d42c2c9d95..55a6a495bf 100644 --- a/plugins/pom.xml +++ b/plugins/pom.xml @@ -11,7 +11,7 @@ org.jenkins-ci.plugins plugin Jenkins plugin POM - 1.585-SNAPSHOT + 1.586-SNAPSHOT pom org.codehaus.woodstox From eaf7a26b37852a5bb6292d07f427f30692d8df3a Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 14 Oct 2014 07:57:23 -0400 Subject: [PATCH 10/40] Factored out common code into applyErrorMessage function. --- .../main/webapp/scripts/hudson-behavior.js | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/war/src/main/webapp/scripts/hudson-behavior.js b/war/src/main/webapp/scripts/hudson-behavior.js index 8946db6662..f6a7107b92 100644 --- a/war/src/main/webapp/scripts/hudson-behavior.js +++ b/war/src/main/webapp/scripts/hudson-behavior.js @@ -167,12 +167,7 @@ var FormChecker = { this.sendRequest(next.url, { method : next.method, onComplete : function(x) { - var i; - next.target.innerHTML = x.status==200 ? x.responseText - : 'ERROR'; - Behaviour.applySubtree(next.target); + applyErrorMessage(next.target, x); FormChecker.inProgress--; FormChecker.schedule(); layoutUpdateCallback.call(); @@ -2194,12 +2189,7 @@ function validateButton(checkUrl,paramList,button) { parameters: parameters, onComplete: function(rsp) { spinner.style.display="none"; - var i; - target.innerHTML = rsp.status==200 ? rsp.responseText - : 'ERROR'; - Behaviour.applySubtree(target); + applyErrorMessage(target, rsp); layoutUpdateCallback.call(); var s = rsp.getResponseHeader("script"); try { @@ -2211,6 +2201,15 @@ function validateButton(checkUrl,paramList,button) { }); } +function applyErrorMessage(elt, rsp) { + var i; + elt.innerHTML = rsp.status==200 ? rsp.responseText + : 'ERROR'; + Behaviour.applySubtree(elt); +} + // create a combobox. // @param idOrField // ID of the element that becomes a combobox, or the field itself. From 9620057a58dc07b52752683a433e200f36d4317d Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 14 Oct 2014 08:16:31 -0400 Subject: [PATCH 11/40] =?UTF-8?q?When=20a=20server=20error=20is=20caught?= =?UTF-8?q?=20by=20applyErrorMessage=20(=E2=80=9CERROR=E2=80=9D=20link),?= =?UTF-8?q?=20do=20not=20render=20the=20whole=20oops.jelly=20page.=20Just?= =?UTF-8?q?=20pick=20out=20the=20=20from?= =?UTF-8?q?=20JENKINS-20772.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../main/webapp/scripts/hudson-behavior.js | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/war/src/main/webapp/scripts/hudson-behavior.js b/war/src/main/webapp/scripts/hudson-behavior.js index f6a7107b92..1bf29852a8 100644 --- a/war/src/main/webapp/scripts/hudson-behavior.js +++ b/war/src/main/webapp/scripts/hudson-behavior.js @@ -2202,11 +2202,22 @@ function validateButton(checkUrl,paramList,button) { } function applyErrorMessage(elt, rsp) { - var i; - elt.innerHTML = rsp.status==200 ? rsp.responseText - : 'ERROR'; + if (rsp.status == 200) { + elt.innerHTML = rsp.responseText; + } else { + var id = 'valerr' + (iota++); + elt.innerHTML = 'ERROR'; + var error = document.getElementById('error-description'); // cf. oops.jelly + if (error) { + var div = document.getElementById(id); + while (div.firstChild) { + div.removeChild(div.firstChild); + } + div.appendChild(error); + } + } Behaviour.applySubtree(elt); } From 95c73c313f64d6a656d993c9ed03184fa95d958f Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 14 Oct 2014 08:32:44 -0400 Subject: [PATCH 12/40] NPE in form validation when configured on a non-AbstractProject, like a workflow job. --- core/src/main/java/hudson/tasks/ArtifactArchiver.java | 3 +++ core/src/main/java/hudson/tasks/Fingerprinter.java | 3 +++ 2 files changed, 6 insertions(+) diff --git a/core/src/main/java/hudson/tasks/ArtifactArchiver.java b/core/src/main/java/hudson/tasks/ArtifactArchiver.java index 00aa79b9b1..53f5d6bedc 100644 --- a/core/src/main/java/hudson/tasks/ArtifactArchiver.java +++ b/core/src/main/java/hudson/tasks/ArtifactArchiver.java @@ -296,6 +296,9 @@ public class ArtifactArchiver extends Recorder implements SimpleBuildStep { * Performs on-the-fly validation on the file mask wildcard. */ public FormValidation doCheckArtifacts(@AncestorInPath AbstractProject project, @QueryParameter String value) throws IOException { + if (project == null) { + return FormValidation.ok(); + } return FilePath.validateFileMask(project.getSomeWorkspace(),value); } diff --git a/core/src/main/java/hudson/tasks/Fingerprinter.java b/core/src/main/java/hudson/tasks/Fingerprinter.java index 7320838ec4..8c00b145b9 100644 --- a/core/src/main/java/hudson/tasks/Fingerprinter.java +++ b/core/src/main/java/hudson/tasks/Fingerprinter.java @@ -260,6 +260,9 @@ public class Fingerprinter extends Recorder implements Serializable, DependencyD } public FormValidation doCheckTargets(@AncestorInPath AbstractProject project, @QueryParameter String value) throws IOException { + if (project == null) { + return FormValidation.ok(); + } return FilePath.validateFileMask(project.getSomeWorkspace(),value); } From 89db519e369c18efef3ca90785264d67fadb9ee4 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 14 Oct 2014 09:40:37 -0400 Subject: [PATCH 13/40] t:listScmBrowsers was not working inside f:dropdownDescriptorSelector. It assumed it was used only inside config-scm.jelly, so assumed it could use ${scmd} rather than the more standard ${descriptor}. --- core/src/main/resources/lib/hudson/listScmBrowsers.jelly | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/src/main/resources/lib/hudson/listScmBrowsers.jelly b/core/src/main/resources/lib/hudson/listScmBrowsers.jelly index b655da6d59..6236f6c369 100644 --- a/core/src/main/resources/lib/hudson/listScmBrowsers.jelly +++ b/core/src/main/resources/lib/hudson/listScmBrowsers.jelly @@ -29,10 +29,11 @@ THE SOFTWARE. form field name. + - + Date: Tue, 14 Oct 2014 11:31:50 -0700 Subject: [PATCH 14/40] Accept "field" specified in the surrounding since this one expects --- core/src/main/resources/lib/form/repeatableHeteroProperty.jelly | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/resources/lib/form/repeatableHeteroProperty.jelly b/core/src/main/resources/lib/form/repeatableHeteroProperty.jelly index b1ab6b739b..77e40529bd 100644 --- a/core/src/main/resources/lib/form/repeatableHeteroProperty.jelly +++ b/core/src/main/resources/lib/form/repeatableHeteroProperty.jelly @@ -65,7 +65,7 @@ THE SOFTWARE. - Date: Tue, 14 Oct 2014 22:19:07 +0200 Subject: [PATCH 15/40] [FIXED JENKINS-3539] Prevent empty file creation if left empty --- .../java/hudson/model/FileParameterValue.java | 2 +- .../java/hudson/model/ParametersTest.java | 23 +++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/hudson/model/FileParameterValue.java b/core/src/main/java/hudson/model/FileParameterValue.java index 090a50aa9f..af4210a37d 100644 --- a/core/src/main/java/hudson/model/FileParameterValue.java +++ b/core/src/main/java/hudson/model/FileParameterValue.java @@ -138,7 +138,7 @@ public class FileParameterValue extends ParameterValue { return new BuildWrapper() { @Override public Environment setUp(AbstractBuild build, Launcher launcher, BuildListener listener) throws IOException, InterruptedException { - if (!StringUtils.isEmpty(location)) { + if (!StringUtils.isEmpty(location) && !StringUtils.isEmpty(file.getName())) { listener.getLogger().println("Copying file to "+location); FilePath locationFilePath = build.getWorkspace().child(location); locationFilePath.getParent().mkdirs(); diff --git a/test/src/test/java/hudson/model/ParametersTest.java b/test/src/test/java/hudson/model/ParametersTest.java index 0cc4044942..f26c7209a7 100644 --- a/test/src/test/java/hudson/model/ParametersTest.java +++ b/test/src/test/java/hudson/model/ParametersTest.java @@ -8,6 +8,8 @@ import com.gargoylesoftware.htmlunit.html.HtmlElement; import com.gargoylesoftware.htmlunit.html.HtmlTextInput; import com.gargoylesoftware.htmlunit.html.HtmlCheckBoxInput; import com.gargoylesoftware.htmlunit.html.HtmlOption; +import org.jvnet.hudson.test.Issue; + import java.util.Set; /** @@ -157,4 +159,25 @@ public class ParametersTest extends HudsonTestCase { assertTrue(sensitiveVars.contains("password")); assertFalse(sensitiveVars.contains("string2")); } + + @Issue("JENKINS-3539") + public void testFileParameterNotSet() throws Exception { + FreeStyleProject project = createFreeStyleProject(); + ParametersDefinitionProperty pdp = new ParametersDefinitionProperty( + new FileParameterDefinition("filename", "description")); + project.addProperty(pdp); + + WebClient wc = new WebClient(); + wc.setThrowExceptionOnFailingStatusCode(false); + HtmlPage page = wc.goTo("/job/" + project.getName() + "/build?delay=0sec"); + HtmlForm form = page.getFormByName("parameters"); + + submit(form); + Queue.Item q = jenkins.getQueue().getItem(project); + if (q != null) q.getFuture().get(); + else Thread.sleep(1000); + + assertFalse("file must not exist", project.getSomeWorkspace().child("filename").exists()); + } + } From d20f28a06bfaa52b97babd20096a4a555a03cc88 Mon Sep 17 00:00:00 2001 From: Oleg Nenashev Date: Wed, 15 Oct 2014 14:13:17 +0400 Subject: [PATCH 16/40] Noting #1428 Signed-off-by: Oleg Nenashev --- changelog.html | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/changelog.html b/changelog.html index 336ea09a1a..d5e879c25a 100644 --- a/changelog.html +++ b/changelog.html @@ -55,7 +55,9 @@ Upcoming changes From cf8f8114f48f465e894f39c6f0f08c569a001342 Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Wed, 15 Oct 2014 18:19:10 +0200 Subject: [PATCH 17/40] Improve #1428 note The commit message cannot really stand alone. --- changelog.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.html b/changelog.html index d5e879c25a..129abe322a 100644 --- a/changelog.html +++ b/changelog.html @@ -56,7 +56,7 @@ Upcoming changes From e168f4d4c925627244731d164075d0d77b0613a8 Mon Sep 17 00:00:00 2001 From: Kenneth Garland Date: Wed, 15 Oct 2014 14:26:05 -0400 Subject: [PATCH 18/40] Changed wording for command line message. --- core/src/main/resources/hudson/cli/Messages.properties | 2 +- core/src/main/resources/hudson/cli/Messages_pt_BR.properties | 2 +- test/src/test/groovy/hudson/cli/BuildCommandTest.groovy | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/core/src/main/resources/hudson/cli/Messages.properties b/core/src/main/resources/hudson/cli/Messages.properties index 2abac75d81..57c9299f81 100644 --- a/core/src/main/resources/hudson/cli/Messages.properties +++ b/core/src/main/resources/hudson/cli/Messages.properties @@ -71,7 +71,7 @@ SessionIdCommand.ShortDescription=Outputs the session ID, which changes every ti UpdateViewCommand.ShortDescription=\ Updates the view definition XML from stdin. The opposite of the get-view command. -BuildCommand.CLICause.ShortDescription=Started by command line by {0} +BuildCommand.CLICause.ShortDescription=Started from command line by {0} BuildCommand.CLICause.CannotBuildDisabled=\ Cannot build {0} because it is disabled. BuildCommand.CLICause.CannotBuildConfigNotSaved=\ diff --git a/core/src/main/resources/hudson/cli/Messages_pt_BR.properties b/core/src/main/resources/hudson/cli/Messages_pt_BR.properties index c4aa5b9062..007038f34b 100644 --- a/core/src/main/resources/hudson/cli/Messages_pt_BR.properties +++ b/core/src/main/resources/hudson/cli/Messages_pt_BR.properties @@ -46,7 +46,7 @@ ConsoleCommand.ShortDescription=Retorna a sa\u00edda do console de um build. UpdateJobCommand.ShortDescription=Atualiza a defini\u00e7\u00e3o XML do job da stdin. \u00c9 o contr\u00e1rio do comando get-job. # Lists all the available commands or a detailed description of single command. HelpCommand.ShortDescription=Lista todos os comandos dispon\u00edveis ou uma descri\u00e7\u00e3o detalhada de um \u00fanico comando. -# Started by command line by {0} +# Started from command line by {0} BuildCommand.CLICause.ShortDescription=Iniciado pela linha de comando por {0} # Reads stdin and sends that out as an e-mail. MailCommand.ShortDescription=L\u00ea a stdin e envia como um e-mail. diff --git a/test/src/test/groovy/hudson/cli/BuildCommandTest.groovy b/test/src/test/groovy/hudson/cli/BuildCommandTest.groovy index 1af3891380..e13c8a59ea 100644 --- a/test/src/test/groovy/hudson/cli/BuildCommandTest.groovy +++ b/test/src/test/groovy/hudson/cli/BuildCommandTest.groovy @@ -167,7 +167,7 @@ public class BuildCommandTest { def o = new ByteArrayOutputStream() cli.execute(["build","-s","-v",p.name],System.in,new TeeOutputStream(System.out,o),System.err) j.assertBuildStatusSuccess(p.getBuildByNumber(1)) - assertTrue(o.toString(), o.toString().contains("Started by command line by anonymous")) + assertTrue(o.toString(), o.toString().contains("Started from command line by anonymous")) assertTrue(o.toString().contains("Finished: SUCCESS")) } finally { cli.close() From e8911ca33c00ef2303247d3a94a356372e9ca5f8 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Tue, 14 Oct 2014 15:30:34 -0700 Subject: [PATCH 19/40] Documenting what this setting controls --- core/src/main/resources/lib/hudson/ballColorTd.jelly | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/core/src/main/resources/lib/hudson/ballColorTd.jelly b/core/src/main/resources/lib/hudson/ballColorTd.jelly index bbc7aee38b..7a778a4cb9 100644 --- a/core/src/main/resources/lib/hudson/ballColorTd.jelly +++ b/core/src/main/resources/lib/hudson/ballColorTd.jelly @@ -28,6 +28,11 @@ THE SOFTWARE. Color of the ball or null to skip drawing. + + Specifies the size of the icon. If unspecified, it inherits from + the nearest setting in the ancestor call tree. + Possible values are "icon-sm" (16x16), "icon-md" (24x24), "icon-lg" (32x32), "icon-xlg" (48x48) + Additional CSS styles to apply. From c7e98823415f3619e7a43d3f99b7f627b873fd5c Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Tue, 14 Oct 2014 15:35:15 -0700 Subject: [PATCH 20/40] Restored backward compatibility. Up to b5843f3, supported @iconSize="16x16" and so on that controls the color. In the new scheme, it needs to be translated to iconSizeClass --- core/src/main/resources/lib/hudson/ballColorTd.jelly | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/src/main/resources/lib/hudson/ballColorTd.jelly b/core/src/main/resources/lib/hudson/ballColorTd.jelly index 7a778a4cb9..9889dbaf59 100644 --- a/core/src/main/resources/lib/hudson/ballColorTd.jelly +++ b/core/src/main/resources/lib/hudson/ballColorTd.jelly @@ -43,6 +43,10 @@ THE SOFTWARE. + + + + From c3b40caab385d78fd668a3b3f743590618f9f653 Mon Sep 17 00:00:00 2001 From: Thiago Zanetti Date: Wed, 15 Oct 2014 17:17:04 -0400 Subject: [PATCH 21/40] Updating the number of provided plugins. The goals is to maintain the information about the provided plugins near to the one showed on jenkins-ci.org home. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index fa8bad71fe..f021c54d33 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ Copyright © 2004–, Kohsuke Kawaguchi, Sun Microsystems, Inc., and a number of About ----- -In a nutshell Jenkins CI is the leading open-source continuous integration server. Built with Java, it provides over 930 plugins to support building and testing virtually any project. +In a nutshell Jenkins CI is the leading open-source continuous integration server. Built with Java, it provides over 1000 plugins to support building and testing virtually any project. Downloads --------- From 04b8ef8a05c18f67d093bfefb3a0cb9fb0c1900d Mon Sep 17 00:00:00 2001 From: Oleg Nenashev Date: Thu, 16 Oct 2014 02:00:47 +0400 Subject: [PATCH 22/40] Annotated hudson.model.User methods. The fix also includes comments in #1424 regarding Nonnull usage Signed-off-by: Oleg Nenashev --- core/src/main/java/hudson/model/User.java | 52 ++++++++++++++--------- 1 file changed, 31 insertions(+), 21 deletions(-) diff --git a/core/src/main/java/hudson/model/User.java b/core/src/main/java/hudson/model/User.java index 41b9ff8548..5f86462ca6 100644 --- a/core/src/main/java/hudson/model/User.java +++ b/core/src/main/java/hudson/model/User.java @@ -81,6 +81,7 @@ import java.util.logging.Level; import java.util.logging.Logger; import javax.annotation.CheckForNull; import javax.annotation.Nonnull; +import javax.annotation.Nullable; /** * Represents a user. @@ -184,11 +185,11 @@ public class User extends AbstractModelObject implements AccessControlled, Descr return id; } - public String getUrl() { + public @Nonnull String getUrl() { return "user/"+Util.rawEncode(idStrategy().keyFor(id)); } - public String getSearchUrl() { + public @Nonnull String getSearchUrl() { return "/user/"+Util.rawEncode(idStrategy().keyFor(id)); } @@ -196,24 +197,22 @@ public class User extends AbstractModelObject implements AccessControlled, Descr * The URL of the user page. */ @Exported(visibility=999) - public String getAbsoluteUrl() { + public @Nonnull String getAbsoluteUrl() { return Jenkins.getInstance().getRootUrl()+getUrl(); } /** * Gets the human readable name of this user. * This is configurable by the user. - * - * @return - * never null. */ @Exported(visibility=999) - public String getFullName() { + public @Nonnull String getFullName() { return fullName; } /** - * Sets the human readable name of thie user. + * Sets the human readable name of the user. + * If the input parameter is empty, the user's ID will be set. */ public void setFullName(String name) { if(Util.fixEmptyAndTrim(name)==null) name=id; @@ -221,7 +220,7 @@ public class User extends AbstractModelObject implements AccessControlled, Descr } @Exported - public String getDescription() { + public @CheckForNull String getDescription() { return description; } @@ -235,7 +234,7 @@ public class User extends AbstractModelObject implements AccessControlled, Descr /** * Updates the user object by adding a property. */ - public synchronized void addProperty(UserProperty p) throws IOException { + public synchronized void addProperty(@Nonnull UserProperty p) throws IOException { UserProperty old = getProperty(p.getClass()); List ps = new ArrayList(properties); if(old!=null) @@ -277,7 +276,7 @@ public class User extends AbstractModelObject implements AccessControlled, Descr * If this user is not a valid user in the backend {@link SecurityRealm}. * @since 1.419 */ - public Authentication impersonate() throws UsernameNotFoundException { + public @Nonnull Authentication impersonate() throws UsernameNotFoundException { try { UserDetails u = new ImpersonatingUserDetailsService( Jenkins.getInstance().getSecurityRealm().getSecurityComponents().userDetails).loadUserByUsername(id); @@ -325,9 +324,11 @@ public class User extends AbstractModelObject implements AccessControlled, Descr * (by creating a new {@link User} object if none exists.) * If false, this method will return null if {@link User} object * with the given name doesn't exist. + * @return Requested user. May be {@code null} if a user does not exist and + * {@code create} is false. * @deprecated use {@link User#get(String, boolean, java.util.Map)} */ - public static User get(String idOrFullName, boolean create) { + public static @Nullable User get(String idOrFullName, boolean create) { return get(idOrFullName, create, Collections.emptyMap()); } @@ -343,8 +344,12 @@ public class User extends AbstractModelObject implements AccessControlled, Descr * @param context * contextual environment this user idOfFullName was retrieved from, * that can help resolve the user ID + * + * @return + * An existing or created user. May be {@code null} if a user does not exist and + * {@code create} is false. */ - public static User get(String idOrFullName, boolean create, Map context) { + public static @Nullable User get(String idOrFullName, boolean create, Map context) { if(idOrFullName==null) return null; @@ -362,14 +367,19 @@ public class User extends AbstractModelObject implements AccessControlled, Descr } } // DefaultUserCanonicalIdResolver will always return a non-null id if all other CanonicalIdResolver failed - + if (id == null) { + throw new IllegalStateException("The user id should be always non-null thanks to DefaultUserCanonicalIdResolver"); + } return getOrCreate(id, idOrFullName, create); } /** - * retrieve a user by its ID, and create a new one if requested + * Retrieve a user by its ID, and create a new one if requested. + * @return + * An existing or created user. May be {@code null} if a user does not exist and + * {@code create} is false. */ - private static User getOrCreate(String id, String fullName, boolean create) { + private static @Nullable User getOrCreate(@Nonnull String id, @Nonnull String fullName, boolean create) { String idkey = idStrategy().keyFor(id); byNameLock.readLock().lock(); @@ -461,7 +471,7 @@ public class User extends AbstractModelObject implements AccessControlled, Descr /** * Gets all the users. */ - public static Collection getAll() { + public static @Nonnull Collection getAll() { final IdStrategy strategy = idStrategy(); if(System.currentTimeMillis() -lastScanned>10000) { // occasionally scan the file system to check new users @@ -548,12 +558,12 @@ public class User extends AbstractModelObject implements AccessControlled, Descr /** * Returns the user name. */ - public String getDisplayName() { + public @Nonnull String getDisplayName() { return getFullName(); } /** true if {@link AbstractBuild#hasParticipant} or {@link hudson.model.Cause.UserIdCause} */ - private boolean relatedTo(AbstractBuild b) { + private boolean relatedTo(@Nonnull AbstractBuild b) { if (b.hasParticipant(this)) { return true; } @@ -573,7 +583,7 @@ public class User extends AbstractModelObject implements AccessControlled, Descr * by the timestamp order. */ @WithBridgeMethods(List.class) - public RunList getBuilds() { + public @Nonnull RunList getBuilds() { return new RunList>(Jenkins.getInstance().getAllItems(Job.class)).filter(new Predicate>() { @Override public boolean apply(Run r) { return r instanceof AbstractBuild && relatedTo((AbstractBuild) r); @@ -585,7 +595,7 @@ public class User extends AbstractModelObject implements AccessControlled, Descr * Gets all the {@link AbstractProject}s that this user has committed to. * @since 1.191 */ - public Set> getProjects() { + public @Nonnull Set> getProjects() { Set> r = new HashSet>(); for (AbstractProject p : Jenkins.getInstance().getAllItems(AbstractProject.class)) if(p.hasParticipant(this)) From 52941e338107299a33c56e138a6bfd78b228332f Mon Sep 17 00:00:00 2001 From: Dominique Date: Wed, 1 Oct 2014 21:49:40 -0400 Subject: [PATCH 23/40] Updating javadoc after struggling to add action to nodes --- core/src/main/java/hudson/model/Computer.java | 10 +++++----- core/src/main/java/hudson/model/Node.java | 6 +++++- core/src/main/java/hudson/slaves/NodeProperty.java | 5 ++++- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/hudson/model/Computer.java b/core/src/main/java/hudson/model/Computer.java index b0c1b39ce8..546204abc8 100644 --- a/core/src/main/java/hudson/model/Computer.java +++ b/core/src/main/java/hudson/model/Computer.java @@ -108,14 +108,14 @@ import static javax.servlet.http.HttpServletResponse.*; * *

* {@link Executor}s on one {@link Computer} are transparently interchangeable - * (that is the definition of {@link Computer}.) + * (that is the definition of {@link Computer}). * *

- * This object is related to {@link Node} but they have some significant difference. + * This object is related to {@link Node} but they have some significant differences. * {@link Computer} primarily works as a holder of {@link Executor}s, so * if a {@link Node} is configured (probably temporarily) with 0 executors, * you won't have a {@link Computer} object for it (except for the master node, - * which always get its {@link Computer} in case we have no static executors and + * which always gets its {@link Computer} in case we have no static executors and * we need to run a {@link FlyweightTask} - see JENKINS-7291 for more discussion.) * * Also, even if you remove a {@link Node}, it takes time for the corresponding @@ -124,8 +124,8 @@ import static javax.servlet.http.HttpServletResponse.*; * remains intact, while all the {@link Node} objects will go away. * *

- * This object also serves UI (since {@link Node} is an interface and can't have - * related side pages.) + * This object also serves UI (unlike {@link Node}), and can be used along with + * {@link TransientComputerActionFactory} to add {@link Action}s to {@link Computer}s. * * @author Kohsuke Kawaguchi */ diff --git a/core/src/main/java/hudson/model/Node.java b/core/src/main/java/hudson/model/Node.java index 03f2c2413e..f5f9f04b59 100644 --- a/core/src/main/java/hudson/model/Node.java +++ b/core/src/main/java/hudson/model/Node.java @@ -69,7 +69,7 @@ import org.kohsuke.stapler.export.ExportedBean; import org.kohsuke.stapler.export.Exported; /** - * Base type of Jenkins slaves (although in practice, you probably extend {@link Slave} to define a new slave type.) + * Base type of Jenkins slaves (although in practice, you probably extend {@link Slave} to define a new slave type). * *

* As a special case, {@link Jenkins} extends from here. @@ -77,6 +77,10 @@ import org.kohsuke.stapler.export.Exported; *

* Nodes are persisted objects that capture user configurations, and instances get thrown away and recreated whenever * the configuration changes. Running state of nodes are captured by {@link Computer}s. + * + *

+ * There is no URL binding for {@link Node}. {@link Computer} and {@link TransientComputerActionFactory} must + * be used to associate new {@link Action}s to slaves. * * @author Kohsuke Kawaguchi * @see NodeMonitor diff --git a/core/src/main/java/hudson/slaves/NodeProperty.java b/core/src/main/java/hudson/slaves/NodeProperty.java index 42da21efeb..68c50230c4 100644 --- a/core/src/main/java/hudson/slaves/NodeProperty.java +++ b/core/src/main/java/hudson/slaves/NodeProperty.java @@ -54,8 +54,11 @@ import javax.annotation.Nonnull; * Extensible property of {@link Node}. * *

- * Plugins can contribute this extension point to add additional data or UI actions to {@link Node}. + * Plugins can contribute this extension point to add additional data to {@link Node}. * {@link NodeProperty}s show up in the configuration screen of a node, and they are persisted with the {@link Node} object. + * + *

+ * To add UI action to {@link Node}s, i.e. a new link shown in the left side menu on a node page (./computer/<a node>), see instead {@link hudson.model.TransientComputerActionFactory}. * * *

Views

From 34b4bf52ad13245eb2330de3f0cdb63a85fd1b19 Mon Sep 17 00:00:00 2001 From: Stephen Connolly Date: Thu, 16 Oct 2014 10:17:31 +0100 Subject: [PATCH 24/40] Do not fail certificate validation because some files in Jenkins Home are not actually certificates - If there are any files in `$JENKINS_HOME/update-center-rootCAs` that are not ending in `.txt` and are not valid certificates, do not blow up the entire certificate checking (and hence the ability to see updates or tool installers) but rather ignore those files and warn the user that the file should not be there. The correct thing to do is only use as trusted certificates those certificates that parse. - Also fix resource leak --- .../jenkins/util/JSONSignatureValidator.java | 48 +++++++++++++++++-- 1 file changed, 45 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/jenkins/util/JSONSignatureValidator.java b/core/src/main/java/jenkins/util/JSONSignatureValidator.java index 77470f39ba..e7d62b423a 100644 --- a/core/src/main/java/jenkins/util/JSONSignatureValidator.java +++ b/core/src/main/java/jenkins/util/JSONSignatureValidator.java @@ -13,11 +13,14 @@ import java.io.ByteArrayInputStream; import java.io.File; import java.io.FileInputStream; import java.io.IOException; +import java.io.InputStream; import java.io.OutputStreamWriter; import java.security.DigestOutputStream; import java.security.GeneralSecurityException; import java.security.MessageDigest; import java.security.Signature; +import java.security.cert.Certificate; +import java.security.cert.CertificateException; import java.security.cert.CertificateExpiredException; import java.security.cert.CertificateFactory; import java.security.cert.CertificateNotYetValidException; @@ -75,18 +78,57 @@ public class JSONSignatureValidator { Jenkins j = Jenkins.getInstance(); for (String cert : (Set) j.servletContext.getResourcePaths("/WEB-INF/update-center-rootCAs")) { if (cert.endsWith(".txt")) continue; // skip text files that are meant to be documentation - anchors.add(new TrustAnchor((X509Certificate)cf.generateCertificate(j.servletContext.getResourceAsStream(cert)),null)); + InputStream in = j.servletContext.getResourceAsStream(cert); + Certificate certificate; + try { + certificate = cf.generateCertificate(in); + } catch (CertificateException e) { + LOGGER.log(Level.WARNING, String.format("Webapp resources in /WEB-INF/update-center-rootCAs are " + + "expected to be either certificates or .txt files documenting the " + + "certificates, but %s did not parse as a certificate. Skipping this " + + "resource for now.", + cert), e); + continue; + } finally { + in.close(); + } + try { + anchors.add(new TrustAnchor((X509Certificate) certificate, null)); + } catch (IllegalArgumentException e) { + LOGGER.log(Level.WARNING, + String.format("The name constraints in the certificate resource %s could not be " + + "decoded. Skipping this resource for now.", + cert), e); + } } File[] cas = new File(j.root, "update-center-rootCAs").listFiles(); if (cas!=null) { for (File cert : cas) { - if (cert.getName().endsWith(".txt")) continue; // skip text files that are meant to be documentation + if (cert.isDirectory() || cert.getName().endsWith(".txt")) { + continue; // skip directories also any text files that are meant to be documentation + } FileInputStream in = new FileInputStream(cert); + Certificate certificate; try { - anchors.add(new TrustAnchor((X509Certificate)cf.generateCertificate(in),null)); + certificate = cf.generateCertificate(in); + } catch (CertificateException e) { + LOGGER.log(Level.WARNING, String.format("Files in %s are expected to be either " + + "certificates or .txt files documenting the certificates, " + + "but %s did not parse as a certificate. Skipping this file for now.", + cert.getParentFile().getAbsolutePath(), + cert.getAbsolutePath()), e); + continue; } finally { in.close(); } + try { + anchors.add(new TrustAnchor((X509Certificate) certificate, null)); + } catch (IllegalArgumentException e) { + LOGGER.log(Level.WARNING, + String.format("The name constraints in the certificate file %s could not be " + + "decoded. Skipping this file for now.", + cert.getAbsolutePath()), e); + } } } CertificateUtil.validatePath(certs, anchors); From f423174d0b60468022c96a7e4d90a829671b815f Mon Sep 17 00:00:00 2001 From: Stephen Connolly Date: Thu, 16 Oct 2014 10:26:27 +0100 Subject: [PATCH 25/40] Additional safety as an addendum to 34b4bf52ad13245eb2330de3f0cdb63a85fd1b19 --- core/src/main/java/jenkins/util/JSONSignatureValidator.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/jenkins/util/JSONSignatureValidator.java b/core/src/main/java/jenkins/util/JSONSignatureValidator.java index e7d62b423a..758b95cedc 100644 --- a/core/src/main/java/jenkins/util/JSONSignatureValidator.java +++ b/core/src/main/java/jenkins/util/JSONSignatureValidator.java @@ -77,8 +77,11 @@ public class JSONSignatureValidator { Set anchors = new HashSet(); // CertificateUtil.getDefaultRootCAs(); Jenkins j = Jenkins.getInstance(); for (String cert : (Set) j.servletContext.getResourcePaths("/WEB-INF/update-center-rootCAs")) { - if (cert.endsWith(".txt")) continue; // skip text files that are meant to be documentation + if (cert.endsWith("/") || cert.endsWith(".txt")) { + continue; // skip directories also any text files that are meant to be documentation + } InputStream in = j.servletContext.getResourceAsStream(cert); + if (in == null) continue; // our test for paths ending in / should prevent this from happening Certificate certificate; try { certificate = cf.generateCertificate(in); From 582128b9ac179a788d43c1478be8a5224dc19710 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Thu, 16 Oct 2014 19:15:56 -0700 Subject: [PATCH 26/40] [FIXED JENKINS-25019] A truly conforming servlet 3.0 container does not allow us to set "secure cookie" flag beyond ServletContextListener.onInitialized(). If we see that, don't scare the users. --- changelog.html | 4 +++ core/src/main/java/hudson/WebAppMain.java | 28 +++++++++++++++++++ .../model/JenkinsLocationConfiguration.java | 16 +++++++---- 3 files changed, 42 insertions(+), 6 deletions(-) diff --git a/changelog.html b/changelog.html index d8a3ebc75a..96236f78f9 100644 --- a/changelog.html +++ b/changelog.html @@ -61,6 +61,10 @@ Upcoming changes
  • Prevent empty file creation if file parameter is left empty. (issue 3539) +
  • + Servlet containers may refuse to let us set secure cookie flag. + Deal with it gracefully. + (issue 25019) diff --git a/core/src/main/java/hudson/WebAppMain.java b/core/src/main/java/hudson/WebAppMain.java index 1f332e9751..11d438d5ab 100644 --- a/core/src/main/java/hudson/WebAppMain.java +++ b/core/src/main/java/hudson/WebAppMain.java @@ -56,6 +56,7 @@ import javax.xml.transform.TransformerFactoryConfigurationError; import java.io.File; import java.io.FileOutputStream; import java.io.IOException; +import java.lang.reflect.Method; import java.net.URL; import java.net.URLClassLoader; import java.util.Date; @@ -116,6 +117,8 @@ public class WebAppMain implements ServletContextListener { installLogger(); + markCookieAsHttpOnly(context); + final FileAndDescription describedHomeDir = getHomeDir(event); home = describedHomeDir.file.getAbsoluteFile(); home.mkdirs(); @@ -251,6 +254,31 @@ public class WebAppMain implements ServletContextListener { } } + /** + * Set the session cookie as HTTP only. + * + * @see discussion of this topic in OWASP + */ + private void markCookieAsHttpOnly(ServletContext context) { + try { + Method m; + try { + m = context.getClass().getMethod("getSessionCookieConfig"); + } catch (NoSuchMethodException x) { // 3.0+ + LOGGER.log(Level.FINE, "Failed to set secure cookie flag", x); + return; + } + Object sessionCookieConfig = m.invoke(context); + + // not exposing session cookie to JavaScript to mitigate damage caused by XSS + Class scc = Class.forName("javax.servlet.SessionCookieConfig"); + Method setHttpOnly = scc.getMethod("setHttpOnly",boolean.class); + setHttpOnly.invoke(sessionCookieConfig,true); + } catch (Exception e) { + LOGGER.log(Level.WARNING, "Failed to set HTTP-only cookie flag", e); + } + } + public void joinInit() throws InterruptedException { initThread.join(); } diff --git a/core/src/main/java/jenkins/model/JenkinsLocationConfiguration.java b/core/src/main/java/jenkins/model/JenkinsLocationConfiguration.java index 6836467217..c10e51d526 100644 --- a/core/src/main/java/jenkins/model/JenkinsLocationConfiguration.java +++ b/core/src/main/java/jenkins/model/JenkinsLocationConfiguration.java @@ -14,6 +14,7 @@ import javax.mail.internet.InternetAddress; import javax.servlet.ServletContext; import java.io.File; import java.io.IOException; +import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.logging.Level; import java.util.logging.Logger; @@ -117,14 +118,17 @@ public class JenkinsLocationConfiguration extends GlobalConfiguration { } Object sessionCookieConfig = m.invoke(context); - // not exposing session cookie to JavaScript to mitigate damage caused by XSS Class scc = Class.forName("javax.servlet.SessionCookieConfig"); - Method setHttpOnly = scc.getMethod("setHttpOnly",boolean.class); - setHttpOnly.invoke(sessionCookieConfig,true); - - Method setSecure = scc.getMethod("setSecure",boolean.class); + Method setSecure = scc.getMethod("setSecure", boolean.class); boolean v = fixNull(jenkinsUrl).startsWith("https"); - setSecure.invoke(sessionCookieConfig,v); + setSecure.invoke(sessionCookieConfig, v); + } catch (InvocationTargetException e) { + if (e.getTargetException() instanceof IllegalStateException) { + // servlet 3.0 spec seems to prohibit this from getting set at runtime, + // though Winstone is happy to accept i. see JENKINS-25019 + return; + } + LOGGER.log(Level.WARNING, "Failed to set secure cookie flag", e); } catch (Exception e) { LOGGER.log(Level.WARNING, "Failed to set secure cookie flag", e); } From f07fb85352b7cce0b6acaa39bfac4773c7608e5a Mon Sep 17 00:00:00 2001 From: Stephen Connolly Date: Fri, 17 Oct 2014 09:38:32 +0100 Subject: [PATCH 27/40] Allow Node and RetentionStrategy to contribute to Computer#isAcceptingTasks --- core/src/main/java/hudson/model/Computer.java | 8 +++++++- core/src/main/java/hudson/model/Node.java | 13 +++++++++++++ .../main/java/hudson/slaves/RetentionStrategy.java | 13 +++++++++++++ 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/hudson/model/Computer.java b/core/src/main/java/hudson/model/Computer.java index ce3efaba3a..fff4a7765e 100644 --- a/core/src/main/java/hudson/model/Computer.java +++ b/core/src/main/java/hudson/model/Computer.java @@ -24,6 +24,8 @@ */ package hudson.model; +import edu.umd.cs.findbugs.annotations.OverrideMustInvoke; +import edu.umd.cs.findbugs.annotations.When; import hudson.EnvVars; import hudson.Launcher.ProcStarter; import hudson.Util; @@ -1318,9 +1320,13 @@ public /*transient*/ abstract class Computer extends Actionable implements Acces * scheduling that does not overlap with being offline. * * @return {@code true} if the computer is accepting tasks + * @see hudson.slaves.RetentionStrategy#isAcceptingTasks(Computer) + * @see hudson.model.Node#isAcceptingTasks() */ + @OverrideMustInvoke(When.ANYTIME) public boolean isAcceptingTasks() { - return true; + final Node node = getNode(); + return getRetentionStrategy().isAcceptingTasks(this) && (node == null || node.isAcceptingTasks()); } /** diff --git a/core/src/main/java/hudson/model/Node.java b/core/src/main/java/hudson/model/Node.java index f5f9f04b59..ff73a2fb9d 100644 --- a/core/src/main/java/hudson/model/Node.java +++ b/core/src/main/java/hudson/model/Node.java @@ -194,6 +194,19 @@ public abstract class Node extends AbstractModelObject implements Reconfigurable */ protected abstract Computer createComputer(); + /** + * Returns {@code true} if the node is accepting tasks. Needed to allow slaves programmatic suspension of task + * scheduling that does not overlap with being offline. Called by {@link Computer#isAcceptingTasks()}. + * This method is distinct from {@link Computer#isAcceptingTasks()} as sometimes the {@link Node} concrete + * class may not have control over the {@link hudson.model.Computer} concrete class associated with it. + * + * @return {@code true} if the node is accepting tasks. + * @see Computer#isAcceptingTasks() + */ + public boolean isAcceptingTasks() { + return true; + } + /** * Let Nodes be aware of the lifecycle of their own {@link Computer}. */ diff --git a/core/src/main/java/hudson/slaves/RetentionStrategy.java b/core/src/main/java/hudson/slaves/RetentionStrategy.java index 95cb99b375..474afa45a1 100644 --- a/core/src/main/java/hudson/slaves/RetentionStrategy.java +++ b/core/src/main/java/hudson/slaves/RetentionStrategy.java @@ -67,6 +67,19 @@ public abstract class RetentionStrategy extends AbstractDesc return true; } + /** + * Returns {@code true} if the computer is accepting tasks. Needed to allow retention strategies programmatic + * suspension of task scheduling that in preparation for going offline. Called by + * {@link hudson.model.Computer#isAcceptingTasks()} + * + * @param c the computer. + * @return {@code true} if the computer is accepting tasks + * @see hudson.model.Computer#isAcceptingTasks() + */ + public boolean isAcceptingTasks(T c) { + return true; + } + /** * Called when a new {@link Computer} object is introduced (such as when Hudson started, or when * a new slave is added.) From 10ac2b3ccd98c80c0d02dd7920202440d487bf88 Mon Sep 17 00:00:00 2001 From: Stephen Connolly Date: Fri, 17 Oct 2014 12:20:07 +0100 Subject: [PATCH 28/40] add @since tags --- core/src/main/java/hudson/model/Node.java | 1 + core/src/main/java/hudson/slaves/RetentionStrategy.java | 1 + 2 files changed, 2 insertions(+) diff --git a/core/src/main/java/hudson/model/Node.java b/core/src/main/java/hudson/model/Node.java index ff73a2fb9d..2175ee215c 100644 --- a/core/src/main/java/hudson/model/Node.java +++ b/core/src/main/java/hudson/model/Node.java @@ -202,6 +202,7 @@ public abstract class Node extends AbstractModelObject implements Reconfigurable * * @return {@code true} if the node is accepting tasks. * @see Computer#isAcceptingTasks() + * @since 1.586 */ public boolean isAcceptingTasks() { return true; diff --git a/core/src/main/java/hudson/slaves/RetentionStrategy.java b/core/src/main/java/hudson/slaves/RetentionStrategy.java index 474afa45a1..6a350e85d6 100644 --- a/core/src/main/java/hudson/slaves/RetentionStrategy.java +++ b/core/src/main/java/hudson/slaves/RetentionStrategy.java @@ -75,6 +75,7 @@ public abstract class RetentionStrategy extends AbstractDesc * @param c the computer. * @return {@code true} if the computer is accepting tasks * @see hudson.model.Computer#isAcceptingTasks() + * @since 1.586 */ public boolean isAcceptingTasks(T c) { return true; From 7acbba330c9eea0c90b05cd0d23b60e4b28e1f66 Mon Sep 17 00:00:00 2001 From: Stephen Connolly Date: Fri, 17 Oct 2014 09:40:28 +0100 Subject: [PATCH 29/40] Typo --- core/src/main/java/hudson/slaves/NodeProvisioner.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/hudson/slaves/NodeProvisioner.java b/core/src/main/java/hudson/slaves/NodeProvisioner.java index 5bb95d18fa..c63a16180a 100644 --- a/core/src/main/java/hudson/slaves/NodeProvisioner.java +++ b/core/src/main/java/hudson/slaves/NodeProvisioner.java @@ -182,7 +182,7 @@ public class NodeProvisioner { cl.onComplete(f,node); hudson.addNode(node); - LOGGER.info(f.displayName+" provisioningE successfully completed. We have now "+hudson.getComputers().length+" computer(s)"); + LOGGER.info(f.displayName+" provisioning successfully completed. We have now "+hudson.getComputers().length+" computer(s)"); } catch (InterruptedException e) { throw new AssertionError(e); // since we confirmed that the future is already done } catch (ExecutionException e) { From 4d987accef89aad2a1284696e4cecf3a31fbd5e4 Mon Sep 17 00:00:00 2001 From: Stephen Connolly Date: Fri, 17 Oct 2014 12:36:01 +0100 Subject: [PATCH 30/40] Clarify when to use SlaveComputer#setAcceptingTasks over RetentionStrategy#isAcceptingTasks(T) --- .../src/main/java/hudson/slaves/SlaveComputer.java | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/hudson/slaves/SlaveComputer.java b/core/src/main/java/hudson/slaves/SlaveComputer.java index 6f641dd92c..9191d764fa 100644 --- a/core/src/main/java/hudson/slaves/SlaveComputer.java +++ b/core/src/main/java/hudson/slaves/SlaveComputer.java @@ -23,6 +23,8 @@ */ package hudson.slaves; +import edu.umd.cs.findbugs.annotations.OverrideMustInvoke; +import edu.umd.cs.findbugs.annotations.When; import hudson.AbortException; import hudson.FilePath; import hudson.Util; @@ -140,8 +142,11 @@ public class SlaveComputer extends Computer { * {@inheritDoc} */ @Override + @OverrideMustInvoke(When.ANYTIME) public boolean isAcceptingTasks() { - return acceptingTasks; + // our boolean flag is an override on any additional programmatic reasons why this slave might not be + // accepting tasks. + return acceptingTasks && super.isAcceptingTasks(); } /** @@ -152,8 +157,11 @@ public class SlaveComputer extends Computer { } /** - * Allows a {@linkplain hudson.slaves.ComputerLauncher} or a {@linkplain hudson.slaves.RetentionStrategy} to - * suspend tasks being accepted by the slave computer. + * Allows suspension of tasks being accepted by the slave computer. While this could be called by a + * {@linkplain hudson.slaves.ComputerLauncher} or a {@linkplain hudson.slaves.RetentionStrategy}, such usage + * can result in fights between multiple actors calling setting differential values. A better approach + * is to override {@link hudson.slaves.RetentionStrategy#isAcceptingTasks(hudson.model.Computer)} if the + * {@link hudson.slaves.RetentionStrategy} needs to control availability. * * @param acceptingTasks {@code true} if the slave can accept tasks. */ From 7fa1252e581115b690bb1e91aac85da112db755e Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Sat, 18 Oct 2014 01:28:11 +0200 Subject: [PATCH 31/40] Noting #1306, 2b6c00d in 1.584 --- changelog.html | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/changelog.html b/changelog.html index 96236f78f9..4a9f1a487f 100644 --- a/changelog.html +++ b/changelog.html @@ -119,6 +119,13 @@ Upcoming changes
  • When a user could not see a view, but could delete/move/rename jobs contained in it, the view was not properly updated. (issue 22769) +
  • + Use POST for cancel quiet down link. + (issue 23020, + issue 23942) +
  • + Do not consider port in use error to be a successful start of Jenkins on Debian. + (issue 24966)

    What's new in 1.583 (2014/10/01)

      From cc06ae78e3955746176f64ff3f36d7f0e3cc80ab Mon Sep 17 00:00:00 2001 From: Oleg Nenashev Date: Sat, 18 Oct 2014 22:03:28 +0400 Subject: [PATCH 32/40] [JENKINS-19017] - Unit tests Signed-off-by: Oleg Nenashev --- .../hudson/model/FileParameterValueTest.java | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 core/src/test/java/hudson/model/FileParameterValueTest.java diff --git a/core/src/test/java/hudson/model/FileParameterValueTest.java b/core/src/test/java/hudson/model/FileParameterValueTest.java new file mode 100644 index 0000000000..cd03b68409 --- /dev/null +++ b/core/src/test/java/hudson/model/FileParameterValueTest.java @@ -0,0 +1,64 @@ +/* + * The MIT License + * + * Copyright 2014 Oleg Nenashev . + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package hudson.model; + +import java.io.File; +import org.junit.Assert; +import org.junit.Test; +import org.jvnet.hudson.test.Issue; + +/** + * Test for {@link FileParameterValue}. + * @author Oleg Nenashev + */ +public class FileParameterValueTest { + + @Issue("JENKINS-19017") + @Test public void compareParamsWithSameName() { + final String paramName = "MY_FILE_PARAM"; // Same paramName (location) reproduces the bug + final FileParameterValue param1 = new FileParameterValue(paramName, new File("ws_param1.txt"), "param1.txt"); + final FileParameterValue param2 = new FileParameterValue(paramName, new File("ws_param2.txt"), "param2.txt"); + + Assert.assertNotEquals("Files with same locations shoud be considered as different", param1, param2); + Assert.assertNotEquals("Files with same locations shoud be considered as different", param2, param1); + } + + @Test public void compareNullParams() { + final String paramName = "MY_FILE_PARAM"; + FileParameterValue nonNullParam = new FileParameterValue(paramName, new File("ws_param1.txt"), "param1.txt"); + FileParameterValue nullParam1 = new FileParameterValue(null, new File("null_param1.txt"), "null_param1.txt"); + FileParameterValue nullParam2 = new FileParameterValue(null, new File("null_param2.txt"), "null_param2.txt"); + + // Combine nulls + Assert.assertEquals(nullParam1, nullParam1); + Assert.assertEquals(nullParam1, nullParam2); + Assert.assertEquals(nullParam2, nullParam1); + Assert.assertEquals(nullParam2, nullParam2); + + // Compare with non-null + Assert.assertNotEquals(nullParam1, nonNullParam); + Assert.assertNotEquals(nonNullParam, nullParam1); + } +} From 6819a620c0e11a84d95e81d299be0159602cf9a2 Mon Sep 17 00:00:00 2001 From: Oleg Nenashev Date: Sun, 19 Oct 2014 23:47:35 +0400 Subject: [PATCH 33/40] Noting #1411 Signed-off-by: Oleg Nenashev --- changelog.html | 3 +++ core/src/main/java/hudson/model/FileParameterValue.java | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/changelog.html b/changelog.html index 4a9f1a487f..1c217937d8 100644 --- a/changelog.html +++ b/changelog.html @@ -65,6 +65,9 @@ Upcoming changes Servlet containers may refuse to let us set secure cookie flag. Deal with it gracefully. (issue 25019) +
    • + Existing FileParameters should be handled as different values to avoid merging of queued builds + (issue 19017)
    diff --git a/core/src/main/java/hudson/model/FileParameterValue.java b/core/src/main/java/hudson/model/FileParameterValue.java index 56d37e3812..930eb41fd8 100644 --- a/core/src/main/java/hudson/model/FileParameterValue.java +++ b/core/src/main/java/hudson/model/FileParameterValue.java @@ -163,7 +163,7 @@ public class FileParameterValue extends ParameterValue { /** * Compares file parameters (existing files will be considered as different). - * Function has been modified in order to avoid JENKINS-19017 issue (wrong merge of builds in the queue). + * @since 1.586 Function has been modified in order to avoid JENKINS-19017 issue (wrong merge of builds in the queue). */ @Override public boolean equals(Object obj) { @@ -178,7 +178,7 @@ public class FileParameterValue extends ParameterValue { if (location == null && other.location == null) return true; // Consider null parameters as equal - //TODO: check fingerprints or checksums to improve the behavior + //TODO: check fingerprints or checksums to improve the behavior (JENKINS-25211) // Return false even if files are equal return false; } From 2820dc723cc4029e529487ec62a71ce17afe1488 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Sun, 19 Oct 2014 19:43:16 -0700 Subject: [PATCH 34/40] [JENKINS-25169] Integrating 2.8 to really ensure SSLv2 is off (even when JDK supports that, and we haven't found any JDKs that do that) --- war/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/war/pom.xml b/war/pom.xml index 5c976e82cf..aeaf315335 100644 --- a/war/pom.xml +++ b/war/pom.xml @@ -103,7 +103,7 @@ THE SOFTWARE. --> org.jenkins-ci winstone - 2.7 + 2.8 test From 80e79b2af011ad743ad8c45475ea158624d4651c Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Sun, 19 Oct 2014 20:03:52 -0700 Subject: [PATCH 35/40] [maven-release-plugin] prepare release jenkins-1.585 --- cli/pom.xml | 2 +- core/pom.xml | 2 +- plugins/pom.xml | 10 +++++----- pom.xml | 4 ++-- test/pom.xml | 2 +- war/pom.xml | 2 +- 6 files changed, 11 insertions(+), 11 deletions(-) diff --git a/cli/pom.xml b/cli/pom.xml index 756ddb787a..c4409de4b2 100644 --- a/cli/pom.xml +++ b/cli/pom.xml @@ -5,7 +5,7 @@ pom org.jenkins-ci.main - 1.585-SNAPSHOT + 1.585 cli diff --git a/core/pom.xml b/core/pom.xml index 35378ce66e..47f0712e77 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -29,7 +29,7 @@ THE SOFTWARE. org.jenkins-ci.main pom - 1.585-SNAPSHOT + 1.585 ../pom.xml diff --git a/plugins/pom.xml b/plugins/pom.xml index d42c2c9d95..c183cb06af 100644 --- a/plugins/pom.xml +++ b/plugins/pom.xml @@ -11,7 +11,7 @@ org.jenkins-ci.plugins plugin Jenkins plugin POM - 1.585-SNAPSHOT + 1.585 pom +

    What's new in 1.585 (2014/10/19)

    • Build health computed repeatedly for a single Weather column cell. @@ -102,7 +105,6 @@ Upcoming changes Incorporated a fix for "Poodle" (CVE-2014-3566) vulnerability in the HTTPS connector of "java -jar jenkins.war" (issue 25169)
    -

    What's new in 1.584 (2014/10/12)

    • From b2fe53aac95d8d57f39cf61f9c6b96438cae2993 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Sun, 19 Oct 2014 21:50:09 -0700 Subject: [PATCH 38/40] updated changelog as a part of the release --- debian/debian/changelog | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/debian/debian/changelog b/debian/debian/changelog index 53bc3f96b2..4ac12482f1 100644 --- a/debian/debian/changelog +++ b/debian/debian/changelog @@ -1,3 +1,9 @@ +jenkins (1.585) unstable; urgency=low + + * See http://jenkins-ci.org/changelog for more details. + + -- Kohsuke Kawaguchi Sun, 19 Oct 2014 21:00:43 -0700 + jenkins (1.584) unstable; urgency=low * See http://jenkins-ci.org/changelog for more details. From c7369f2cd6cfa4a5cdbe49d1e8c1b0766f398db8 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Sun, 19 Oct 2014 21:51:14 -0700 Subject: [PATCH 39/40] creating an RC branch --- changelog.html | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/changelog.html b/changelog.html index 4e1e45b102..8256834c0f 100644 --- a/changelog.html +++ b/changelog.html @@ -54,6 +54,14 @@ Upcoming changes + + + - - -

      What's new in 1.585 (2014/10/19)

        From 94590fdd69debdfe8469a074b3358f08f6ff0779 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Sat, 18 Oct 2014 20:48:50 -0700 Subject: [PATCH 40/40] [JENKINS-25169] integrated the fix for POODLE (cherry picked from commit 1351b18e42b1677ee9bb14255583f5b3624d530a) Conflicts: war/pom.xml --- war/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/war/pom.xml b/war/pom.xml index 0d4edfac91..df6a970a2c 100644 --- a/war/pom.xml +++ b/war/pom.xml @@ -103,7 +103,7 @@ THE SOFTWARE. --> org.jenkins-ci winstone - 0.9.10-jenkins-47 + 2.8 test