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

Parameterized trigger plugin evaluates null or unset variables as their symbol name.

    XMLWordPrintable

    Details

    • Similar Issues:

      Description

      I have 2 jobs Job A and Job B.

      1. Job A does some logic and then injects some variables into the build via the inject variable plugin. The only variable that is set is $PARAM1, and occasionally $PARAM2
      2. Job A then runs a parameterized trigger for Job B. Job B requires two parameters: PARAM1 and PARAM2.  Both parameters of this job default to blank strings.  The trigger is parameterized and passes the following "pre-defined" parameters to Job B as follows:
      PARAM1=$PARAM1
      PARAM2=$PARAM2
      See attached photo:

      3. While parsing the parameters for Job B, because PARAM2 is not defined. Instead of passing a blank string to Job B, PARAM2 is set to the literal string '$PARAM2'.
      4. This then causes a world of chaos in Job B because not only is PARAM2 not set to a blank string, but it is also set to a wildly incorrect value.
      Echoing out these values in Job B will show the following:
      The parameter PARAM1="I was set and injected in the last job"
      The parameter PARAM2="$PARAM2"

      The fix for this seems simple enough and that is to evaluate all symbols that aren't set as either blank strings "" or refuse to set the variable in the downstream project allowing the default parameter to take precedence (Maybe configurable?)

        Attachments

          Activity

          Hide
          rusty Russell Weber added a comment -

          Important note: if PARAM2 is set, then the variable gets passed down to Job B as expected, this only affects the case where the parameter is not set.

          Show
          rusty Russell Weber added a comment - Important note: if PARAM2 is set, then the variable gets passed down to Job B as expected, this only affects the case where the parameter is not set.
          Hide
          rusty Russell Weber added a comment -

          I believe I have found the offending piece of code.

          @Nullable
          public static String replaceMacro(@CheckForNull String s, @Nonnull VariableResolver<String> resolver) {
              if (s == null) {
                  return null;
              } else {
                  int idx = 0;
          
                  while(true) {
                      Matcher m = VARIABLE.matcher(s);
                      if (!m.find(idx)) {
                          return s;
                      }
          
                      String key = m.group().substring(1);
                      String value;
                      if (key.charAt(0) == '$') {
                          value = "$";
                      } else {
                          if (key.charAt(0) == '{') {
                              key = key.substring(1, key.length() - 1);
                          }
          
                          value = (String)resolver.resolve(key);
                      }
          
                      if (value == null) {
                          idx = m.end();
                      } else {
                          s = s.substring(0, m.start()) + value + s.substring(m.end());
                          idx = m.start() + value.length();
                      }
                  }
              }
          }

          In this segment of code, if I understood it correctly, there is a segment of code that attempts to match the string s with a variable name.  If the variable is not found, then s is returned.  However, it seems to me that this should include additional logic to check to see if the string is attempting to reference a variable and since the matcher could not find a corresponding variable, that it should be set at some reasonable default and not just returned back.

           

          Show
          rusty Russell Weber added a comment - I believe I have found the offending piece of code. @Nullable public static String replaceMacro(@CheckForNull String s, @Nonnull VariableResolver< String > resolver) { if (s == null ) { return null ; } else { int idx = 0; while ( true ) { Matcher m = VARIABLE.matcher(s); if (!m.find(idx)) { return s; } String key = m.group().substring(1); String value; if (key.charAt(0) == '$' ) { value = "$" ; } else { if (key.charAt(0) == '{' ) { key = key.substring(1, key.length() - 1); } value = ( String )resolver.resolve(key); } if (value == null ) { idx = m.end(); } else { s = s.substring(0, m.start()) + value + s.substring(m.end()); idx = m.start() + value.length(); } } } } In this segment of code, if I understood it correctly, there is a segment of code that attempts to match the string s with a variable name.  If the variable is not found, then s is returned.  However, it seems to me that this should include additional logic to check to see if the string is attempting to reference a variable and since the matcher could not find a corresponding variable, that it should be set at some reasonable default and not just returned back.  

            People

            Assignee:
            Unassigned Unassigned
            Reporter:
            rusty Russell Weber
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

              Dates

              Created:
              Updated: