[ftpfs] [regression] mc adds spaces at the beginning of some files/dirs on AIX ftp servers

[leadman]

Hi everyone,
as I was asked in ticket #2635 , I have compiled latest 'mc' from git. The problem with leading spaces in ftp connection still occurs. This bug occurs when connecting to FTP servers on AIX 5.3.0.0 and 6.1.0.0. The only difference with previous version of 'mc' is that now it shows spaces before some file names instantly after establishing an FTP connection(no part with "?" mark). I can provide gdb stack trace if this is necessary. I suppose correct component would be mc-vfs? As this bug makes 'mc' unusable with AIX ftp server I suggest to change it's priority to critical.

Below you will find compiled version:

patben@esp-patben-lin:~$ LC_MESSAGES=C /usr/local/bin/mc -V
GNU Midnight Commander 4.8.3-114-ga343070
Built with GLib 2.32.3
Using the S-Lang library with terminfo database
With builtin Editor
With subshell support as default
With support for background operations
With mouse support on xterm and Linux console
With support for X11 events
With internationalization support
With multiple codepages support
Virtual File Systems: cpiofs, tarfs, sfs, extfs, ftpfs, fish
Data types: char: 8; int: 32; long: 64; void *: 64; size_t: 64; off_t: 64;
patben@esp-patben-lin:~$ LC_MESSAGES=C /usr/local/bin/mc -F
Root directory: /home/patben

[System data]
    Config directory: /usr/local/etc/mc/
    Data directory:   /usr/local/share/mc/
    VFS plugins and scripts: /usr/local/libexec/mc/
	extfs.d:        /usr/local/libexec/mc/extfs.d/
	fish:           /usr/local/libexec/mc/fish/

[User data]
    Config directory: /home/patben/.config/mc/
    Data directory:   /home/patben/.local/share/mc/
	skins:          /home/patben/.local/share/mc/skins/
	extfs.d:        /home/patben/.local/share/mc/extfs.d/
	fish:           /home/patben/.local/share/mc/fish/
	mcedit macros:  /home/patben/.local/share/mc/mc.macros
	mcedit external macros: /home/patben/.local/share/mc/mcedit/macros.d/macro.*
    Cache directory:  /home/patben/.cache/mc/
patben@esp-patben-lin:~$ /usr/local/bin/mc --configure-options

patben@esp-patben-lin:~$ 

[slavazanko]

Can you log in to AIX FTP-server (via console ftp client or via telnet) and provide to us output of 'ls' FTP-command?
This bug strongly related to the output format of 'ls' command from FTP-server, therefore we needed this output for checking our parser.

[leadman]

I have already provided output of 'ls' FTP-command in ticket #2635, but since you insist, here it follows:

