- Introduced a SYST-based test to properly set-up name format when dealing with the OS/400 FTP server.

- Fixed an ftp_readresp() bug preventing detection of failing control socket  and causing FTP client to loop forever.

6 files changed, 195 insertions, 6 deletions

diff --git a/CHANGES b/CHANGES
index 1bd2c223b..ef5896f20 100644
--- a/CHANGES
+++ b/CHANGES
@@ -6,6 +6,13 @@
 
                                   Changelog
 
+Patrick Monnerat (24 Aug 2009)
+- Introduced a SYST-based test to properly set-up name format when dealing
+  with the OS/400 FTP server.
+
+- Fixed an ftp_readresp() bug preventing detection of failing control socket
+  and causing FTP client to loop forever.
+
 Daniel Stenberg (24 Aug 2009)
 - Marc de Bruin pointed out that configure --with-gnutls=PATH didn't work
   properly and provided a fix. http://curl.haxx.se/bug/view.cgi?id=2843008
diff --git a/lib/ftp.c b/lib/ftp.c
index b68cc027c..26cf19795 100644
--- a/lib/ftp.c
+++ b/lib/ftp.c
@@ -439,13 +439,15 @@ static CURLcode ftp_readresp(curl_socket_t sockfd,
 #ifdef CURL_DOES_CONVERSIONS
       if((res == CURLE_OK) && (gotbytes > 0)) {
         /* convert from the network encoding */
-        result = res = Curl_convert_from_network(data, ptr, gotbytes);
+        res = Curl_convert_from_network(data, ptr, gotbytes);
         /* Curl_convert_from_network calls failf if unsuccessful */
       }
 #endif /* CURL_DOES_CONVERSIONS */
 
-      if(CURLE_OK != res)
+      if(CURLE_OK != res) {
+        result = res;	/* Set the outer result variable to this error. */
         keepon = FALSE;
+      }
     }
 
     if(!keepon)
@@ -742,6 +744,8 @@ static void state(struct connectdata *conn,
     "PROT",
     "CCC",
     "PWD",
+    "SYST",
+    "NAMEFMT",
     "QUOTE",
     "RETR_PREQUOTE",
     "STOR_PREQUOTE",
@@ -2733,10 +2737,11 @@ static CURLcode ftp_statemach_act(struct connectdata *conn)
 
     case FTP_PWD:
       if(ftpcode == 257) {
-        char *dir = malloc(nread+1);
-        char *store=dir;
         char *ptr=&data->state.buffer[4];  /* start on the first letter */
+        char *dir;
+        char *store;
 
+        dir = malloc(nread + 1);
         if(!dir)
           return CURLE_OUT_OF_MEMORY;
 
@@ -2751,7 +2756,7 @@ static CURLcode ftp_statemach_act(struct connectdata *conn)
         if('\"' == *ptr) {
           /* it started good */
           ptr++;
-          while(ptr && *ptr) {
+          for (store = dir; *ptr;) {
             if('\"' == *ptr) {
               if('\"' == ptr[1]) {
                 /* "quote-doubling" */
@@ -2769,10 +2774,30 @@ static CURLcode ftp_statemach_act(struct connectdata *conn)
             store++;
             ptr++;
           }
+          if(ftpc->entrypath)
+            free(ftpc->entrypath);
           ftpc->entrypath =dir; /* remember this */
           infof(data, "Entry path is '%s'\n", ftpc->entrypath);
           /* also save it where getinfo can access it: */
           data->state.most_recent_ftp_entrypath = ftpc->entrypath;
+
+          /* If the path name does not look like an absolute path (i.e.: it
+             does not start with a '/'), we probably need some server-dependent
+             adjustments. For example, this is the case when connecting to
+             an OS400 FTP server: this server supports two name syntaxes,
+             the default one being incompatible with standard pathes. In
+             addition, this server switches automatically to the regular path
+             syntax when one is encountered in a command: this results in
+             having an entrypath in the wrong syntax when later used in CWD.
+               The method used here is to check the server OS: we do it only
+             if the path name looks strange to minimize overhead on other
+             systems. */
+
+          if(!ftpc->server_os && ftpc->entrypath[0] != '/') {
+            NBFTPSENDF(conn, "SYST", NULL);
+            state(conn, FTP_SYST);
+            break;
+          }
         }
         else {
           /* couldn't get the path */
@@ -2784,6 +2809,57 @@ static CURLcode ftp_statemach_act(struct connectdata *conn)
       DEBUGF(infof(data, "protocol connect phase DONE\n"));
       break;
 
+    case FTP_SYST:
+      if(ftpcode == 215) {
+        char *ptr=&data->state.buffer[4];  /* start on the first letter */
+        char *os;
+        char *store;
+
+        os = malloc(nread + 1);
+        if(!os)
+          return CURLE_OUT_OF_MEMORY;
+
+        /* Reply format is like
+           215<space><OS-name><space><commentary>
+        */
+        while (*ptr == ' ')
+          ptr++;
+        for (store = os; *ptr && *ptr != ' ';)
+          *store++ = *ptr++;
+        *store = '\0'; /* zero terminate */
+        ftpc->server_os = os;
+
+        /* Check for special servers here. */
+
+        if(strequal(ftpc->server_os, "OS/400")) {
+          /* Force OS400 name format 1. */
+          NBFTPSENDF(conn, "SITE NAMEFMT 1", NULL);
+          state(conn, FTP_NAMEFMT);
+          break;
+        }
+      else {
+        /* Nothing special for the target server. */
+       }
+      }
+      else {
+        /* Cannot identify server OS. Continue anyway and cross fingers. */
+      }
+
+      state(conn, FTP_STOP); /* we are done with the CONNECT phase! */
+      DEBUGF(infof(data, "protocol connect phase DONE\n"));
+      break;
+
+    case FTP_NAMEFMT:
+      if(ftpcode == 250) {
+        /* Name format change successful: reload initial path. */
+        ftp_state_pwd(conn);
+        break;
+      }
+
+      state(conn, FTP_STOP); /* we are done with the CONNECT phase! */
+      DEBUGF(infof(data, "protocol connect phase DONE\n"));
+      break;
+
     case FTP_QUOTE:
     case FTP_POSTQUOTE:
     case FTP_RETR_PREQUOTE:
@@ -3843,6 +3919,10 @@ static CURLcode ftp_disconnect(struct connectdata *conn)
     free(ftpc->prevpath);
     ftpc->prevpath = NULL;
   }
+  if(ftpc->server_os) {
+    free(ftpc->server_os);
+    ftpc->server_os = NULL;
+  }
 
   return CURLE_OK;
 }
diff --git a/lib/urldata.h b/lib/urldata.h
index 1ee6637d2..bc2eff960 100644
--- a/lib/urldata.h
+++ b/lib/urldata.h
@@ -362,6 +362,8 @@ typedef enum {
   FTP_PROT,
   FTP_CCC,
   FTP_PWD,
+  FTP_SYST,
+  FTP_NAMEFMT,
   FTP_QUOTE, /* waiting for a response to a command sent in a quote list */
   FTP_RETR_PREQUOTE,
   FTP_STOR_PREQUOTE,
@@ -458,6 +460,7 @@ struct ftp_conn {
   struct timeval response; /* set to Curl_tvnow() when a command has been sent
                               off, used to time-out response reading */
   ftpstate state; /* always use ftp.c:state() to change state! */
+  char * server_os;     /* The target server operating system. */
 };
 
 /****************************************************************************
diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index 72afc5128..8e5b4c727 100644
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -62,7 +62,7 @@ EXTRA_DIST = test1 test108 test117 test127 test20 test27 test34 test46	   \
  test635 test636 test637 test558 test559 test1086 test1087 test1088        \
  test1089 test1090 test1091 test1092 test1093 test1094 test1095 test1096   \
  test1097 test560 test561 test1098 test1099 test562 test563 test1100       \
- test564 test1101
+ test564 test1101 test1102 test1103
 
 filecheck:
 	@mkdir test-place; \
diff --git a/tests/data/test1102 b/tests/data/test1102
new file mode 100644
index 000000000..addc7b572
--- /dev/null
+++ b/tests/data/test1102
@@ -0,0 +1,51 @@
+<testcase>
+<info>
+<keywords>
+FTP
+SYST
+SITE
+OS400
+</keywords>
+</info>
+# Server-side
+<reply>
+<data nocheck="yes">
+blabla
+</data>
+<servercmd>
+REPLY PWD 257 "QGPL" is the current library
+REPLY SYST 215  OS/400 runs this server
+REPLY SITE 250 Name format set to 1
+</servercmd>
+</reply>
+
+# Client-side
+<client>
+<server>
+ftp
+</server>
+ <name>
+FTP OS/400 server name format check
+ </name>
+ <command>
+ftp://%HOSTIP:%FTPPORT/1102
+</command>
+</client>
+
+# Verify data after the test has been "shot"
+<verify>
+<protocol>
+USER anonymous

+PASS ftp@example.com

+PWD

+SYST

+SITE NAMEFMT 1

+PWD

+EPSV

+TYPE I

+SIZE 1102

+RETR 1102

+QUIT

+</protocol>
+</verify>
+</testcase>
diff --git a/tests/data/test1103 b/tests/data/test1103
new file mode 100644
index 000000000..4d45056d5
--- /dev/null
+++ b/tests/data/test1103
@@ -0,0 +1,48 @@
+<testcase>
+<info>
+<keywords>
+FTP
+SYST
+SITE
+OS400
+</keywords>
+</info>
+# Server-side
+<reply>
+<data nocheck="yes">
+blabla
+</data>
+<servercmd>
+REPLY PWD 257 "C:/somedir" is the current directory
+REPLY SYST 215  unknown-OS runs this server
+</servercmd>
+</reply>
+
+# Client-side
+<client>
+<server>
+ftp
+</server>
+ <name>
+FTP non-OS/400 server
+ </name>
+ <command>
+ftp://%HOSTIP:%FTPPORT/1103
+</command>
+</client>
+
+# Verify data after the test has been "shot"
+<verify>
+<protocol>
+USER anonymous

+PASS ftp@example.com

+PWD

+SYST

+EPSV

+TYPE I

+SIZE 1103

+RETR 1103

+QUIT

+</protocol>
+</verify>
+</testcase>