[FIXED JENKINS-7478]

Create Line Ending Conversion utility to convert scripts to proper OS line ending type. Tests included.
jenkinsci · Aug 15, 2014 · 67ac47af0709dde6297dffeb210b03d8b65f6033 · 67ac47a
1 parent 3efe07f
commit 67ac47af0709dde6297dffeb210b03d8b65f6033
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 @@ private static String addCrForNonASCII(String s) {
    }
    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 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 @@
    @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 getCommandFileExtension() {
        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 @@
    @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 getCommandFileExtension() {
        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");
+  
+    }
+  
+}