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

Quotes in Gerrit parameters not escaped

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Component/s: gerrit-trigger-plugin
    • Labels:
      None

      Description

      If there are quotes in the Gerrit change subject, they get passed on when the Ant builder (or, I think, Maven builder) are invoked, but the escaping comes out badly - we should probably escape the quotes in any string, if possible.

      ant -file sauce-parallel-runner.xml '-DGERRIT_CHANGE_SUBJECT=Some small changes to "real time" digg count updates:' -DGERRIT_REFSPEC=refs/changes/63/2263/4 -DGERRIT_BRANCH=master -DGERRIT_PATCHSET_NUMBER=4 -DGERRIT_CHANGE_URL=http://review.digg.internal:8080/2263 -DGERRIT_CHANGE_ID=Ie4d4ec03a3576898edc8445a59517ac47a5960b9 -DGERRIT_PATCHSET_REVISION=802b31a9f5da030306746f24bd5906fec6d5d613 -DGERRIT_CHANGE_NUMBER=2263 -DGERRIT_PROJECT=bobcat -Dvhost.subdomain=$NODE_NAME "-Dreplace.twist.tags=!digg, !unstable, !in-progress, smoke" -Dvhost.domain=digg.internal -Dgit.branch=master -Dgit.repo=qa/bobtwist sauce
      

        Activity

        Hide
        rsandell rsandell added a comment -

        Can we really escape it all the time?
        I agree that it needs to be done when for example passed as a parameter to a script of some sort, but there might be scenarios when you don't want it escaped.
        Perhaps if you just want to echo it to the build log or if some other plugin is using the parameters it might be "tricky" to un-escape it.

        I'm not against it, just trying to see if there is a scenario when the escaped string can cause trouble.

        Show
        rsandell rsandell added a comment - Can we really escape it all the time? I agree that it needs to be done when for example passed as a parameter to a script of some sort, but there might be scenarios when you don't want it escaped. Perhaps if you just want to echo it to the build log or if some other plugin is using the parameters it might be "tricky" to un-escape it. I'm not against it, just trying to see if there is a scenario when the escaped string can cause trouble.
        Hide
        rsandell rsandell added a comment -

        So I'm thinking that the parameters should be escaped by default, at least the subject.
        And then an "advanced option" on the trigger config to disable it doing so.

        But my time is very limited at the moment, so I don't know when I will have the time to implement it.

        Show
        rsandell rsandell added a comment - So I'm thinking that the parameters should be escaped by default, at least the subject. And then an "advanced option" on the trigger config to disable it doing so. But my time is very limited at the moment, so I don't know when I will have the time to implement it.
        Hide
        rsandell rsandell added a comment -

        commit: ca4ae084123983e0320d
        Released in version 2.3.0

        Show
        rsandell rsandell added a comment - commit: ca4ae084123983e0320d Released in version 2.3.0

          People

          • Assignee:
            rsandell rsandell
            Reporter:
            abayer abayer
          • Votes:
            1 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: