Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-37599

f:repeatable mishandles minimum (was: Custom Groovy Script for Promoted Builds always includes bogus classpath entry)

      When trying to add a custom groovy script to a promotion process on windows, an invalid classpath is set preventing the script from running. If you delete the classpath, it returns on the next save of the project.

          [JENKINS-37599] f:repeatable mishandles minimum (was: Custom Groovy Script for Promoted Builds always includes bogus classpath entry)

          Jesse Glick added a comment -

          Reproduced on Linux in 2.7.3. The repeatable list is always showing an entry even where there should be none.

          Jesse Glick added a comment - Reproduced on Linux in 2.7.3. The repeatable list is always showing an entry even where there should be none.

          Jesse Glick added a comment -

          Also reproduced in 1.580.1. But only with Promoted Builds; not, for example, with Groovy Post-Build.

          Jesse Glick added a comment - Also reproduced in 1.580.1. But only with Promoted Builds; not, for example, with Groovy Post-Build.

          Jesse Glick added a comment -

          I think I know what is going on. This code sets minimum=1, intended for the activeItems entry. This code does not bother specifying a minimum, since the default is 0, which is what we want. But the variable is mistakenly inherited across forms. Really this should be using attrs.minimum, which this should also propagate.

          Can be worked around in script-security via

          diff --git a/src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript/config.jelly b/src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript/config.jelly
          index d60c722..3154e4d 100644
          --- a/src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript/config.jelly
          +++ b/src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript/config.jelly
          @@ -34,6 +34,6 @@ THE SOFTWARE.
                   <f:checkbox title="${%Use Groovy Sandbox}" default="${!h.hasPermission(app.RUN_SCRIPTS)}" />
               </f:entry>
               <f:entry title="${%Additional classpath}" field="classpath">
          -        <f:repeatableProperty add="${%Add entry}" header="${%Classpath entry}" field="classpath"/>
          +        <f:repeatableProperty add="${%Add entry}" header="${%Classpath entry}" field="classpath" minimum="0"/>
               </f:entry>
           </j:jelly>
          

          but really this plugin is not at fault, and there may be many other promotion conditions which also suffer. A more tolerable workaround would be in promoted-builds, probably specifying

          <j:set var="minimum" value="0"/>
          

          inside the repeatable blocks in JobPropertyImpl/config.jelly and PublisherImpl/config.jelly, the two places where minimum is specified. But should also be fixed in core.

          Jesse Glick added a comment - I think I know what is going on. This code sets minimum=1 , intended for the activeItems entry. This code does not bother specifying a minimum , since the default is 0, which is what we want. But the variable is mistakenly inherited across forms. Really this should be using attrs.minimum , which this should also propagate. Can be worked around in script-security via diff --git a/src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript/config.jelly b/src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript/config.jelly index d60c722..3154e4d 100644 --- a/src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript/config.jelly +++ b/src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript/config.jelly @@ -34,6 +34,6 @@ THE SOFTWARE. <f:checkbox title= "${%Use Groovy Sandbox}" default= "${!h.hasPermission(app.RUN_SCRIPTS)}" /> </f:entry> <f:entry title= "${%Additional classpath}" field= "classpath" > - <f:repeatableProperty add= "${%Add entry}" header= "${%Classpath entry}" field= "classpath" /> + <f:repeatableProperty add= "${%Add entry}" header= "${%Classpath entry}" field= "classpath" minimum= "0" /> </f:entry> </j:jelly> but really this plugin is not at fault, and there may be many other promotion conditions which also suffer. A more tolerable workaround would be in promoted-builds , probably specifying <j:set var= "minimum" value= "0" /> inside the repeatable blocks in JobPropertyImpl/config.jelly and PublisherImpl/config.jelly , the two places where minimum is specified. But should also be fixed in core.

          Jesse Glick added a comment -

          Not exactly critical since there is a straightforward workaround: remove the stray path when you (re-)configure the project.

          Jesse Glick added a comment - Not exactly critical since there is a straightforward workaround: remove the stray path when you (re-)configure the project.

          Jesse Glick added a comment -

          Filed a PR in Script Security to fail fast and report the issue number.

          Jesse Glick added a comment - Filed a PR in Script Security to fail fast and report the issue number.

          Code changed in jenkins
          User: Jesse Glick
          Path:
          src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntry.java
          src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScriptTest.java
          http://jenkins-ci.org/commit/script-security-plugin/246c22ed034258a4808da0396cb70413ae39c324
          Log:
          JENKINS-37599 Prevent empty classpath entries from being saved accidentally.

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntry.java src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScriptTest.java http://jenkins-ci.org/commit/script-security-plugin/246c22ed034258a4808da0396cb70413ae39c324 Log: JENKINS-37599 Prevent empty classpath entries from being saved accidentally.

          Code changed in jenkins
          User: Jesse Glick
          Path:
          src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntry.java
          src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScriptTest.java
          src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntryTest.java
          http://jenkins-ci.org/commit/script-security-plugin/07d1063094fdef9e090d31e6527e63f3910301cd
          Log:
          Merge pull request #84 from jglick/empty-ClasspathEntry-JENKINS-37599

          JENKINS-37599 Prevent empty classpath entries from being saved accidentally

          Compare: https://github.com/jenkinsci/script-security-plugin/compare/feb65ce6d6c2...07d1063094fd

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntry.java src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScriptTest.java src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntryTest.java http://jenkins-ci.org/commit/script-security-plugin/07d1063094fdef9e090d31e6527e63f3910301cd Log: Merge pull request #84 from jglick/empty-ClasspathEntry- JENKINS-37599 JENKINS-37599 Prevent empty classpath entries from being saved accidentally Compare: https://github.com/jenkinsci/script-security-plugin/compare/feb65ce6d6c2...07d1063094fd

          jglick Thanks for looking into this. Your proposed work around didn't work for me. Even if the class path was not shown in the UI, it was saved into config.xml thus causing the build to fail.

          Michael Fowler added a comment - jglick Thanks for looking into this. Your proposed work around didn't work for me. Even if the class path was not shown in the UI, it was saved into config.xml thus causing the build to fail.

          I have the same issue

          Ebrahim Moshaya added a comment - I have the same issue

          jglick Any update on this?

          Ebrahim Moshaya added a comment - jglick Any update on this?

          Valerie Fernandes added a comment - - edited

          jglick : we also have this issue. Any update when it will be resolved?

           

          Valerie Fernandes added a comment - - edited jglick : we also have this issue. Any update when it will be resolved?  

          Jesse Glick added a comment -

          emoshaya_cognitoiq valeriefdes please do not add “me-too” comments. If an issue is not marked In Progress, no one is working on it. It may or may not ever be resolved. There is far more code in the Jenkins source base than there are (competent) developers to fix even serious bugs. Use the “votes” feature in JIRA to silently register your interest, and/or file a PR with a tested fix if you have Jenkins plugin development expertise.

          Jesse Glick added a comment - emoshaya_cognitoiq valeriefdes please do not add “me-too” comments. If an issue is not marked In Progress, no one is working on it. It may or may not ever be resolved. There is far more code in the Jenkins source base than there are (competent) developers to fix even serious bugs. Use the “votes” feature in JIRA to silently register your interest, and/or file a PR with a tested fix if you have Jenkins plugin development expertise.

          Giuseppe Verdoliva added a comment - Fix this issue: https://github.com/jenkinsci/script-security-plugin/pull/208

          ikedam added a comment -

          This looks an issue of Jenkins core rather than that of script-security or promoted-build, and should be fixed in core.

          ikedam added a comment - This looks an issue of Jenkins core rather than that of script-security or promoted-build, and should be fixed in core.

          ikedam added a comment -

          ikedam added a comment - https://github.com/jenkinsci/jenkins/pull/3583

          Devin Nusbaum added a comment -

          Looks like the fix was recently released in Jenkins 2.139. Thanks ikedam!

          Devin Nusbaum added a comment - Looks like the fix was recently released in Jenkins 2.139 . Thanks ikedam !

            ikedam ikedam
            mdkf Michael Fowler
            Votes:
            8 Vote for this issue
            Watchers:
            14 Start watching this issue

              Created:
              Updated:
              Resolved: