[FIXED JENKINS-7478]

Create Line Ending Conversion utility to convert scripts to proper OS line ending type. Tests included. (cherry picked from commit 67ac47a)
jenkinsci · Oct 12, 2014 · efae48a · efae48a
1 parent 4d455fb
commit efae48a
Show file tree

Hide file tree

Showing 11 changed files with 233 additions and 53 deletions.
diff --git a/core/src/main/java/hudson/tasks/BatchFile.java b/core/src/main/java/hudson/tasks/BatchFile.java
@@ -26,9 +26,11 @@
 import hudson.FilePath;
 import hudson.Extension;
 import hudson.model.AbstractProject;
+import hudson.util.LineEndingConversion;
 import net.sf.json.JSONObject;
 import org.kohsuke.stapler.DataBoundConstructor;
 import org.kohsuke.stapler.StaplerRequest;
+import java.io.ObjectStreamException;
 
 /**
  * Executes commands by using Windows batch file.
@@ -38,21 +40,25 @@
 public class BatchFile extends CommandInterpreter {
     @DataBoundConstructor
     public BatchFile(String command) {
-        super(command);
+        super(LineEndingConversion.convertEOL(command, LineEndingConversion.EOLType.Windows));
     }
 
     public String[] buildCommandLine(FilePath script) {
         return new String[] {"cmd","/c","call",script.getRemote()};
     }
 
     protected String getContents() {
-        return command+"\r\nexit %ERRORLEVEL%";
+        return LineEndingConversion.convertEOL(command+"\r\nexit %ERRORLEVEL%",LineEndingConversion.EOLType.Windows);
     }
 
     protected String getFileExtension() {
         return ".bat";
     }
 
+    private Object readResolve() throws ObjectStreamException {
+        return new BatchFile(command);
+    }
+
     @Extension
     public static final class DescriptorImpl extends BuildStepDescriptor<Builder> {
         @Override

diff --git a/core/src/main/java/hudson/tasks/Shell.java b/core/src/main/java/hudson/tasks/Shell.java
@@ -32,6 +32,8 @@
 import hudson.remoting.VirtualChannel;
 import hudson.util.FormValidation;
 import java.io.IOException;
+import java.io.ObjectStreamException;
+import hudson.util.LineEndingConversion;
 import net.sf.json.JSONObject;
 import org.kohsuke.stapler.DataBoundConstructor;
 import org.kohsuke.stapler.StaplerRequest;
@@ -50,37 +52,15 @@
 public class Shell extends CommandInterpreter {
     @DataBoundConstructor
     public Shell(String command) {
-        super(fixCrLf(command));
-    }
-
-    /**
-     * Fix CR/LF and always make it Unix style.
-     */
-    private static String fixCrLf(String s) {
-        // eliminate CR
-        int idx;
-        while((idx=s.indexOf("\r\n"))!=-1)
-            s = s.substring(0,idx)+s.substring(idx+1);
-
-        //// add CR back if this is for Windows
-        //if(isWindows()) {
-        //    idx=0;
-        //    while(true) {
-        //        idx = s.indexOf('\n',idx);
-        //        if(idx==-1) break;
-        //        s = s.substring(0,idx)+'\r'+s.substring(idx);
-        //        idx+=2;
-        //    }
-        //}
-        return s;
+        super(LineEndingConversion.convertEOL(command, LineEndingConversion.EOLType.Unix));
     }
 
     /**
      * Older versions of bash have a bug where non-ASCII on the first line
      * makes the shell think the file is a binary file and not a script. Adding
      * a leading line feed works around this problem.
      */
