Bug 452

Summary: sftp does not abort when commands given via -b fail
Product: Portable OpenSSH Reporter: Aran Cox <spin>
Component: sftpAssignee: OpenSSH Bugzilla mailing list <openssh-bugs>
Status: CLOSED FIXED    
Severity: normal    
Priority: P2    
Version: -current   
Hardware: ix86   
OS: Linux   

Description Aran Cox 2002-12-06 03:14:19 AEDT 
The man page states that sftp -b will abort when commands (including get) fail.
 However, when I give sftp a list of get -P commands via the -b option it
continues unconcerned when the requested file doesn't exist and it doesn't exit
with a error code either (which would be enough for me.)

# /usr/bin/sftp -b /tmp/files roger@remote && echo ok
Connecting to remote...
sftp> GET -P /data/200211/njxj1-cdr_2002_11_30__11_00_00.cdr
Couldn't stat remote file: No such file or directory
File "/data/200211/njxj1-cdr_2002_11_30__11_00_00.cdr" not found.
sftp> GET -P /data/200211/cwxj1-cdr_2002_11_26__05_00_00.cdr.bz2
Fetching /data/200211/cwxj1-cdr_2002_11_26__05_00_00.cdr.bz2 to
cwxj1-cdr_2002_11_26__05_00_00.cdr.bz2
sftp> GET -P /data/200211/cwxj2-cdr_2002_11_26__05_00_00.cdr.bz2
Fetching /data/200211/cwxj2-cdr_2002_11_26__05_00_00.cdr.bz2 to
cwxj2-cdr_2002_11_26__05_00_00.cdr.bz2
sftp> 
ok
Comment 1 Aran Cox 2002-12-06 03:15:17 AEDT 
Forgot to mention that this is 3.1p1-6 RedHat RPM.
Comment 2 Colin Watson 2002-12-18 04:39:21 AEDT 
This is also Debian bug #173456 (http://bugs.debian.org/173456).
Comment 3 Colin Watson 2002-12-30 11:18:36 AEDT 
Robert Bihlmeyer <robbe@orcus.priv.at> added these comments and suggested the
following patch:

The following "-b" script should work if everything ran as documented:

  get the-remote-file
  rm the-remote-file

In the status quo it can incur dataloss (and did for me, aargh!) if
the get fails for some reason (e.g. disk full).

The appended patch fixes the some cases, and should point people in
the right direction. There are still more places where process_get
will return -1 (which will not lead to abort). Either all these get
changed to 1, or -- as it seems the intention is not to abort on every
error -- the manpage documents exactly in which cases an abort will
happen. The current situation is just bad.

I'd be most satisfied with a "abort on every miniscule error" switch,
akin to sh's -e.

diff -u openssh-3.5p1/sftp-int.c~ openssh-3.5p1/sftp-int.c
--- openssh-3.5p1/sftp-int.c~   2002-12-30 01:13:29.000000000 +0100
+++ openssh-3.5p1/sftp-int.c    2002-12-30 01:13:29.000000000 +0100
@@ -426,7 +426,7 @@
                        goto out;
                }
                printf("Fetching %s to %s\n", g.gl_pathv[0], abs_dst);
-               err = do_download(conn, g.gl_pathv[0], abs_dst, pflag);
+               err = do_download(conn, g.gl_pathv[0], abs_dst, pflag) ? 1 : 0;
                goto out;
        }

@@ -451,7 +451,7 @@

                printf("Fetching %s to %s\n", g.gl_pathv[i], abs_dst);
                if (do_download(conn, g.gl_pathv[i], abs_dst, pflag) == -1)
-                       err = -1;
+                       err = 1;
                xfree(abs_dst);
                abs_dst = NULL;
        }
Comment 4 Damien Miller 2003-01-09 10:51:30 AEDT 
Fixed in CVS. Any error in any command will now be considered a fatal error,
unless you prefix the command with a '-' sign.

E.g.

put /tmp/existing 
-put /tmp/nonexisting
put /tmp/existing2

will run to completion
Comment 5 Damien Miller 2004-04-14 12:24:18 AEST 
Mass change of RESOLVED bugs to CLOSED