patben@esp-patben-lin:/$ ftp P550
Connected to PROMAK.
220 p550 FTP server (Version 4.2 Wed Nov 26 05:54:26 CST 2008) ready.
Name (P550:patben): promak
331 Password required for promak.
Password:
230-Last unsuccessful login: Mon Jun 25 18:01:09 DFT 2012 on ssh from 10.251.224.190
230-Last login: Wed Jul  4 17:11:48 DFT 2012 on ftp from ::ffff:10.251.224.190
230 User promak logged in.
Remote system type is UNIX.
Using binary mode to transfer files.
ftp> pwd
257 "/appl/promak" is current directory.
ftp> ls
200 PORT command successful.
150 Opening data connection for /bin/ls.
total 107808
-rw-rw----    1 darkna   promak         6624 Jun 26 10:21 ${plktmp}ddd
-rw-rw----    1 brydro   promak            0 Mar 11 2007  *
drwxrws---    3 promak   promak          256 Oct 01 2007  .cpan
-rwxr-----    1 promak   promak          879 Aug 13 2009  .profile
-rw-------    1 promak   promak         6344 Jul 05 00:11 .sh_history
drwx--S---    2 promak   promak          256 Jan 23 2009  .ssh
-rw-------    1 promak   promak          277 Jun 30 00:41 .vi_history
drwxrwsrwx   21 promak   promak         4096 Dec 22 2010  A
drwxrws---  104 promak   promak        16384 May 25 20:37 JP
drwxrws---    3 promak   promak         4096 Feb 01 14:43 MDM
drwx--x---    2 promak   promak          256 Mar 12 2007  Mail
drwxrws---   22 promak   promak         4096 Jun 28 18:09 N
drwxrws---   26 promak   promak         4096 May 25 23:20 O
drwxrws---   10 promak   promak         4096 Jun 21 10:18 PB
drwxrws---    4 promak   promak         4096 Aug 12 2009  S
drwxr-s---   13 promak   promak         4096 Nov 26 2009  TECH
-rw-rw----    1 matpil   promak            0 Jul 05 09:10 TMP_FILED
-rw-rw----    1 promak   promak         2808 Sep 05 2011  ala.txt
-rw-rw----    1 promak   promak          228 Apr 16 2009  archiwum.log
drwxr-s---    2 promak   promak          256 Mar 23 2011  bin
-rw-rw----    1 brydro   promak           69 Apr 25 14:23 brydro20120425122315.txt
-rw-rw----    1 darkna   promak            0 Jul 03 08:15 core
-rw-rw----    1 promak   promak         2664 Jul 31 2007  dupadupa.sql
-rwxr-x---    1 promak   promak         7340 Aug 25 2007  dupkop.sh
-rw-rw----    1 promak   promak        16365 Jul 19 2010  duppppa
drwxr-s---    2 promak   promak          256 Jun 22 2011  err.log
-rw-rw----    1 promak   promak            0 Mar 12 2007  err_bkx
drwxr-s---    2 promak   promak         4096 Aug 12 2009  etc
-rw-rw-r--    1 promak   promak           94 Feb 21 09:18 ftp594398.cfg
-rw-r-----    1 promak   promak         2855 Apr 22 2009  goenv_arch
-rw-r-----    1 promak   promak         2875 Aug 12 2009  goenv_nsdr
-rw-r-----    1 promak   promak         2821 Aug 12 2009  goenv_optksg
-rw-r-----    1 promak   promak     10691433 Mar 01 14:22 ike.elg
-rw-rw----    1 promak   promak        32136 Mar 20 11:14 index.html
drwxrws---   65 promak   promak         4096 Mar 01 12:38 jkk
-rw-r-----    1 promak   promak           55 Feb 22 2011  jkk.awk
-rw-rw----    1 matpil   promak           64 Jul 04 19:19 jkk.log
-rw-rw----    1 promak   promak          338 Mar 12 2007  jkkver.sh
-rw-rw----    1 matpil   promak           12 Jun 13 12:37 jpdatatmp.txt
-rw-rw----    1 promak   promak          132 Jun 25 2010  jpsir.log
-rw-r-----    1 promak   promak            3 Mar 12 2007  koniecstrony
-rw-rw-rw-    1 promak   promak           64 Jul 04 17:38 ksg_kart_drw.unl
-rw-rw----    1 promak   promak          190 Aug 17 2009  ksg_kartpw.test.unl
drwxr-s---    2 promak   promak          256 Aug 28 2009  libsh
drwxr-xr-x    2 root     system          256 Mar 06 2007  lost+found
-rw-rw----    1 matpil   promak           69 Mar 27 19:57 matpil20120327175745.txt
-rw-rw----    1 matpil   promak           81 May 14 19:57 matpil20120514175733.txt
-rw-------    1 promak   promak        10743 Aug 13 2009  mbox
-rwxr-x---    1 promak   promak           68 Mar 12 2007  odproappl
-rw-rw----    1 promak   promak           83 Nov 19 2010  papier_notow.sql
-rw-rw-r--    1 promak   promak         1785 Jun 14 2011  plikwyj.txt
-rw-r--r--    1 promak   promak      8477579 Jul 02 23:24 proappl.err
-rwx------    1 promak   promak         1957 Aug 24 2009  proappl.sh
-rwx------    1 promak   promak          674 Mar 12 2007  proappl2f80
-rwx------    1 promak   promak         9272 Mar 28 2007  prokop.bkp
-rwx------    1 promak   promak      1107611 Jul 04 22:57 prokop.log
-rw-r--r--    1 promak   promak     20194873 Jul 04 23:23 prokop.promak
-rwxr-x---    1 promak   promak         8769 May 09 18:17 prokop.sh
-rwx------    1 promak   promak         8072 Jun 26 2010  prokop.sh.old
-rw-r--r--    1 promak   promak       345415 Jun 30 22:25 prokop.wsunle
-rwx------    1 promak   promak         6004 Mar 12 2007  prokop2
-rwx------    1 promak   promak         6169 Mar 12 2007  prokopF50
-rwx------    1 promak   promak         6156 Mar 12 2007  prokopF80
-rw-rw----    1 promak   promak            0 Aug 12 2010  promak
-rwx------    1 promak   promak       201476 Jul 04 23:06 promak.log
-rw-rw----    1 promak   promak      4594135 Aug 12 2009  promak_pliki.txt
-rw-r--r--    1 promak   promak       222319 Jul 05 09:05 promonusr.err
-rwx------    1 promak   promak          334 Apr 22 2009  promonusr.sh
-rw-r--r--    1 promak   promak      7058202 Jul 05 06:07 prostat.err
-rwx------    1 promak   promak          525 Sep 19 2009  prostat.sh
-rw-r--r--    1 promak   promak       536893 Jul 04 19:15 prowsunle.err
-rwx------    1 promak   promak          868 Nov 27 2008  prowsunle.sh
-rw-rw----    1 barros   promak            0 Jul 14 2010  salda_z_bazy
-rw-rw----    1 promak   promak         1932 Jun 19 13:00 smit.log
-rw-rw----    1 promak   promak          908 Jun 19 13:00 smit.script
-rw-rw----    1 promak   promak         1174 Jun 19 13:00 smit.transaction
-rw-rw-rw-    1 promak   promak          613 Jun 20 2011  sqexplain.out
-rwxr-xr-x    1 promak   promak           45 Mar 12 2007  srozl.sh
-rwxr-x---    1 promak   promak         1602 Aug 12 2009  taruj.sh
-rw-rw----    1 darkna   promak      1270950 Jan 23 16:41 tax_dznk_nz.unl
drwxrws---   14 promak   promak         4096 Aug 04 2009  temp
drwxrwx---    3 promak   promak         4096 Mar 23 2011  tmp
-rw-rw----    1 promak   promak           75 Jul 22 2010  tmp_drw_pwnul.unl
-rw-rw-rw-    1 promak   promak          104 Jul 04 17:38 tmp_konta4nik.unl
-rw-rw-rw-    1 promak   promak          196 Jun 11 09:29 tmpa_drw_portfel09:29:44.774
-rw-rw-rw-    1 promak   promak          196 Jun 11 09:29 tmpa_drw_portfel09:29:44.967
-rw-rw-rw-    1 promak   promak            0 Jun 11 09:29 tmpa_drw_portfel_pw09:29:44.774
-rw-rw-rw-    1 promak   promak            0 Jun 11 09:29 tmpa_drw_portfel_pw09:29:44.967
-rw-rw-rw-    1 promak   promak         3293 Jun 11 09:29 tmpa_drw_pw09:29:44.774
-rw-rw-rw-    1 promak   promak         3293 Jun 11 09:29 tmpa_drw_pw09:29:44.967
-rwxr-x---    1 promak   promak         1562 Dec 04 2009  ws_go
-rwxr-x---    1 promak   promak          406 Aug 14 2009  ws_off
-rw-rw----    1 promak   promak        41803 Jun 30 22:13 wsunle.log
-rw-r--r--    1 promak   promak         1601 Apr 04 2007  wsunload.err
-rw-rw----    1 promak   promak          129 Mar 29 2011  zagraniczne.log
-rw-r-----    1 promak   promak           57 Mar 12 2007  zrestartuj_informixa
226 Transfer complete.
ftp> 

