Merge pull request #62 from synopsys-arc-oss/JENKINS-25732

[FIXED JENKINS-25732] - Don't fail with NPE if an environment variable has null value
jenkinsci · Nov 30, 2014 · 80360f0 · 80360f0
2 parents b3c84d1 + 45392c6
commit 80360f0
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 1 deletion.
diff --git a/src/main/java/hudson/plugins/perforce/utils/MacroStringHelper.java b/src/main/java/hudson/plugins/perforce/utils/MacroStringHelper.java
@@ -182,7 +182,12 @@ private static String substituteParametersNoCheck(
         }
         String newString = string;
         for (Map.Entry<String, String> entry : subst.entrySet()) {
-            newString = newString.replace("${" + entry.getKey() + "}", entry.getValue());
+            final @CheckForNull String key = entry.getKey();
+            final @CheckForNull String value = entry.getValue(); 
+            if (key == null || value == null) {
+                continue;
+            }
+            newString = newString.replace("${" + key + "}", value);
         }
         return newString;
     }

diff --git a/src/test/java/com/synopsys/arc/jenkinsci/plugins/perforce/MacroStringHelperTest.java b/src/test/java/com/synopsys/arc/jenkinsci/plugins/perforce/MacroStringHelperTest.java
@@ -23,6 +23,7 @@
  */
 package com.synopsys.arc.jenkinsci.plugins.perforce;
 
+import hudson.EnvVars;
 import hudson.plugins.perforce.utils.MacroStringHelper;
 import hudson.plugins.perforce.utils.ParameterSubstitutionException;
 import javax.annotation.Nonnull;
@@ -72,6 +73,17 @@ public void Sanity_CheckNull() {
         checkStringForMacros("//depot1/path1/... //placeholder/path1/...\n//depot1/path2/... //placeholder/path2/...", false);
     }
 
+    @Bug(25732)
+    public @Test void nullEntriesInEnvVars() throws ParameterSubstitutionException {   
+        // Check that params with null values are being ignored
+        try {
+           MacroStringHelper.substituteParameters("Test sring with ${PARAM}", new EnvVars("PARAM", null));
+        } catch (ParameterSubstitutionException ex) {
+            return; // OK
+        }
+        Assert.fail("Expected ParameterSubstitutionException on null parameter value");
+    }
+
     public static void checkStringForMacros(@Nonnull String string, boolean expectMacro) {
         boolean isMacro = MacroStringHelper.containsMacro(string);
         if (isMacro != expectMacro) {