Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trigger phrase accumulates unwanted escape characters #197

Closed
jdonald opened this issue Sep 24, 2015 · 19 comments
Closed

Trigger phrase accumulates unwanted escape characters #197

jdonald opened this issue Sep 24, 2015 · 19 comments

Comments

@jdonald
Copy link

jdonald commented Sep 24, 2015

We are using a regular expression in the trigger phrase via the new feature in #157. However, it tends to stop functioning after each day as the trigger phrase accumulates special characters. For example, .ok to sim. becomes \Q._ok to sim._\E

image

This makes the regular expression no longer matching phrases it did before like ok to simulate.

The accumulated characters actually happen on non-regexp phrases too, but seem to be less of a problem when not concatenated with .*

image

@DavidTanner said to post the config.xml configVersion value and it should be three.

  <scm class="hudson.plugins.git.GitSCM" plugin="git@2.4.0">
    <configVersion>2</configVersion>

The version is two. Could that be tied to the problem?

@DavidTanner
Copy link
Contributor

Please check with 1.28.6

@jdonald
Copy link
Author

jdonald commented Sep 25, 2015

We upgraded, restarted Jenkins, and removed \Q and \E from the trigger phrase. However, the characters were added spontaneously again, possibly when we restarted Jenkins a second time.

Last time I only noticed one configVersion in config.xml, but perhaps I didn't search completely. AS of now there are several, many of which are not 3.


      <matrixOptions>
        <throttleMatrixBuilds>true</throttleMatrixBuilds>
        <throttleMatrixConfigurations>false</throttleMatrixConfigurations>
      </matrixOptions>
      <configVersion>1</configVersion>

and slightly below that:

  <scm class="hudson.plugins.git.GitSCM" plugin="git@2.4.0">
    <configVersion>2</configVersion>

The one next to the trigger phrase does say 3 though :

  <triggers>
    <org.jenkinsci.plugins.ghprb.GhprbTrigger plugin="ghprb@1.28.6">
      <spec>*/1 * * * *</spec>
      <triggerPhrase>\Q.*ok to simul.*\E</triggerPhrase>
      <configVersion>3</configVersion>

@DavidTanner
Copy link
Contributor

So once the version is 3 the plugin won't change the trigger phrase anymore. The idea was that if there were regex characters in the trigger phrase that weren't intended for a regex then they should be escaped. Once it is on configVersion 3, just for the trigger, then it won't touch the value any more.

As a final test, if you were to restart Jenkins now you would not have the triggerPhrase be changed anymore.

@jdonald
Copy link
Author

jdonald commented Sep 30, 2015

Last Friday we restarted our Jenkins master several times and it seemed to accumulate \Q and \E characters regardless, which I had to then promptly remove. Nonetheless I opted to let it bake over the weekend and it was pleasing to see no unwanted escape characters had appeared again up through yesterday.

However, this morning it is back. \Q.*ok to simul.*\E as a trigger phrase, so this is not fixed on our end. The pluginManager still says we still have version 1.28.6 of ghprb.

Anything else I can look at for troubleshooting?

@DavidTanner
Copy link
Contributor

This is fixed in 1.29 The configVersion isn't saved until each job config is saved again, except in the new code the configVersion is saved when the trigger starts.

@jdonald
Copy link
Author

jdonald commented Oct 2, 2015

Since 1.29, we haven't seen this issue recur and it has been a few days. Will reopen if that changes.

@jdonald jdonald closed this as completed Oct 2, 2015
@mmitche
Copy link
Contributor

mmitche commented Oct 9, 2015

@DavidTanner What happens if the Job DSL plugin generates a job that doesn't have configVersion set (which is what happens today)? We're seeing this in our CI, which uses generated jobs. I am thinking that the DSL plugin needs to set the configVersion.

@mmitche
Copy link
Contributor

mmitche commented Oct 9, 2015

/cc @tannergooding

@mmitche
Copy link
Contributor

mmitche commented Oct 9, 2015

@DavidTanner Okay that makes sense now. so would that mean that if you used the job DSL plugin to create a new job and it didn't set that field, that when that gets initially saved it would have configVersion 1?

@mmitche
Copy link
Contributor

mmitche commented Oct 9, 2015

(which would then get incremented after saving again?)

@DavidTanner
Copy link
Contributor

I don't know how that plugin works, but it would start out with whatever version it is created with, run the checks on the versions, and finally update the version to 3. When the trigger is started afterwards that version would be saved to a config.xml, so if each job is created on the fly, then the check would run every time.

@mmitche
Copy link
Contributor

mmitche commented Oct 9, 2015

@DavidTanner I want to understand the change that was made so I can make the right change in the DSL plugin. Prior to configVersion 3, was it not using a regex? Why would quoting version < 3 be the right thing but not for version == 3

@DavidTanner
Copy link
Contributor

before version 3 the trigger phrase was just matching the string directly, as of 3 it is a regex. I am assuming the pre version 3 you didn't intend for the trigger to match regex special characters.

@mmitche
Copy link
Contributor

mmitche commented Oct 9, 2015

Oh oh okay. Makes perfect sense now. So, the right thing to do here is to update the DSL plugin to generate config version 3 directly.

@jmdh
Copy link
Contributor

jmdh commented Oct 14, 2015

FWIW, we're seeing the behaviour described at the top of this issue; when the job config is saved manually (with no changes) the configVersion is being reset to 1, and then when Jenkins is restarted, the doubled up quote characters appear.We're running 1.29 (and Jenkins 1.633).

@jdonald
Copy link
Author

jdonald commented Oct 14, 2015

Thanks @jmdh. We had no issues for a week or so, then it recurred and when I investigated the configVersion was set back to 1. I don't know what triggered that, but possibly one of our admins saved it with no changes, and yes we're still on 1.29.

I'll go ahead and reopen this as it has been confirmed for multiple parties.

@jdonald jdonald reopened this Oct 14, 2015
@jdonald
Copy link
Author

jdonald commented Oct 22, 2015

We have not seen the issue recur in the past 8 days after upgrading to 1.29.2, so I believe this is indeed fixed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants