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

Field validation does not pass required query parameters when some fields are specified in a nested Describable

    XMLWordPrintable

    Details

    • Similar Issues:

      Description

      The collabnet plugin was updated/modernized a couple years ago to include code like the following:

      CNDocumentUploader.java DescriptorImpl

      /**
        * Form validation for upload path.
        */
      public FormValidation doCheckUploadPath(CollabNetApp app, @QueryParameter String project, @QueryParameter String value)
      

      CollabNetApp.java

          public static CollabNetApp fromStapler(@QueryParameter boolean overrideAuth, @QueryParameter String url,
                                                 @QueryParameter String username, @QueryParameter String password) 
      

      config.jelly

        <f:property field="connectionFactory" />
      ...
        <f:entry title="Upload Folder Path" field="uploadPath">
          <f:textbox />
        </f:entry>
      

      ConnectionFactory has url, username, and password properties

      The generated field validation javascript then uses the 'nearBy' function when passing query parameters to the checkUploadPath backend in order to find values of url, username, and password. However uploadPath has a parent element with name 'publisher', while the direct parent of url, username, and password is a div has name 'connectionFactory'. 'publisher' is then a grandparent of url, username, and password and because of this, the needed query parameters are not included.

      One might argue that the collabnet plugin should avoid this design, since it does not work, but the changes came from Kohsuke, so attempting find a fix which preserves the current design.

      A patch to the findNearBy function in hudson-behavior.js is attached.

        Attachments

          Activity

          Hide
          jmcnally John McNally added a comment -

          In case the short presentation of the plugin code is not clear, here is the entire commit:
          https://github.com/jenkinsci/collabnet-plugin/commit/4c7b516a7975ffc126ddf717476cefe4e0a84081

          Show
          jmcnally John McNally added a comment - In case the short presentation of the plugin code is not clear, here is the entire commit: https://github.com/jenkinsci/collabnet-plugin/commit/4c7b516a7975ffc126ddf717476cefe4e0a84081
          Hide
          kohsuke Kohsuke Kawaguchi added a comment -

          Thanks for doing the detective work and the patch, and my apologies for introducing the problem in the first place.

          See https://github.com/jenkinsci/jenkins/pull/487 for how we fixed this. More specifically, https://github.com/jenkinsci/jenkins/commit/99b11d0df29ba6428a64486a8335a8f55a2c3d27 contains the UI sample for how to use this.

          In this case, parameters in CollabNetApp.fromStapler would each need @RelativePath("connectionFactory")

          Show
          kohsuke Kohsuke Kawaguchi added a comment - Thanks for doing the detective work and the patch, and my apologies for introducing the problem in the first place. See https://github.com/jenkinsci/jenkins/pull/487 for how we fixed this. More specifically, https://github.com/jenkinsci/jenkins/commit/99b11d0df29ba6428a64486a8335a8f55a2c3d27 contains the UI sample for how to use this. In this case, parameters in CollabNetApp.fromStapler would each need @RelativePath("connectionFactory")
          Hide
          dharmsheta Dharmesh Sheta added a comment -

          Hi Kohsuke,

          unfortunately, adding @RelativePath("connectionFactory") did not work with our original jelly file:

          <j:choose>
          <j:when test="${descriptor.canInheritAuth()}">
          <f:nested>
          <table style="width:100%">
          <f:optionalProperty title="Override the Global CollabNet Configuration" field="connectionFactory" />
          </table>
          </f:nested>
          </j:when>
          <j:otherwise>
          <f:property field="connectionFactory" />
          </j:otherwise>
          </j:choose>

          Stapler seems to have a problem with the fact that connectionFactory is not always present but nested in a j:choose element.

          FWIW, we modified our jelly like this and got the parameters we are looking for:

          <f:property field="connectionFactory" />

          However, this does not solve our problem because we only want connection factory to show up under certain conditions. How do we have to modify the @RelativePath annotation parameter to work with our original jelly file?

          Show
          dharmsheta Dharmesh Sheta added a comment - Hi Kohsuke, unfortunately, adding @RelativePath("connectionFactory") did not work with our original jelly file: <j:choose> <j:when test="${descriptor.canInheritAuth()}"> <f:nested> <table style="width:100%"> <f:optionalProperty title="Override the Global CollabNet Configuration" field="connectionFactory" /> </table> </f:nested> </j:when> <j:otherwise> <f:property field="connectionFactory" /> </j:otherwise> </j:choose> Stapler seems to have a problem with the fact that connectionFactory is not always present but nested in a j:choose element. FWIW, we modified our jelly like this and got the parameters we are looking for: <f:property field="connectionFactory" /> However, this does not solve our problem because we only want connection factory to show up under certain conditions. How do we have to modify the @RelativePath annotation parameter to work with our original jelly file?
          Hide
          jonico Johannes Nicolai added a comment -

          assigning to Kohsuke with the hope that he looks at the latest comment (please tell us if we should follow a different workflow to notify you in the future)

          Show
          jonico Johannes Nicolai added a comment - assigning to Kohsuke with the hope that he looks at the latest comment (please tell us if we should follow a different workflow to notify you in the future)
          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: John McNally
          Path:
          src/main/java/com/collabnet/ce/webservices/CTFProject.java
          src/main/java/com/collabnet/ce/webservices/CollabNetApp.java
          src/main/java/hudson/plugins/collabnet/ConnectionFactory.java
          src/main/java/hudson/plugins/collabnet/auth/CNAuthProjectProperty.java
          src/main/java/hudson/plugins/collabnet/browser/TeamForge.java
          src/main/java/hudson/plugins/collabnet/tracker/CNTracker.java
          src/main/java/hudson/plugins/collabnet/util/CNFormFieldValidator.java
          src/main/java/hudson/plugins/collabnet/util/ComboBoxUpdater.java
          http://jenkins-ci.org/commit/collabnet-plugin/028c3cf91ffa39a544f98cb287d9554beead76e1
          Log:
          JENKINS-13742 Field validation does not pass required query parameters
          when some fields are specified in a nested Describable

          1. On branch master
          2. Changes not staged for commit:
          3. (use "git add <file>..." to update what will be committed)
          4. (use "git checkout – <file>..." to discard changes in working directory)
          • src/main/java/com/collabnet/ce/webservices/CTFProject.java
            (getOrCreateDocumentFolder, verifyPath): strip file separators from beginning and end of path
          • src/main/java/com/collabnet/ce/webservices/CollabNetApp.java
            (fromStapler): renamed overrideAuth to connectionFactory, the former is more descriptive,
            but the checkbox takes the name of the composite field. Added RelativeParameter
            annotations to the other arguments. Also the password field may contain the encrypted
            form, so using Secret to transform into plaintext.
          • src/main/java/hudson/plugins/collabnet/ConnectionFactory.java
            Add a fromStapler method so this can be used in validations that are internal to the
            composite field, such as the modified doCheckPassword method.
          • src/main/java/hudson/plugins/collabnet/auth/CNAuthProjectProperty.java
            (doFillProjectItems): simplified by using getProjectList method.
          • src/main/java/hudson/plugins/collabnet/browser/TeamForge.java
            (doFillProjectItems, doFillRepoItems): Added code to logoff the CTF session
          • src/main/java/hudson/plugins/collabnet/tracker/CNTracker.java
            (doFillTrackerItems): Add a null check to avoid NPE, if auth data is not found
          • src/main/java/hudson/plugins/collabnet/util/CNFormFieldValidator.java
            Several methods are modified to add null checks for the CollabNetAuth arg and to
            ensure the session is logged off. Prior to this change some methods did one
            and others did the other and some did neither.
          • src/main/java/hudson/plugins/collabnet/util/ComboBoxUpdater.java
            (getUsers): Add null check on CollabNetApp arg
          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: John McNally Path: src/main/java/com/collabnet/ce/webservices/CTFProject.java src/main/java/com/collabnet/ce/webservices/CollabNetApp.java src/main/java/hudson/plugins/collabnet/ConnectionFactory.java src/main/java/hudson/plugins/collabnet/auth/CNAuthProjectProperty.java src/main/java/hudson/plugins/collabnet/browser/TeamForge.java src/main/java/hudson/plugins/collabnet/tracker/CNTracker.java src/main/java/hudson/plugins/collabnet/util/CNFormFieldValidator.java src/main/java/hudson/plugins/collabnet/util/ComboBoxUpdater.java http://jenkins-ci.org/commit/collabnet-plugin/028c3cf91ffa39a544f98cb287d9554beead76e1 Log: JENKINS-13742 Field validation does not pass required query parameters when some fields are specified in a nested Describable On branch master Changes not staged for commit: (use "git add <file>..." to update what will be committed) (use "git checkout – <file>..." to discard changes in working directory) src/main/java/com/collabnet/ce/webservices/CTFProject.java (getOrCreateDocumentFolder, verifyPath): strip file separators from beginning and end of path src/main/java/com/collabnet/ce/webservices/CollabNetApp.java (fromStapler): renamed overrideAuth to connectionFactory, the former is more descriptive, but the checkbox takes the name of the composite field. Added RelativeParameter annotations to the other arguments. Also the password field may contain the encrypted form, so using Secret to transform into plaintext. src/main/java/hudson/plugins/collabnet/ConnectionFactory.java Add a fromStapler method so this can be used in validations that are internal to the composite field, such as the modified doCheckPassword method. src/main/java/hudson/plugins/collabnet/auth/CNAuthProjectProperty.java (doFillProjectItems): simplified by using getProjectList method. src/main/java/hudson/plugins/collabnet/browser/TeamForge.java (doFillProjectItems, doFillRepoItems): Added code to logoff the CTF session src/main/java/hudson/plugins/collabnet/tracker/CNTracker.java (doFillTrackerItems): Add a null check to avoid NPE, if auth data is not found src/main/java/hudson/plugins/collabnet/util/CNFormFieldValidator.java Several methods are modified to add null checks for the CollabNetAuth arg and to ensure the session is logged off. Prior to this change some methods did one and others did the other and some did neither. src/main/java/hudson/plugins/collabnet/util/ComboBoxUpdater.java (getUsers): Add null check on CollabNetApp arg
          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: jmcnally
          Path:
          src/main/java/com/collabnet/ce/webservices/CTFProject.java
          src/main/java/com/collabnet/ce/webservices/CollabNetApp.java
          src/main/java/hudson/plugins/collabnet/ConnectionFactory.java
          src/main/java/hudson/plugins/collabnet/auth/CNAuthProjectProperty.java
          src/main/java/hudson/plugins/collabnet/browser/TeamForge.java
          src/main/java/hudson/plugins/collabnet/tracker/CNTracker.java
          src/main/java/hudson/plugins/collabnet/util/CNFormFieldValidator.java
          src/main/java/hudson/plugins/collabnet/util/ComboBoxUpdater.java
          http://jenkins-ci.org/commit/collabnet-plugin/70070f6d7d3745e2af30756f1152eca690e1f60f
          Log:
          Merge pull request #1 from jmcnally/master

          JENKINS-13742 Errors in field validations

          Compare: https://github.com/jenkinsci/collabnet-plugin/compare/d13d8d2554c3...70070f6d7d37


          You received this message because you are subscribed to the Google Groups "Jenkins Commits" group.
          To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-commits+unsubscribe@googlegroups.com.
          For more options, visit https://groups.google.com/groups/opt_out.

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: jmcnally Path: src/main/java/com/collabnet/ce/webservices/CTFProject.java src/main/java/com/collabnet/ce/webservices/CollabNetApp.java src/main/java/hudson/plugins/collabnet/ConnectionFactory.java src/main/java/hudson/plugins/collabnet/auth/CNAuthProjectProperty.java src/main/java/hudson/plugins/collabnet/browser/TeamForge.java src/main/java/hudson/plugins/collabnet/tracker/CNTracker.java src/main/java/hudson/plugins/collabnet/util/CNFormFieldValidator.java src/main/java/hudson/plugins/collabnet/util/ComboBoxUpdater.java http://jenkins-ci.org/commit/collabnet-plugin/70070f6d7d3745e2af30756f1152eca690e1f60f Log: Merge pull request #1 from jmcnally/master JENKINS-13742 Errors in field validations Compare: https://github.com/jenkinsci/collabnet-plugin/compare/d13d8d2554c3...70070f6d7d37 – You received this message because you are subscribed to the Google Groups "Jenkins Commits" group. To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-commits+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out .
          Hide
          jmcnally John McNally added a comment -

          Kohsuke,
          As you pointed out there was some work in the collabnet plugin which could improve this behavior and I have used this issue for a commit to the plugin. However, there are a couple of issues in the core that still prevent the plugin from optimium behavior. Feel free to assign this issue to me. But could you look at JENKINS-16676 and JENKINS-16719 for bugs in core Jenkins?

          Show
          jmcnally John McNally added a comment - Kohsuke, As you pointed out there was some work in the collabnet plugin which could improve this behavior and I have used this issue for a commit to the plugin. However, there are a couple of issues in the core that still prevent the plugin from optimium behavior. Feel free to assign this issue to me. But could you look at JENKINS-16676 and JENKINS-16719 for bugs in core Jenkins?
          Hide
          jmcnally John McNally added a comment -

          Committing the fix for JENKINS-16676 would allow override of the global auth credentials, but the work that can be accomplished in the plugin is complete.

          Show
          jmcnally John McNally added a comment - Committing the fix for JENKINS-16676 would allow override of the global auth credentials, but the work that can be accomplished in the plugin is complete.

            People

            • Assignee:
              jmcnally John McNally
              Reporter:
              jmcnally John McNally
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: