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

Cannot use spaces in groovy script parameters

    Details

    • Similar Issues:

      Description

      The issue is caused by oversimplified parameters parsing implementation in this class: https://github.com/jenkinsci/groovy-plugin/blob/master/src/main/java/hudson/plugins/groovy/Groovy.java

              //Add groovy parameters
              if(parameters != null) {
                  StringTokenizer tokens = new StringTokenizer(parameters);
                  while(tokens.hasMoreTokens()) {
                      list.add(Util.replaceMacro(tokens.nextToken(),vr));
                  }
              }
      

      I would suggest to switch to org.apache.commons.exec.CommandLine.parse(..) method from Apache commons-exec.
      Not ideal, I know, but probably the best possible option if parsing command-line is inevitable.

      Is anyone interested in the patch? It would be very straightforward.

        Attachments

          Issue Links

            Activity

            enterit enterit created issue -
            Hide
            vjuranek vjuranek added a comment -

            using some groovy executable parameters with spaces seems to me quite rare use case for which using CommandLine seems to me like an overkill. Would some more simple solution like quoting parameter with spaces and using e.g. reg. exp. for parsing solve your problem?

            Show
            vjuranek vjuranek added a comment - using some groovy executable parameters with spaces seems to me quite rare use case for which using CommandLine seems to me like an overkill. Would some more simple solution like quoting parameter with spaces and using e.g. reg. exp. for parsing solve your problem?
            Hide
            danielbeck Daniel Beck added a comment - - edited

            (stupid comment)

            Show
            danielbeck Daniel Beck added a comment - - edited (stupid comment)
            Hide
            enterit enterit added a comment -

            Why do you think using spaces is a quite rare use case? It may happen quite often, especially if other build params are passed to the script, like job description, text chunks etc.
            There is any easy workaround if you specify script text. But not if you specify script name to execute.
            If adding this new dependency is undesirable, we could copy CommandLine.parse(..) implementation. Ideally we should not be parsing command line at all, but hudson.Launcher.ProcStarter does not seem to accept single-string command.
            I do not think there is an easy way to parse command line using regexp - it can be error-prone.

            Show
            enterit enterit added a comment - Why do you think using spaces is a quite rare use case? It may happen quite often, especially if other build params are passed to the script, like job description, text chunks etc. There is any easy workaround if you specify script text. But not if you specify script name to execute. If adding this new dependency is undesirable, we could copy CommandLine.parse(..) implementation. Ideally we should not be parsing command line at all, but hudson.Launcher.ProcStarter does not seem to accept single-string command. I do not think there is an easy way to parse command line using regexp - it can be error-prone.
            Hide
            vjuranek vjuranek added a comment -

            > Why do you think using spaces is a quite rare use case?
            based on the fact, that I've never heard someone needs it before (well, we have pretty large deployment and use groovy heavily, so I thought that most of the usecases are already covered by complains of my co-workers)

            > I do not think there is an easy way to parse command line using regexp - it can be error-prone.
            agree that it could be error-prone. Will look into CommandLine implementation and other possibilities (if I find any) over the weekend.

            Show
            vjuranek vjuranek added a comment - > Why do you think using spaces is a quite rare use case? based on the fact, that I've never heard someone needs it before (well, we have pretty large deployment and use groovy heavily, so I thought that most of the usecases are already covered by complains of my co-workers) > I do not think there is an easy way to parse command line using regexp - it can be error-prone. agree that it could be error-prone. Will look into CommandLine implementation and other possibilities (if I find any) over the weekend.
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Vojtech Juranek
            Path:
            pom.xml
            src/main/java/hudson/plugins/groovy/Groovy.java
            http://jenkins-ci.org/commit/groovy-plugin/800175fb90c15f6bd543125ba02e829410667997
            Log:
            [FIXED JENKINS-23617] Improve parsing of paramaters passed to groovy binary

            Compare: https://github.com/jenkinsci/groovy-plugin/compare/7d7972b3f95c^...800175fb90c1

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Vojtech Juranek Path: pom.xml src/main/java/hudson/plugins/groovy/Groovy.java http://jenkins-ci.org/commit/groovy-plugin/800175fb90c15f6bd543125ba02e829410667997 Log: [FIXED JENKINS-23617] Improve parsing of paramaters passed to groovy binary Compare: https://github.com/jenkinsci/groovy-plugin/compare/7d7972b3f95c ^...800175fb90c1
            scm_issue_link SCM/JIRA link daemon made changes -
            Field Original Value New Value
            Status Open [ 1 ] Resolved [ 5 ]
            Resolution Fixed [ 1 ]
            Hide
            vjuranek vjuranek added a comment -

            ups, not intended to close it... Due to lack of time I didn't tested any other libraries and use proposed commons-exec, however this lib is IMHO far from perfect as it e.g. doesn't handle backslashed spaces (e.g. -cp /path/to/my\ groovy/libs splits into 3 args), but this is not subject of this issue and PR should be sent against used library.

            @enterit - please test JENKINS-23617 branch if it fixes your usecase, if yes I'll marge it to main branch and release

            Show
            vjuranek vjuranek added a comment - ups, not intended to close it... Due to lack of time I didn't tested any other libraries and use proposed commons-exec , however this lib is IMHO far from perfect as it e.g. doesn't handle backslashed spaces (e.g. -cp /path/to/my\ groovy/libs splits into 3 args), but this is not subject of this issue and PR should be sent against used library. @enterit - please test JENKINS-23617 branch if it fixes your usecase, if yes I'll marge it to main branch and release
            vjuranek vjuranek made changes -
            Resolution Fixed [ 1 ]
            Status Resolved [ 5 ] Reopened [ 4 ]
            Hide
            vjuranek vjuranek added a comment -

            Merged into master branch and released in 1.19.

            Show
            vjuranek vjuranek added a comment - Merged into master branch and released in 1.19.
            vjuranek vjuranek made changes -
            Status Reopened [ 4 ] Resolved [ 5 ]
            Resolution Fixed [ 1 ]
            Hide
            nigel_charman nigel_charman added a comment -

            Hi, Using 1.20, I'm unable to get parameters with spaces working. I've tried with double and single quotes around the parameter. Can you advise how to get spaces working? thanks

            Show
            nigel_charman nigel_charman added a comment - Hi, Using 1.20, I'm unable to get parameters with spaces working. I've tried with double and single quotes around the parameter. Can you advise how to get spaces working? thanks
            Hide
            danielbeck Daniel Beck added a comment -

            Jira is not a support site. Please ask in #jenkins on Freenode, or on the jenkinsci-users mailing list.

            Show
            danielbeck Daniel Beck added a comment - Jira is not a support site. Please ask in #jenkins on Freenode, or on the jenkinsci-users mailing list.
            Hide
            vjuranek vjuranek added a comment -

            @nigel_charman: there are two parameters options, Groovy parameters - parameter for groovy executable and Script parameters - parameters for script. It was implemented only for the first one, Groovy parameters. I've created JENKINS-24757 for allowing spaces also in Script parameters.

            Show
            vjuranek vjuranek added a comment - @nigel_charman: there are two parameters options, Groovy parameters - parameter for groovy executable and Script parameters - parameters for script. It was implemented only for the first one, Groovy parameters . I've created JENKINS-24757 for allowing spaces also in Script parameters .
            stautz85 Sebastian made changes -
            Link This issue is blocking JENKINS-28960 [ JENKINS-28960 ]
            rtyler R. Tyler Croy made changes -
            Workflow JNJira [ 156400 ] JNJira + In-Review [ 195399 ]

              People

              • Assignee:
                vjuranek vjuranek
                Reporter:
                enterit enterit
              • Votes:
                0 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: