Bug 54408 - SSHExec task reverses input and output stream from JSCH
Summary: SSHExec task reverses input and output stream from JSCH
Status: NEEDINFO
Alias: None
Product: Ant
Classification: Unclassified
Component: Optional Tasks (show other bugs)
Version: 1.9.1
Hardware: All All
: P2 normal (vote)
Target Milestone: ---
Assignee: Ant Notifications List
URL:
Keywords: PatchAvailable
Depends on:
Blocks:
 
Reported: 2013-01-11 21:29 UTC by Ben Wing
Modified: 2014-01-08 14:48 UTC (History)
1 user (show)



Attachments
build file and accompanying documents exercising 3 examples of SSHExec task (5.35 KB, application/zip)
2013-01-11 21:29 UTC, Ben Wing
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ben Wing 2013-01-11 21:29:15 UTC
Created attachment 29848 [details]
build file and accompanying documents exercising 3 examples of SSHExec task

When using the SSHExec task to try to pass a password to the sudo command using inputproperty attribute my password was being printed to the tasks logging, and my sudo command would never complete.
I found that the InputStream and OutputStream of the JSch ChannelExec object where configured reverse to how they should be according to the JSch Sudo Example at http://www.jcraft.com/jsch/examples/Sudo.java

I developed the following patch to address the issue, and have attached a test case using the 3 input styles (inputproperty, inputstring, and input attributes).

$ svn diff SSHExec.java
Index: SSHExec.java
===================================================================
--- SSHExec.java        (revision 1431539)
+++ SSHExec.java        (working copy)
@@ -28,6 +28,8 @@
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.StringReader;
+import java.io.OutputStream;
+import java.util.ArrayList;

 import org.apache.tools.ant.BuildException;
 import org.apache.tools.ant.Project;
@@ -63,7 +65,7 @@
     private File outputFile = null;   // like <exec>
     private String inputProperty = null;
     private String inputString = null;   // like <exec>
-    private File inputFile = null;   // like <exec>
+    private Resource inputFile = null;
     private boolean append = false;   // like <exec>
     private boolean usePty = false;

@@ -125,7 +127,7 @@
      * @since Ant 1.8.0
      */
     public void setInput(File input) {
-        inputFile = input;
+        inputFile = new FileResource(input);
     }

     /**
@@ -181,11 +183,11 @@
     }

     /**
-     * Execute the command on the remote host.
+     * Has the user set all necessary attributes?
      *
-     * @exception BuildException  Most likely a network error or bad parameter.
+     * @exception BuildException if there are missing required parameters.
      */
-    public void execute() throws BuildException {
+    private void checkConfiguration() throws BuildException {
         if (getHost() == null) {
             throw new BuildException("Host is required.");
         }
@@ -208,40 +210,37 @@
                                      + " inputFile, inputProperty and"
                                      + " inputString.");
         }
-        if (inputFile != null && !inputFile.exists()) {
+        if (inputFile != null && !inputFile.isExists()) {
             throw new BuildException("The input file "
-                                     + inputFile.getAbsolutePath()
+                                     + ((FileResource)inputFile).getFile().getAbsolutePath()
                                      + " does not exist.");
         }

+    }
+
+    /**
+     * Execute the command on the remote host.
+     *
+     * @exception BuildException Most likely a network error or bad parameter.
+     */
+    public void execute() throws BuildException {
+        checkConfiguration();
+        ArrayList inputs = getInputs();
+        ArrayList cmds = getCmds();
+        checkInputCmdCounts(inputs, cmds);
         Session session = null;
         StringBuffer output = new StringBuffer();
         try {
-            session = openSession();
-            /* called once */
-            if (command != null) {
-                log("cmd : " + command, Project.MSG_INFO);
-                executeCommand(session, command, output);
-            } else { // read command resource and execute for each command
-                try {
-                    BufferedReader br = new BufferedReader(
-                            new InputStreamReader(commandResource.getInputStream()));
-                    String cmd;
-                    while ((cmd = br.readLine()) != null) {
-                        log("cmd : " + cmd, Project.MSG_INFO);
-                        output.append(cmd).append(" : ");
-                        executeCommand(session, cmd, output);
-                        output.append("\n");
-                    }
-                    FileUtils.close(br);
-                } catch (IOException e) {
-                    if (getFailonerror()) {
-                        throw new BuildException(e);
-                    } else {
-                        log("Caught exception: " + e.getMessage(),
-                            Project.MSG_ERR);
-                    }
-                }
+            session = openSession();  /* called once */
+            for(int i = 0; i < cmds.size(); i++) {
+                String cmd = (String)cmds.get(i);
+                String input = null;
+                if (inputs.size() > 0 && i < inputs.size())
+                    input=(String)inputs.get(i);
+                log("cmd : " + cmd, Project.MSG_INFO);
+                output.append(cmd).append(" : ");
+                executeCommand(session, cmd, input, output);
+                output.append("\n");
             }
         } catch (JSchException e) {
             if (getFailonerror()) {
@@ -259,17 +258,35 @@
         }
     }

-    private void executeCommand(Session session, String cmd, StringBuffer sb)
-        throws BuildException {
-        ByteArrayOutputStream out = new ByteArrayOutputStream();
-        TeeOutputStream tee =
-            new TeeOutputStream(out,
-                                KeepAliveOutputStream.wrapSystemOut());
+    private void checkInputCmdCounts(ArrayList inputs, ArrayList cmds) {
+        int cmdsCnt = cmds.size();
+        if (cmdsCnt == 0) {
+            throw new BuildException("There are no commands to execute in "
+                                     + ((FileResource)commandResource).getFile().getAbsolutePath());
+        }
+        int diff = cmdsCnt-inputs.size();
+        if (diff > 0 && cmdsCnt > 1) {
+            log("There are fewer inputs than commands provided so the last "+diff
+               +" commands won't be provided any input", Project.MSG_WARN);
+        }
+        if (diff < 0) {
+            log("There are more inputs than commands provided so the last "+Math.abs(diff)
+                    +" inputs won't used", Project.MSG_WARN);

-        InputStream istream = null ;
+        }
+    }
+
+    private ArrayList getInputs() {
+        ArrayList inputData = new ArrayList();
         if (inputFile != null) {
             try {
-                istream = new FileInputStream(inputFile) ;
+                String line;
+                BufferedReader lines = new BufferedReader(
+                        new InputStreamReader(inputFile.getInputStream()));
+                while ((line = lines.readLine()) != null) {
+                    inputData.add(line+"\n");
+                }
+                FileUtils.close(lines);
             } catch (IOException e) {
                 // because we checked the existence before, this one
                 // shouldn't happen What if the file exists, but there
@@ -278,37 +295,111 @@
                     + e.getMessage(), Project.MSG_WARN);
             }
         }
-        if (inputProperty != null) {
-            String inputData = getProject().getProperty(inputProperty) ;
-            if (inputData != null) {
-                istream = new ByteArrayInputStream(inputData.getBytes()) ;
-            }
+        else if (inputProperty != null) {
+            String propertyValue = getProject().getProperty(inputProperty);
+            if (propertyValue != null)
+                inputData.add(propertyValue+"\n");
         }
-        if (inputString != null) {
-            istream = new ByteArrayInputStream(inputString.getBytes());
+        else if (inputString != null) {
+            inputData.add(inputString+"\n");
         }
+        return inputData;
+    }

+    private ArrayList getCmds() {
+        ArrayList cmds = new ArrayList();
+        if (commandResource != null) {
+            try {
+                BufferedReader br = new BufferedReader(
+                        new InputStreamReader(commandResource.getInputStream()));
+                String cmd;
+                while ((cmd = br.readLine()) != null) {
+                    cmds.add(cmd);
+                }
+                FileUtils.close(br);
+            } catch (IOException e) {
+                if (getFailonerror()) {
+                    throw new BuildException(e);
+                } else {
+                    log("Caught exception: " + e.getMessage(),
+                        Project.MSG_ERR);
+                }
+            }
+        }
+        else if (command != null) {
+            cmds.add(command);
+        }
+        return cmds;
+    }
+
+    private InputStream inputFromCmd = null;
+    private OutputStream outputToCmd = null;
+    private TeeOutputStream teeToSysOut = null;
+
+    private void executeCommand(Session session, String cmd, String input, StringBuffer sb)
+            throws BuildException {
+        ByteArrayOutputStream out = new ByteArrayOutputStream();
+        teeToSysOut = new TeeOutputStream(out,
+                KeepAliveOutputStream.wrapSystemOut());
+        ByteArrayOutputStream err = new ByteArrayOutputStream();
+        TeeOutputStream teeToSysErr = new TeeOutputStream(err,
+                KeepAliveOutputStream.wrapSystemErr());
+
         try {
             final ChannelExec channel;
             session.setTimeout((int) maxwait);
             /* execute the command */
             channel = (ChannelExec) session.openChannel("exec");
             channel.setCommand(cmd);
-            channel.setOutputStream(tee);
-            channel.setExtOutputStream(tee);
-            if (istream != null) {
-                channel.setInputStream(istream);
-            }
             channel.setPty(usePty);
+
+            if (input != null) {
+                outputToCmd = channel.getOutputStream();
+            } else
+                channel.setOutputStream(null);
+
+            inputFromCmd = channel.getInputStream();
+            channel.setErrStream(teeToSysErr);
             channel.connect();
+            log("successfully connected");
+            if (input != null) {
+                outputToCmd.write(input.getBytes());
+                outputToCmd.flush();
+            }
+
             // wait for it to finish
             thread =
                 new Thread() {
                     public void run() {
-                        while (!channel.isClosed()) {
+                        byte[] tmp = new byte[1024];
+                        while (true) {
+                            log("attempting to read input from SSHExec command", Project.MSG_DEBUG);
+                            try {
+                                while (inputFromCmd.available() > 0) {
+                                    int read = inputFromCmd.read(tmp);
+                                    if (read < 0)
+                                        break;
+                                    // sb.append((char[]) tmp, 0, read);
+                                    teeToSysOut.write(tmp, 0, read);
+                                }
+                            } catch (IOException ioe) {
+                                handleErrorFlush("Error handling output from cmd:"
+                                                 + ioe.getMessage());
+                            }
                             if (thread == null) {
                                 return;
                             }
+                            if (channel.isClosed()) {
+                                String str = "cmd exited with status: "
+                                            + channel.getExitStatus();
+                                try {
+                                    teeToSysOut.write(str.getBytes());
+                                } catch (IOException ioe) {
+                                    handleErrorFlush("Error handling channel close message:"
+                                                     + ioe.getMessage());
+                                }
+                                return;
+                            }
                             try {
                                 sleep(RETRY_INTERVAL);
                             } catch (Exception e) {
@@ -372,7 +463,6 @@
             }
         } finally {
             sb.append(out.toString());
-            FileUtils.close(istream);
         }
     }

@@ -407,4 +497,4 @@
             }
         }
     }
-}
\ No newline at end of file
+}
Comment 1 Antoine Levy-Lambert 2013-02-05 04:43:02 UTC
Ben, I am not able to reproduce your problem using Java 1.7 or Java 1.6 and jsch 0.1.49 and the current code of Ant. I have targetted my own Mac and an ubuntu VM, running the build file on Mac if this matters.
Comment 2 Atsuhiko Yamanaka 2014-01-08 13:26:45 UTC
I'm the author of JSch, and have also tried the attached build.xml with Apache Ant 1.9.3, but I can't find the problem for the test case of sudo.  I think the status should be changed to "NEEDINFO".
Comment 3 Stefan Bodewig 2014-01-08 14:48:23 UTC
Thanks for checking!