[leadman]

Recently nothing was happening around this bug, so I investigated it a bit and have noticed following relation. The files with additional space in name are ONLY the ones, which have NOT been modified at current day. In such cases the 'hh:mm' part of modification date is replaced by 'YYYY ' followed by additional space.
I am attaching screen shot, illustrating this situation:
Ticket_2841_mc_adds_spaces_at_the_beginning_of_some_files_on_AIX_ftp_servers.png

[leadman]

Again I have taken deeper look into this bug and might have found cause for this problem. If you take a look at remotes/origin/master/lib/vfs/parse_ls_vga.c at line 806:

*num_spaces = column_ptr[idx] - column_ptr[idx - 1] - strlen (columns[idx - 1]);

I would change this line to:

/* AFAIU, following line checks how many spaces file name contain at the beginning of itself?
 * I am aware that this is probably not the best place to do it, but apparently column containing
 * 'hh:mm' or 'YYYY' string is always in the same place, and is always 5 characters long. The only
 * difference among FTP servers is whether additional space is served before or after 'YYYY' string.
 * Taking above into account, maybe instead of counting *num_spaces this way:
 *      *num_spaces = column_ptr[idx] - column_ptr[idx - 1] - strlen (columns[idx - 1]);
 * 
 * ...it would be sufficient just to calculate it as follows?
 *      *num_spaces = column_ptr[idx] - column_ptr[idx - 1] - 5;
 * 
 * Theoretically 'column_ptr[idx - 1]' can change it's value depending on starting position of 'YYYY' string,
 * but I have just compiled it my way, and it was working fine both with AIX and Debian FTP servers.
 * I am not sure how this change may interfere with other mc's components...
 */
