More FTP bug fixes

git-svn-id: svn://svn.code.sf.net/p/nuttx/code/trunk@3669 42af7a65-404d-4744-a932-0658087f49c3

5 files changed, 170 insertions, 100 deletions

diff --git a/apps/netutils/ftpc/ftpc_getfile.c b/apps/netutils/ftpc/ftpc_getfile.c
index 846a1e1db..8a60c514b 100644
--- a/apps/netutils/ftpc/ftpc_getfile.c
+++ b/apps/netutils/ftpc/ftpc_getfile.c
@@ -149,14 +149,18 @@ static int ftpc_recvinit(struct ftpc_session_s *session, FAR const char *path,
       return ERROR;
     }
 
-  /* Accept a connection on the data socket (unless passive mode then the
-   * function does nothing).
+  /* In active mode, we need to accept a connection on the data socket
+   * (in passive mode, we have already connected the data channel to
+   * the FTP server).
    */
 
-  ret = ftpc_sockaccept(&session->data, FTPC_IS_PASSIVE(session));
-  if (ret != OK)
+  if (!FTPC_IS_PASSIVE(session))
     {
-      ndbg("Data connection not accepted\n");
+      ret = ftpc_sockaccept(&session->data);
+      if (ret != OK)
+        {
+          ndbg("Data connection not accepted\n");
+        }
     }
 
   return ret;
