[FIXED JENKINS-15733] ArrayIndexOutOfBoundsException from Launcher du…

…e to misinterpretation of ProcStarter.envs in f5cebdc. (Should this field be made a [LinkedHash]Map<String,String> or even EnvVars? It is protected in a final class, so effectively package-private, thus not a b/w compat issue.)
jenkinsci · Nov 7, 2012 · 58484bb · 58484bb
1 parent e484673
commit 58484bb
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 14 deletions.
diff --git a/changelog.html b/changelog.html
@@ -55,6 +55,9 @@
 <!-- Record your changes in the trunk here. -->
 <div id="trunk" style="display:none"><!--=TRUNK-BEGIN=-->
 <ul class=image>
+  <li class="major bug">
+    <code>ArrayIndexOutOfBoundsException</code> or <code>StringIndexOutOfBoundsException</code> launching processes such as Mercurial polling. (Regression in 1.489.)
+    (<a href="https://issues.jenkins-ci.org/browse/JENKINS-15733">issue 15733</a>)
   <li class=rfe>
     Support failsafe the same way as surefire in maven2 jobs
 </ul>

diff --git a/core/src/main/java/hudson/Launcher.java b/core/src/main/java/hudson/Launcher.java
@@ -51,7 +51,6 @@
 import java.util.Map;
 import java.util.List;
 import java.util.ArrayList;
-import java.util.Map.Entry;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -279,20 +278,28 @@ public InputStream stdin() {
          * becomes the "current" process), these variables will be also set.
          */
         public ProcStarter envs(Map<String, String> overrides) {
-            return envs(Util.mapToEnv(overrides));
+            this.envs = Util.mapToEnv(overrides);
+            return this;
         }
 
         /**
          * @param overrides
          *      List of "VAR=VALUE". See {@link #envs(Map)} for the semantics.
          */
         public ProcStarter envs(String... overrides) {
+            if (overrides != null) {
+                for (String override : overrides) {
+                    if (override.indexOf('=') == -1) {
+                        throw new IllegalArgumentException(override);
+                    }
+                }
+            }
             this.envs = overrides;
             return this;
         }
 
         public String[] envs() {
-            return envs;
+            return envs.clone();
         }
 
         /**
@@ -708,18 +715,11 @@ public final Launcher decorateByEnv(EnvVars _env) {
             public Proc launch(ProcStarter starter) throws IOException {
                 EnvVars e = new EnvVars(env);
                 if (starter.envs!=null) {
-                    for (int i=0; i<starter.envs.length; i+=2)
-                        e.put(starter.envs[i],starter.envs[i+1]);
-                }
-
-                String[] r = new String[e.size()*2];
-                int idx=0;
-                for (Entry<String, String> i : e.entrySet()) {
-                    r[idx++] = i.getKey();
-                    r[idx++] = i.getValue();
+                    for (String env : starter.envs) {
+                        e.addLine(env);
+                    }
                 }
-
-                starter.envs = r;
+                starter.envs = Util.mapToEnv(e);
                 return outer.launch(starter);
             }
 

diff --git a/core/src/test/java/hudson/LauncherTest.java b/core/src/test/java/hudson/LauncherTest.java
@@ -24,9 +24,12 @@
 
 package hudson;
 
+import hudson.model.StreamBuildListener;
+import hudson.model.TaskListener;
 import hudson.util.ProcessTree;
 import hudson.util.StreamTaskListener;
 import hudson.remoting.Callable;
+import java.io.ByteArrayOutputStream;
 import org.apache.commons.io.FileUtils;
 import org.jvnet.hudson.test.Bug;
 
@@ -75,4 +78,18 @@ public Object call() throws RuntimeException {
             return null;
         }
     };
+
+    @Bug(15733)
+    public void testDecorateByEnv() throws Exception {
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        TaskListener l = new StreamBuildListener(baos);
+        Launcher base = new Launcher.LocalLauncher(l);
+        EnvVars env = new EnvVars("key1", "val1");
+        Launcher decorated = base.decorateByEnv(env);
+        int res = decorated.launch().envs("key2=val2").cmds(Functions.isWindows() ? new String[] {"cmd", "/q", "/c", "echo %key1% %key2%"} : new String[] {"sh", "-c", "echo $key1 $key2"}).stdout(l).join();
+        String log = baos.toString();
+        assertEquals(log, 0, res);
+        assertTrue(log, log.contains("val1 val2"));
+    }
+
 }