*num_spaces = column_ptr[idx] - column_ptr[idx - 1] - 5;

I have created local git branch, tracking the master, but frankly speaking I am afraid to break something with mc's repository. Could someone give me a hint what to do next to push my local branch for others to test it? Or some good how-to?

[andrew_b]

A new FTP parser is implemented in MC. Thanks ​lftp project for that.
#3174 should be fixed too.

Branch: 2841_ftp_ls_parser.
Initial changeset:63cd8142f8728f9ea69f4206921e4e4a559ee678

[andrew_b]

(In #3174) This bug should be fixed as well as #2841 due to new FTP parser in the 2841_ftp_ls_parser branch.
Initial changeset:ac5794641ebaac52420dd6cb041b6ee775cda819

[ossi]

is that branch still WIP? it contains several unsquashed fixup! commits.
i'd also squash the "sync with lftp" commits into the initial import, as they don't add value here.

i don't quite understand the "get rid of extra string duplication" commit - while the second portion is obvious (and in fact i already thought of it while reading the import commit), the one inside the loop seems bogus, as the parsers *can* in fact clobber the string (at least MLSD does), and several are tried on the same input.

it would probably make sense to save the guessed parser persistently inside the connection object, both for performance and for reliability (when dirs with few entries are subsequently entered).

[andrew_b]

Replying to ossi:

is that branch still WIP? it contains several unsquashed fixup! commits.

Thanks for review! push'd -f.

i'd also squash the "sync with lftp" commits into the initial import, as they don't add value here.

Done,

i don't quite understand the "get rid of extra string duplication" commit - while the second portion is obvious (and in fact i already thought of it while reading the import commit), the one inside the loop seems bogus, as the parsers *can* in fact clobber the string (at least MLSD does), and several are tried on the same input.

Agree. Get rid of extra string duplication in parse_ls_line().

it would probably make sense to save the guessed parser persistently inside the connection object, both for performance and for reliability (when dirs with few entries are subsequently entered).

It's not so simple as wanted. Let's keep the parser as close to the borrowed one as possible for now and make optimization after some time.

[andrew_b]

Merged to master: [ff0949ece83fc8a7b5166efcc9609f3289392384].

git log --pretty=oneline 32c28026f..ff0949ece