@@ -204,6 +208,8 @@ static int ftpc_recvbinary(FAR struct ftpc_session_s *session,
   int err;
   int ret = OK;
 
+  /* Allocate an I/O buffer */
+
   buf = (FAR char *)malloc(CONFIG_FTP_BUFSIZE);
   if (!buf)
     {
@@ -213,13 +219,18 @@ static int ftpc_recvbinary(FAR struct ftpc_session_s *session,
 
   for (;;)
     {
-      if (ftpc_waitinput(session) != 0)
+      /* Wait for input on the socket */
+
+      ret = ftpc_waitinput(session);
+      if (ret != OK)
         {
           nvdbg("ftpc_waitinput() failed\n");
           err = EIO;
           goto errout_with_buf;
         }
 
+      /* Read the data from the socket */
+
       nread = fread(buf, sizeof(char), CONFIG_FTP_BUFSIZE, rinstream);
       if (nread <= 0)
         {
@@ -227,12 +238,16 @@ static int ftpc_recvbinary(FAR struct ftpc_session_s *session,
 
           if (nread < 0)
             {
+              /* errno should already be set by fread */
+
               (void)ftpc_xfrabort(session, rinstream);
               ret = ERROR;
             }
           break;
         }
 
+      /* Write the data to the file */
+
       nwritten = fwrite(buf, sizeof(char), nread, loutstream);
       if (nwritten != nread)
         {
@@ -372,11 +387,11 @@ int ftpc_getfile(SESSION handle, FAR const char *rname, FAR const char *lname,
 
   if (xfrmode == FTPC_XFRMODE_ASCII)
     {
-      ret = ftpc_recvbinary(session, session->data.instream, loutstream);
+      ret = ftpc_recvtext(session, session->data.instream, loutstream);
     }
   else
     {
-      ret = ftpc_recvtext(session, session->data.instream, loutstream);
+      ret = ftpc_recvbinary(session, session->data.instream, loutstream);
     }
 
   ftpc_sockclose(&session->data);
@@ -419,40 +434,68 @@ errout:
 int ftpc_recvtext(FAR struct ftpc_session_s *session,
                   FAR FILE *rinstream, FAR FILE *loutstream)
 {
-  char *buf = (char *)malloc(CONFIG_FTP_BUFSIZE);
-  int c;
+  FAR char *buf;
+  int ch;
   int ret = OK;
 
-  while((c = fgetc(rinstream)) != EOF) {
+  /* Allocate an I/O buffer */
 
-    if (ftpc_waitinput(session) != 0)
-      {
-        break;
-      }
+  buf = (FAR char *)malloc(CONFIG_FTP_BUFSIZE);
+  if (!buf)
+    {
+      set_errno(ENOMEM);
+      return ERROR;
+    }
+
+  /* Read the next character from the incoming data stream */
 
-    if (c == '\r')
+  while ((ch = fgetc(rinstream)) != EOF)
     {
-      c = fgetc(rinstream);
-      if (c == EOF)
+      /* Wait for input on the socket */
+
+      ret = ftpc_waitinput(session);
+      if (ret != OK)
         {
-         (void)ftpc_xfrabort(session, rinstream);
-          ret = ERROR;
+          nvdbg("ftpc_waitinput() failed\n");
           break;
         }
-      if (c != '\n')
+
+      /* Is it a carriage return? Compress \r\n to \n */
+
+      if (ch == '\r')
         {
-          ungetc(c, rinstream);
-          c = '\r';
+          /* Get the next character */
+
+          ch = fgetc(rinstream);
+          if (ch == EOF)
+            {
+              /* Ooops... */
+
+              (void)ftpc_xfrabort(session, rinstream);
+              ret = ERROR;
+              break;
+            }
+
+          /* If its not a newline, then keep the carriage return */
+
+          if (ch != '\n')
+            {
+              ungetc(ch, rinstream);
+              ch = '\r';
+            }
         }
-    }
 
-    if (fputc(c, loutstream) == EOF)
+    /* Then write the character to the output file */
+
+    if (fputc(ch, loutstream) == EOF)
       {
         (void)ftpc_xfrabort(session, loutstream);
         ret = ERROR;
         break;
       }
 
+    /* Increase the actual size of the file by one */
+
     session->size++;
   }
 
diff --git a/apps/netutils/ftpc/ftpc_internal.h b/apps/netutils/ftpc/ftpc_internal.h
index 6600b1220..20fbbccc4 100644
--- a/apps/netutils/ftpc/ftpc_internal.h
+++ b/apps/netutils/ftpc/ftpc_internal.h
@@ -257,7 +257,7 @@ EXTERN int ftpc_sockconnect(FAR struct ftpc_socket_s *sock,
                            FAR struct sockaddr_in *addr);
 EXTERN int ftpc_sockgetsockname(FAR struct ftpc_socket_s *sock,
                                 FAR struct sockaddr_in *sa);
-EXTERN int ftpc_sockaccept(FAR struct ftpc_socket_s *sock, bool passive);
+EXTERN int ftpc_sockaccept(FAR struct ftpc_socket_s *sock);
 EXTERN int ftpc_socklisten(FAR struct ftpc_socket_s *sock);
 EXTERN void ftpc_sockcopy(FAR struct ftpc_socket_s *dest,
                           FAR const struct ftpc_socket_s *src);
diff --git a/apps/netutils/ftpc/ftpc_listdir.c b/apps/netutils/ftpc/ftpc_listdir.c
index d5de10ec3..e3af9b122 100644
--- a/apps/netutils/ftpc/ftpc_listdir.c
+++ b/apps/netutils/ftpc/ftpc_listdir.c
@@ -199,13 +199,19 @@ static int ftpc_recvdir(FAR struct ftpc_session_s *session,
       return ERROR;
     }
 
-  /* Accept the connection from the server */
+  /* In active mode, we need to accept a connection on the data socket
+   * (in passive mode, we have already connected the data channel to
+   * the FTP server).
+   */
 
-  ret = ftpc_sockaccept(&session->data, FTPC_IS_PASSIVE(session));
-  if (ret != OK)
+  if (!FTPC_IS_PASSIVE(session))
     {
-      ndbg("ftpc_sockaccept() failed: %d\n", errno);
-      return ERROR;
+      ret = ftpc_sockaccept(&session->data);
+      if (ret != OK)
+        {
+          ndbg("ftpc_sockaccept() failed: %d\n", errno);
+          return ERROR;
+        }
     }
 
   /* Receive the NLST directory list  */
diff --git a/apps/netutils/ftpc/ftpc_putfile.c b/apps/netutils/ftpc/ftpc_putfile.c
index 94942edba..e008da3b1 100644
--- a/apps/netutils/ftpc/ftpc_putfile.c
+++ b/apps/netutils/ftpc/ftpc_putfile.c
@@ -114,6 +114,8 @@ static int ftpc_sendbinary(FAR struct ftpc_session_s *session,
   buf = (char *)malloc(CONFIG_FTP_BUFSIZE);
   for (;;)
     {
+      /* Read data from the file */
+
       nread = fread(buf, sizeof(char), CONFIG_FTP_BUFSIZE, linstream);
       if (nread <= 0)
         {
@@ -127,12 +129,17 @@ static int ftpc_sendbinary(FAR struct ftpc_session_s *session,
           break;
         }
 
-      if (ftpc_waitoutput(session) != 0)
+      /* Wait to make sure that we send the data without blocking */
+
+      ret = ftpc_waitoutput(session);
+      if (ret != OK)
         {
           ret = ERROR;
           break;
         }
 
+      /* Send the data */
+
       nwritten = fwrite(buf, sizeof(char), nread, routstream);
       if (nwritten != nread)
         {
@@ -141,6 +148,8 @@ static int ftpc_sendbinary(FAR struct ftpc_session_s *session,
            break;
         }
 
+      /* Increment the size of the file sent */
+
       session->size += nread;
     }
 
@@ -163,13 +172,19 @@ static int ftpc_sendtext(FAR struct ftpc_session_s *session,
   int c;
   int ret = OK;
 
+  /* Write characters one at a time. */
+
   while((c = fgetc(linstream)) != EOF)
     {
+      /* Make sure that we can send the character without blocking */
+
       if (ftpc_waitoutput(session) != 0)
         {
           break;
         }
 
+      /* If it is a newline, send a carriage return too */
+
       if (c == '\n')
         {
           if (fputc('\r', routstream) == EOF)
@@ -179,9 +194,13 @@ static int ftpc_sendtext(FAR struct ftpc_session_s *session,
               break;
             }
 
+          /* Increment the size of the file sent */
+
           session->size++;
         }
 
+      /* Send the character */
+
       if (fputc(c, routstream) == EOF)
         {
           (void)ftpc_xfrabort(session, routstream);
@@ -189,6 +208,8 @@ static int ftpc_sendtext(FAR struct ftpc_session_s *session,
           break;
         }
 
+      /* Increment the size of the file sent */
+
       session->size++;
     }
 
@@ -340,20 +361,30 @@ static int ftpc_sendfile(struct ftpc_session_s *session, const char *path,
    *   with a mark. 
    */
 
-  ret = ftpc_sockaccept(&session->data, FTPC_IS_PASSIVE(session));
-  if (ret != OK)
+  /* In active mode, we need to accept a connection on the data socket
+   * (in passive mode, we have already connected the data channel to
+   * the FTP server).
+   */
+
+  if (!FTPC_IS_PASSIVE(session))
     {
-      ndbg("Data connection not accepted\n");
-      return ERROR;
+      ret = ftpc_sockaccept(&session->data);
+      if (ret != OK)
+        {
+          ndbg("Data connection not accepted\n");
+          return ERROR;
+        }
     }
 
+  /* Then perform the data transfer */
+
   if (xfrmode == FTPC_XFRMODE_ASCII)
     {
-      ret = ftpc_sendbinary(session, stream, session->data.instream);
+      ret = ftpc_sendtext(session, stream, session->data.instream);
     }
   else
     {
-      ret = ftpc_sendtext(session, stream, session->data.instream);
+      ret = ftpc_sendbinary(session, stream, session->data.instream);
     }
 
   ftpc_sockflush(&session->data);
diff --git a/apps/netutils/ftpc/ftpc_socket.c b/apps/netutils/ftpc/ftpc_socket.c
index bfa8ec7c1..d245c812d 100644
--- a/apps/netutils/ftpc/ftpc_socket.c
+++ b/apps/netutils/ftpc/ftpc_socket.c
@@ -209,81 +209,71 @@ void ftpc_sockcopy(FAR struct ftpc_socket_s *dest,
  * Name: ftpc_sockaccept
  *
  * Description:
- *   Accept a connection on the data socket (unless passive mode then this
- *   function does nothing).
+ *   Accept a connection on the data socket.  This function is onlly used
+ *   in active mode. 
+ *
+ *   In active mode FTP the client connects from a random port (N>1023) to the
+ *   FTP server's command port, port 21. Then, the client starts listening to
+ *   port N+1 and sends the FTP command PORT N+1 to the FTP server. The server
+ *   will then connect back to the client's specified data port from its local
+ *   data port, which is port 20. In passive mode FTP the client initiates
+ *   both connections to the server, solving the problem of firewalls filtering 
+ *   the incoming data port connection to the client from the server. When
+ *   opening an FTP connection, the client opens two random ports locally
+ *   (N>1023 and N+1). The first port contacts the server on port 21, but
+ *   instead of then issuing a PORT command and allowing the server to connect
+ *   back to its data port, the client will issue the PASV command. The result
+ *   of this is that the server then opens a random unprivileged port (P >
+ *   1023) and sends the PORT P command back to the client. The client then
+ *   initiates the connection from port N+1 to port P on the server to transfer 
+ *   data.
  *
  ****************************************************************************/
 
-int ftpc_sockaccept(struct ftpc_socket_s *sock, bool passive)
+int ftpc_sockaccept(FAR struct ftpc_socket_s *sock)
 {
   struct sockaddr addr;
   socklen_t addrlen;
 
-  /* In active mode FTP the client connects from a random port (N>1023) to the
-   * FTP server's command port, port 21. Then, the client starts listening to
-   * port N+1 and sends the FTP command PORT N+1 to the FTP server. The server
-   * will then connect back to the client's specified data port from its local
-   * data port, which is port 20. In passive mode FTP the client initiates
-   * both connections to the server, solving the problem of firewalls filtering 
-   * the incoming data port connection to the client from the server. When
-   * opening an FTP connection, the client opens two random ports locally
-   * (N>1023 and N+1). The first port contacts the server on port 21, but
-   * instead of then issuing a PORT command and allowing the server to connect
-   * back to its data port, the client will issue the PASV command. The result
-   * of this is that the server then opens a random unprivileged port (P >
-   * 1023) and sends the PORT P command back to the client. The client then
-   * initiates the connection from port N+1 to port P on the server to transfer 
-   * data.
+  /* Any previous socket should have been uninitialized (0) or explicitly
+   * closed (-1).  But the path to this function may include a call to
+   * ftpc_sockinit().  If so... close that socket and call accept to
+   * get a new one.
    */
 
-  if (!passive)
+  if (sock->sd > 0)
     {
-      /* Any previous socket should have been uninitialized (0) or explicitly
-       * closed (-1).  But the path to this function may include a call to
-       * ftpc_sockinit().  If so... close that socket and call accept to
-       * get a new one.
-       */
-
-      if (sock->sd > 0)
-        {
-          ftpc_sockclose(sock);
-        }
-
-      addrlen  = sizeof(addr);
-      sock->sd = accept(sock->sd, &addr, &addrlen);
-      if (sock->sd == -1)
-        {
-          ndbg("accept() failed: %d\n", errno);
-          return ERROR;
-        }
-
-      memcpy(&sock->laddr, &addr, sizeof(sock->laddr));
-
-      /* Create in/out C buffer I/O streams on the data channel.  First,
-       * create the incoming buffered stream.
-       */
-
-      sock->instream = fdopen(sock->sd, "r");
-      if (!sock->instream)
-        {
-          ndbg("fdopen() failed: %d\n", errno);
-          goto errout_with_sd;
-        }
-
-      /* Create the outgoing stream */
-
-      sock->outstream = fdopen(sock->sd, "w");
-      if (!sock->outstream)
-        {
-          ndbg("fdopen() failed: %d\n", errno);
-          goto errout_with_instream;
-        }
+      ftpc_sockclose(sock);
+    }
+
+  addrlen  = sizeof(addr);
+  sock->sd = accept(sock->sd, &addr, &addrlen);
+  if (sock->sd == -1)
+    {
+      ndbg("accept() failed: %d\n", errno);
+      return ERROR;
     }
-  else
+
+  memcpy(&sock->laddr, &addr, sizeof(sock->laddr));
+
+  /* Create in/out C buffer I/O streams on the data channel.  First,
+   * create the incoming buffered stream.
+   */
+
+  sock->instream = fdopen(sock->sd, "r");
+  if (!sock->instream)
     {
-      /* Should already be set up from ftpc_sockinit() call */
+      ndbg("fdopen() failed: %d\n", errno);
+      goto errout_with_sd;
+    }
+
+  /* Create the outgoing stream */
 
-      DEBUGASSERT(sock->sd >= 0 && sock->instream && sock->outstream);
+  sock->outstream = fdopen(sock->sd, "w");
+  if (!sock->outstream)
+    {
+      ndbg("fdopen() failed: %d\n", errno);
+      goto errout_with_instream;
     }
 
   return OK;