-    private static String addCrForNonASCII(String s) {
+    private static String addLineFeedForNonASCII(String s) {
         if(!s.startsWith("#!")) {
             if (s.indexOf('\n')!=0) {
                 return "\n" + s;
@@ -105,7 +85,7 @@ public String[] buildCommandLine(FilePath script) {
     }
 
     protected String getContents() {
-        return addCrForNonASCII(fixCrLf(command));
+        return addLineFeedForNonASCII(LineEndingConversion.convertEOL(command,LineEndingConversion.EOLType.Unix));
     }
 
     protected String getFileExtension() {
@@ -117,6 +97,10 @@ public DescriptorImpl getDescriptor() {
         return (DescriptorImpl)super.getDescriptor();
     }
 
+    private Object readResolve() throws ObjectStreamException {
+        return new Shell(command);
+    }
+
     @Extension
     public static class DescriptorImpl extends BuildStepDescriptor<Builder> {
         /**

diff --git a/core/src/main/java/hudson/tools/AbstractCommandInstaller.java b/core/src/main/java/hudson/tools/AbstractCommandInstaller.java
@@ -48,7 +48,7 @@ public abstract class AbstractCommandInstaller extends ToolInstaller {
 
     public AbstractCommandInstaller(String label, String command, String toolHome) {
         super(label);
-        this.command = fixCrLf(command);
+        this.command = command;
         this.toolHome = toolHome;
     }
 
@@ -84,19 +84,6 @@ public FilePath performInstallation(ToolInstallation tool, Node node, TaskListen
         return dir.child(getToolHome());
     }
 
-    /**
-     * Fix CR/LF and always make it Unix style.
-     */
-    //TODO: replace by a Windows style
-    private static String fixCrLf(String s) {
-        // eliminate CR
-        int idx;
-        while ((idx = s.indexOf("\r\n")) != -1) {
-            s = s.substring(0, idx) + s.substring(idx + 1);
-        }
-        return s;
-    }
-
     public static abstract class Descriptor<TInstallerClass extends AbstractCommandInstaller>
             extends ToolInstallerDescriptor<TInstallerClass> {
 

diff --git a/core/src/main/java/hudson/tools/BatchCommandInstaller.java b/core/src/main/java/hudson/tools/BatchCommandInstaller.java
@@ -26,7 +26,9 @@
 
 import hudson.Extension;
 import hudson.FilePath;
+import hudson.util.LineEndingConversion;
 import org.kohsuke.stapler.DataBoundConstructor;
+import java.io.ObjectStreamException;
 
 /**
  * Installs tool via script execution of Batch script.
@@ -37,7 +39,7 @@ public class BatchCommandInstaller extends AbstractCommandInstaller {
 
     @DataBoundConstructor
     public BatchCommandInstaller(String label, String command, String toolHome) {
-        super(label, command, toolHome);
+        super(label, LineEndingConversion.convertEOL(command, LineEndingConversion.EOLType.Windows), toolHome);
     }
 
     @Override
@@ -50,6 +52,10 @@ public String[] getCommandCall(FilePath script) {
         String[] cmd = {"cmd", "/c", "call", script.getRemote()};
         return cmd;
     }
+
+    private Object readResolve() throws ObjectStreamException {
+        return new BatchCommandInstaller(getLabel(), getCommand(), getToolHome());
+    }
 
     @Extension
     public static class DescriptorImpl extends Descriptor<BatchCommandInstaller> {

diff --git a/core/src/main/java/hudson/tools/CommandInstaller.java b/core/src/main/java/hudson/tools/CommandInstaller.java
@@ -26,7 +26,9 @@
 
 import hudson.Extension;
 import hudson.FilePath;
+import hudson.util.LineEndingConversion;
 import org.kohsuke.stapler.DataBoundConstructor;
+import java.io.ObjectStreamException;
 
 /**
  * Installs a tool by running an arbitrary shell command.
@@ -36,7 +38,7 @@ public class CommandInstaller extends AbstractCommandInstaller {
 
     @DataBoundConstructor
     public CommandInstaller(String label, String command, String toolHome) {
-        super(label, command, toolHome);
+        super(label, LineEndingConversion.convertEOL(command, LineEndingConversion.EOLType.Unix), toolHome);
     }
 
     @Override
@@ -50,6 +52,10 @@ public String[] getCommandCall(FilePath script) {
         return cmd;
     }
 
+    private Object readResolve() throws ObjectStreamException {
+        return new CommandInstaller(getLabel(), getCommand(), getToolHome());
+    }
+
     @Extension
     public static class DescriptorImpl extends Descriptor<CommandInstaller> {
 

diff --git a/core/src/main/java/hudson/util/LineEndingConversion.java b/core/src/main/java/hudson/util/LineEndingConversion.java
@@ -0,0 +1,66 @@
+package hudson.util;
+
+/**
+ * Converts line endings of a string.
+ *
+ * @since TODO
+ * @author David Ruhmann
+ */
+public class LineEndingConversion {
+
+    /**
+     * Supported line ending types for conversion
+     */
+    public enum EOLType {
+        CR,
+        CRLF,
+        LF,
+        LFCR,
+        Mac,
+        Unix,
+        Windows
+    }
+
+    /**
+     * Convert line endings of a string to the given type.  Default to Unix type.
+     *
+     * @param input
+     *     The string containing line endings to be converted.
+     * @param type
+     *     Type of line endings to convert the string into.
+     * @return
+     *     String updated with the new line endings or null if given null.
+     */
+    public static String convertEOL(String input, EOLType type) {
+        if (null == input || 0 == input.length()) {
+            return input;
+        }
+        // Convert line endings to Unix LF,
+        // which also sets up the string for other conversions
+        input = input.replace("\r\n","\n");
+        input = input.replace("\r","\n");
+        switch (type) {
+            case CR:
+            case Mac:
+                // Convert line endings to CR
+                input = input.replace("\n", "\r");
+                break;
+            case CRLF:
+            case Windows:
+                // Convert line endings to Windows CR/LF
+                input = input.replace("\n", "\r\n");
+                break;
+            default:
+            case LF:
+            case Unix:
+                // Conversion already completed
+                return input;
+            case LFCR:
+                // Convert line endings to LF/CR
+                input = input.replace("\n", "\n\r");
+                break;
+        }
+        return input;
+    }
+}
+
diff --git a/test/src/test/java/hudson/tasks/BatchFileTest.java b/test/src/test/java/hudson/tasks/BatchFileTest.java
@@ -0,0 +1,30 @@
+package hudson.tasks;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.Issue;
+import org.jvnet.hudson.test.JenkinsRule;
+
+/**
+ * Tests for the BatchFile tasks class.
+ *
+ * @author David Ruhmann
+ */
+public class BatchFileTest {
+
+    @Rule
+    public JenkinsRule rule = new JenkinsRule();
+
+    @Issue("JENKINS-7478")
+    @Test
+    public void validateBatchFileCommandEOL() throws Exception {
+        BatchFile obj = new BatchFile("echo A\necho B\recho C");
+        rule.assertStringContains(obj.getCommand(), "echo A\r\necho B\r\necho C");
+    }
+
+    @Test
+    public void validateBatchFileContents() throws Exception {
+        BatchFile obj = new BatchFile("echo A\necho B\recho C");
+        rule.assertStringContains(obj.getContents(), "echo A\r\necho B\r\necho C\r\nexit %ERRORLEVEL%");
+    }
+}
diff --git a/test/src/test/java/hudson/tasks/ShellTest.java b/test/src/test/java/hudson/tasks/ShellTest.java
@@ -7,31 +7,54 @@
 import hudson.model.FreeStyleProject;
 import org.apache.commons.io.FileUtils;
 import org.jvnet.hudson.test.FakeLauncher;
-import org.jvnet.hudson.test.HudsonTestCase;
 import org.jvnet.hudson.test.PretendSlave;
 
 import java.io.File;
 import java.io.IOException;
 import java.io.PrintStream;
 import java.util.List;
 
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.JenkinsRule;
+
 /**
+ * Tests for the Shell tasks class
+ *
  * @author Kohsuke Kawaguchi
  */
-public class ShellTest extends HudsonTestCase {
+public class ShellTest {
+
+    @Rule
+    public JenkinsRule rule = new JenkinsRule();
+
+    @Test
+    public void validateShellCommandEOL() throws Exception {
+        Shell obj = new Shell("echo A\r\necho B\recho C");
+        rule.assertStringContains(obj.getCommand(), "echo A\necho B\necho C");
+    }
+
+    @Test
+    public void validateShellContents() throws Exception {
+        Shell obj = new Shell("echo A\r\necho B\recho C");
+        rule.assertStringContains(obj.getContents(), "\necho A\necho B\necho C");
+    }
+
+    @Test
     public void testBasic() throws Exception {
         // If we're on Windows, don't bother doing this.
         if (Functions.isWindows())
             return;
 
         // TODO: define a FakeLauncher implementation with easymock so that this kind of assertions can be simplified.
-        PretendSlave s = createPretendSlave(new FakeLauncher() {
+        PretendSlave s = rule.createPretendSlave(new FakeLauncher() {
             public Proc onLaunch(ProcStarter p) throws IOException {
                 // test the command line argument.
                 List<String> cmds = p.cmds();
-                assertEquals("/bin/sh",cmds.get(0));
-                assertEquals("-xe",cmds.get(1));
-                assertTrue(new File(cmds.get(2)).exists());
+                rule.assertStringContains("/bin/sh",cmds.get(0));
+                rule.assertStringContains("-xe",cmds.get(1));
+                Assert.assertTrue(new File(cmds.get(2)).exists());
 
                 // fake the execution
                 PrintStream ps = new PrintStream(p.stdout());
@@ -41,13 +64,13 @@ public Proc onLaunch(ProcStarter p) throws IOException {
                 return new FinishedProc(0);
             }
         });
-        FreeStyleProject p = createFreeStyleProject();
+        FreeStyleProject p = rule.createFreeStyleProject();
         p.getBuildersList().add(new Shell("echo abc"));
         p.setAssignedNode(s);
 
-        FreeStyleBuild b = assertBuildStatusSuccess(p.scheduleBuild2(0).get());
+        FreeStyleBuild b = rule.assertBuildStatusSuccess(p.scheduleBuild2(0).get());
 
-        assertEquals(1,s.numLaunch);
-        assertTrue(FileUtils.readFileToString(b.getLogFile()).contains("Hudson was here"));
+        Assert.assertEquals(1,s.numLaunch);
+        Assert.assertTrue(FileUtils.readFileToString(b.getLogFile()).contains("Hudson was here"));
     }
 }
diff --git a/test/src/test/java/hudson/tools/BatchCommandInstallerTest.java b/test/src/test/java/hudson/tools/BatchCommandInstallerTest.java
@@ -0,0 +1,22 @@
+package hudson.tools;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.JenkinsRule;
+
+/**
+ * Tests for the BatchCommandInstaller tools class.
+ *
+ * @author David Ruhmann
+ */
+public class BatchCommandInstallerTest {
+
+    @Rule
+    public JenkinsRule rule = new JenkinsRule();
+
+    @Test
+    public void validateBatchCommandInstallerCommandEOL() throws Exception {
+        BatchCommandInstaller obj = new BatchCommandInstaller("", "echo A\necho B\recho C", "");
+        rule.assertStringContains(obj.getCommand(), "echo A\r\necho B\r\necho C");
+    }
+}