bug-inetutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug-inetutils] Re: Bug in inetutils-1.6: FTPD transfers files with a si


From: Rachid Koucha
Subject: [bug-inetutils] Re: Bug in inetutils-1.6: FTPD transfers files with a size of 0
Date: Thu, 22 Jan 2009 00:47:03 +0100
User-agent: Thunderbird 2.0.0.19 (X11/20090105)

I just saw today that Mr Asier Tamayo faced the same problem in inetutils-1.5 ==> Look at thread : Bug in ftpd (inetutils 1.5)

Rachid Koucha a écrit :
Hi,

I made a patch for this issue. Please find attached the description of the patch in HTML format and the patch itself.

Regards.

Rachid Koucha a écrit :
Hi,

On platforms where HAVE_MMAP is not defined, FTPD transfers file with a size of 0.

Here is an analysis that I have done for my company (Alcatel-Lucent).


The RETR command sent by the client to the server to get a file, triggers the following:

  • ftpcmd.y (yacc grammar) triggers the retrieve() function with the following rule:
    | RETR check_login SP pathname CRLF
        {
            if ($2 && $4 != NULL)
                retrieve((char *) 0, $4);
            if ($4 != NULL)
                free($4);

  • ftpd.c/retrieve(NULL, pathname) begins with the following:
size_t buffer_size = 0;

if (cmd == 0)
  {
    fin = fopen (name, "r"), closefunc = fclose;
    st.st_size = 0;
  }

  • ...the function gets the size of the file to transfer (st.st_size):
  if (cmd == 0 && (fstat (fileno (fin), &st) < 0 || !S_ISREG (st.st_mode)))
    {
      reply (550, "%s: not a plain file.", name);
      goto done;
    }

  • ...the function calls datacon() to establish the data connection to transfer the data of the file:
  dout = dataconn (name, st.st_size, "w");
  • ... then it calls  send_data() to send the content of the file over the data connection:
  send_data (fin, dout, buffer_size);
  • But as you can see, the variable "buffer_size" passed to send_data() is equal to 0 because it has not been modified since the beginning of the function where it has been initialized to 0
  • In ftpd.c/send_data(), the parameter "blksize" is equal to 0 (value of the preceding "buffer_size" variable)
  • ftpd.c/send_data() behaves in two possible ways. Either it reads the file to send with mmap() or it reads the file with the file system services. This is controlled by HAVE_MMAP flag. This flag is defined with the "configure" script located at the top level of the inetutils subtree. When "configure" is run, it generates the file "config.h" at the top level of the inetutils subtree. On my laptop running Linux ubuntu, "configure" sets the following in config.h:
/* Define to 1 if you have a working `mmap' system call. */
#define HAVE_MMAP 1
  • When it is defined, everything is fine in send_data() both for ASCII mode (case TYPE_A:) and Binary mode (case TYPE_I:) because we don't use the parameter "blksize". For example, for TYPE I, we run the following code to send the data:
    case TYPE_I:
    case TYPE_L:
#ifdef HAVE_MMAP
      if (file_size > 0 && curpos >= 0 && buf != MAP_FAILED)
    {
      bp = buf;
      len = filesize;
      do
        {
          cnt = write (netfd, bp, len);
          len -= cnt;
          bp += cnt;
          if (cnt > 0)
        byte_count += cnt;
        }
      while (cnt > 0 && len > 0);
      transflag = 0;
      munmap (buf, (size_t) filesize);
      if (cnt < 0)
        goto data_err;
      reply (226, "Transfer complete with mmap (blksize = %u).", (u_int)blksize);
      return;
    }
#endif
  • But if HAVE_MMAP is not defined in "config.h" (I commented it out), the "case TYPE_A" still works because it does not use "blksize", but the "case TYPE I" does not work because it uses the parameter "blksize" which has a size of 0. The following while loop stops immediately as we call read() with a blksize equal to 0:
      buf = malloc ((u_int) blksize);
      if (buf == NULL)
    {
      transflag = 0;
      perror_reply (451, "Local resource failure: malloc");
      return;
    }
      while ((cnt = read (filefd, buf, (u_int) blksize)) > 0 &&
         write (netfd, buf, cnt) == cnt)
    byte_count += cnt;
      transflag = 0;
      free (buf);
      if (cnt != 0)
    {
      if (cnt < 0)
        goto file_err;
      goto data_err;
    }
      reply (226, "Transfer complete.");
      return;

So, I would propose the following patch:
  • In ftpd.c/retrieve(), we initialize the local_variable "buffer_size" to 4096 bytes or better ST_BLKSIZE (st) provided that we call fstat(fileno (fin), &st):
void
retrieve (const char *cmd, const char *name)
{
  FILE *fin, *dout;
  struct stat st;
  int (*closefunc) (FILE *);
  size_t buffer_size = 0;

  if (cmd == 0)
    {
      fin = fopen (name, "r"), closefunc = fclose;
      st.st_size = 0;
     
buffer_size = 4096;
    }
  else
    {
      char line[BUFSIZ];

      snprintf (line, sizeof line, cmd, name);
      name = line;
      fin = ftpd_popen (line, "r"), closefunc = ftpd_pclose;
      st.st_size = -1;
      buffer_size = BUFSIZ;
    }

I tried it with HAVE_MMAP not defined in "config.h" and it works.


Regards.

Rachid Koucha.



--
signature Rachid Koucha
Tel: 0619838579

Mail: address@hidden
WEB site: http://rachid.koucha.free.fr/

Patch for FTPD in inetutils-1.6 to fix the problem of file transfer which ends with 0 bytes received on client side Author: R. Koucha Last update:15-Jan-2009

Patch to fix the reception of files with a size of 0 bytes

 in

FTPD from inetutils-1.6

Introduction Detailed description How to apply the patch About the author

Introduction

When HAVE_MMAP is not defined in inetutils-1.6, the RETR commands from the FTP client ends with the sending of files with a size of 0 bytes from FTPD server.

Detailed description

The RETR command sent by the client to the server to get a file, triggers the following when HAVE_MMAP flag is not defined:
  • ftpcmd.y (yacc grammar) triggers the retrieve() function with the following rule:
    | RETR check_login SP pathname CRLF         {             if ($2 && $4 != NULL)                 retrieve((char *) 0, $4);             if ($4 != NULL)                 free($4);
  • ftpd.c/retrieve(NULL, pathname) begins with the following:
size_t buffer_size = 0; if (cmd == 0)   {     fin = fopen (name, "r"), closefunc = fclose;     st.st_size = 0;   }
  • ...the function gets the size of the file to transfer (st.st_size):
  if (cmd == 0 && (fstat (fileno (fin), &st) < 0 || !S_ISREG (st.st_mode)))     {       reply (550, "%s: not a plain file.", name);       goto done;     }
  • ...the function calls datacon() to establish the data connection:
  dout = dataconn (name, st.st_size, "w");
  • ... then it calls  send_data() to send the content of the file over the data connection:
  send_data (fin, dout, buffer_size);
  • But as you can see, the variable "buffer_size" passed to send_data() is equal to 0 because it has not been modified since the beginning of the function where it has been initialized to 0
  • In ftpd.c/send_data(), the parameter "blksize" is equal to 0 (value of the preceding "buffer_size" variable)
  • ftpd.c/send_data() behaves in two possible ways. Either it reads the file to send with mmap() or it reads the file with the file system services. This is controlled by HAVE_MMAP flag. This flag is defined with the "configure" script located at the top level of the inetutils subtree. When "configure" is run, it generates the file "config.h" at the top level of the inetutils subtree. On my laptop running Linux ubuntu, "configure" sets the following in config.h:
/* Define to 1 if you have a working `mmap' system call. */ #define HAVE_MMAP 1
  • When it is defined, everything is fine in send_data() both for ASCII mode (case TYPE_A:) and Binary mode (case TYPE_I:) because we don't use the parameter "blksize". For example, for TYPE I, we run the following code to send the data:
    case TYPE_I:     case TYPE_L: #ifdef HAVE_MMAP       if (file_size > 0 && curpos >= 0 && buf != MAP_FAILED)     {       bp = buf;       len = filesize;       do         {           cnt = write (netfd, bp, len);           len -= cnt;           bp += cnt;           if (cnt > 0)         byte_count += cnt;         }       while (cnt > 0 && len > 0);       transflag = 0;       munmap (buf, (size_t) filesize);       if (cnt < 0)         goto data_err;       reply (226, "Transfer complete with mmap (blksize = %u).", (u_int)blksize);       return;     } #endif
  • But if HAVE_MMAP is not defined in "config.h" (I commented it out), the "case TYPE_A" still works because it does not use "blksize", but the "case TYPE I" does not work because it uses the parameter blksize which has a size of 0. The following while loop stops immediately as we call read() with a blksize equal to 0:
      buf = malloc ((u_int) blksize);       if (buf == NULL)     {       transflag = 0;       perror_reply (451, "Local resource failure: malloc");       return;     }       while ((cnt = read (filefd, buf, (u_int) blksize)) > 0 &&          write (netfd, buf, cnt) == cnt)     byte_count += cnt;       transflag = 0;       free (buf);       if (cnt != 0)     {       if (cnt < 0)         goto file_err;       goto data_err;     }       reply (226, "Transfer complete.");       return; So, I would propose the following patch:
  • In ftpd.c/retrieve(), we initialize the local_variable "buffer_size" to 4096 bytes:
#define FTPD_ST_BLKSIZE(st)  ((st).st_blksize ? (st).st_blksize : 4096) void retrieve (const char *cmd, const char *name) {   FILE *fin, *dout;   struct stat st;   int (*closefunc) (FILE *);   size_t buffer_size = 0;   if (cmd == 0)     {       fin = fopen (name, "r"), closefunc = fclose;       if (fin == NULL)         {           if (errno != 0)         {           perror_reply (550, name);           if (cmd == 0)             {               LOGCMD ("get", name);             }         }           return;         }       if (fstat(fileno(fin), &st) < 0 || !S_ISREG (st.st_mode))         {           reply (550, "%s: not a plain file.", name);           goto done;         }       buffer_size = FTPD_ST_BLKSIZE(st);     }   else     {       char line[BUFSIZ];       snprintf (line, sizeof line, cmd, name);       name = line;       fin = ftpd_popen (line, "r"), closefunc = ftpd_pclose;       if (fin == NULL)         {           if (errno != 0)         {           perror_reply (550, name);           if (cmd == 0)             {               LOGCMD ("get", name);             }         }           return;         }       st.st_size = -1;       buffer_size = BUFSIZ;     }   byte_count = -1;   if (restart_point)     {       if (type == TYPE_A)     {       off_t i, n;       int c;       n = restart_point;       i = 0;       while (i++ < n)         {           c = getc (fin);           if (c == EOF)         {           perror_reply (550, name);           goto done;         }           if (c == '\n')         i++;         }     }       else if (lseek (fileno (fin), restart_point, SEEK_SET) < 0)     {       perror_reply (550, name);       goto done;     }     }   dout = dataconn (name, st.st_size, "w");   if (dout == NULL)     goto done;   send_data (fin, dout, buffer_size);   fclose (dout);   data = "">   pdata = -1; done:   if (cmd == 0)     LOGBYTES ("get", name, byte_count);   (*closefunc) (fin); } I tried it with HAVE_MMAP not defined in "config.h" and it works.

How to apply the patch

$ tar xvfz inetutils-1.6.tar.gz $ cd inetutils-1.6 $ cp ....../inetutils-1.6-retr_size_0.diff . $ patch -p1 < inetutils-1.6-retr_size_0.diff patching file ftpd/ftpd.c

About the author

The author is an engineer in computer sciences located in France. He is glad to graciously offer this utility under the GPL open source license. He can be contacted at "rachid dot koucha at free dot fr" or you can have a look at his WEB home page.

--
signature Rachid Koucha
Tel: 0619838579

Mail: address@hidden
WEB site: http://rachid.koucha.free.fr/

reply via email to

[Prev in Thread] Current Thread [Next in Thread]