openssh unix dev - Oct 2003 - Problem in sftp 'ls' command output

Hello,

I have recently moved from OpenSSH 3.6 to 3.7 (portable version) :

[pyb at nihao tmp]$ ssh -v
OpenSSH_3.7p1, SSH protocols 1.5/2.0, OpenSSL 0.9.6h  5 Dec 2002

(okay, the SSL release is a little old but that's not the subject).
I am also using scripts to perform automated sftp transfers between some 
systems. The 3.7 release of OpenSSH broke my scripts, which made me sad 
and my customers angry :-(
The main problem I met is in processing 'ls' output. The format changed 
dramatically WRT my scripts' expectations. Where I got ls-like output 
previously, now I get :

sftp> ls send
send/.          send/.. 
send/000000676.lotsend/000000677.lotsend/000000678.lot
send/000000679.lotsend/000000680.lotsend/000000681.lotsend/000000682.lotsend/endfile


[my shitty wrapping email client could make the problem not obvious. 
Here it is : the file names are concatenated, without spaces in-between]

Known problem ? If so, my apologies. If not, I did a little homework and 
found that the sftp-int.c/do_ls_dir function seems to be the culprit. We 
have first :

--------
if (!(lflag & SHORT_VIEW)) {
     int m = 0, width = 80;
     struct winsize ws;

     /* Count entries for sort and find longest filename */
     for (n = 0; d[n] != NULL; n++)
         m = MAX(m, strlen(d[n]->filename));

     if (ioctl(fileno(stdin), TIOCGWINSZ, &ws) != -1)
         width = ws.ws_col;

     columns = width / (m + 2);
     columns = MAX(columns, 1);
     colspace = width / columns;
}
--------

which aims to compute the column width and column spacing.
But when printing, a few lines below, we perform :

--------
tmp = path_append(path, d[n]->filename);
fname = path_strip(tmp, strip_path);
xfree(tmp);

if (lflag & LONG_VIEW) {
/* irrelevant code */
} else {
     printf("%-*s", colspace, fname);
     if (c >= columns) {
	printf("\n");
	c = 1;
    } else
	c++;
}
--------

What it boils down is that column width and spacing are computed on 
filenames, but we print path + filename. So column width is wrong on 
almost all cases.

Here is a short patch. Warning : I assumed path and strip_path are never 
NULL and that strip_path is never "bigger" thant path. So I did NO 
control. This may be a little rough... I bet there are some better ways 
out, if this is really an error and not a misunderstanding on my part.

Hth,
-- 
Pierre-Yves Bonnetain
B&A Consultants - Networks and Computers Security
Phone : +33 (0) 563 277 241 - Fax : +33 (0) 563 277 245
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: openssh-diff
Url:
http://lists.mindrot.org/pipermail/openssh-unix-dev/attachments/20031013/bb412f6a/attachment.ksh

On Mon, 13 Oct 2003, Pierre-Yves Bonnetain wrote:
> Hello,
>
> I have recently moved from OpenSSH 3.6 to 3.7 (portable version) :
>
> [pyb at nihao tmp]$ ssh -v
> OpenSSH_3.7p1, SSH protocols 1.5/2.0, OpenSSL 0.9.6h  5 Dec 2002
>
> (okay, the SSL release is a little old but that's not the subject).
> I am also using scripts to perform automated sftp transfers between some
> systems. The 3.7 release of OpenSSH broke my scripts, which made me sad
> and my customers angry :-(
> The main problem I met is in processing 'ls' output. The format
changed
> dramatically WRT my scripts' expectations. Where I got ls-like output
> previously, now I get :
>
> sftp> ls send
> send/.          send/..
> send/000000676.lotsend/000000677.lotsend/000000678.lot
>
send/000000679.lotsend/000000680.lotsend/000000681.lotsend/000000682.lotsend/endfile
>
Your patch didn't go through, but does this one work?

Index: sftp-int.c
==================================================================RCS file:
/cvs/src/usr.bin/ssh/sftp-int.c,v
retrieving revision 1.63
diff -u -r1.63 sftp-int.c
--- sftp-int.c	7 Oct 2003 07:04:16 -0000	1.63
+++ sftp-int.c	14 Oct 2003 01:39:38 -0000
@@ -594,17 +594,24 @@
 	if (!(lflag & SHORT_VIEW)) {
 		int m = 0, width = 80;
 		struct winsize ws;
+		char *tmp;

 		/* Count entries for sort and find longest filename */
 		for (n = 0; d[n] != NULL; n++)
 			m = MAX(m, strlen(d[n]->filename));

+		/* Add any subpath that also needs to be counted */
+		tmp = path_strip(path, strip_path);
+		m += strlen(tmp);
+		xfree(tmp);
+
 		if (ioctl(fileno(stdin), TIOCGWINSZ, &ws) != -1)
 			width = ws.ws_col;

 		columns = width / (m + 2);
 		columns = MAX(columns, 1);
 		colspace = width / columns;
+		colspace = MIN(colspace, width);
 	}

 	qsort(d, n, sizeof(*d), sdirent_comp);