httpd-cvs mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From wr...@apache.org
Subject svn commit: r794522 - in /httpd/mod_ftp/trunk: CHANGES-FTP include/mod_ftp.h modules/ftp/ftp_commands.c modules/ftp/ftp_protocol.c
Date Thu, 16 Jul 2009 04:43:03 GMT
Author: wrowe
Date: Thu Jul 16 04:43:02 2009
New Revision: 794522

URL: http://svn.apache.org/viewvc?rev=794522&view=rev
Log:
Conformance patch to RFC 2640 with respect to leading and trailing
whitespace, embedded <IAC> or <CR><NUL> sequences per the application
telnet specification referenced by RFC 959 and RFC 1123.


Modified:
    httpd/mod_ftp/trunk/CHANGES-FTP
    httpd/mod_ftp/trunk/include/mod_ftp.h
    httpd/mod_ftp/trunk/modules/ftp/ftp_commands.c
    httpd/mod_ftp/trunk/modules/ftp/ftp_protocol.c

Modified: httpd/mod_ftp/trunk/CHANGES-FTP
URL: http://svn.apache.org/viewvc/httpd/mod_ftp/trunk/CHANGES-FTP?rev=794522&r1=794521&r2=794522&view=diff
==============================================================================
--- httpd/mod_ftp/trunk/CHANGES-FTP (original)
+++ httpd/mod_ftp/trunk/CHANGES-FTP Thu Jul 16 04:43:02 2009
@@ -1,5 +1,9 @@
 Changes in 0.9.5:
 
+  *) Correct RFC2640 conformance with respect to filenames containing
+     leading and/or trailing whitespace.  This introduces a new cmd flag,
+     FTP_KEEP_WHITESPACE (aliased as FTP_TAKE1_PATH).  [William Rowe]
+
   *) Accept lowercase TYPE arguments.  [William Rowe]
 
   *) Implement MODE S, STRU F and TYPE L 8 (treated as I) and TYPE A N

Modified: httpd/mod_ftp/trunk/include/mod_ftp.h
URL: http://svn.apache.org/viewvc/httpd/mod_ftp/trunk/include/mod_ftp.h?rev=794522&r1=794521&r2=794522&view=diff
==============================================================================
--- httpd/mod_ftp/trunk/include/mod_ftp.h (original)
+++ httpd/mod_ftp/trunk/include/mod_ftp.h Thu Jul 16 04:43:02 2009
@@ -209,6 +209,7 @@
  *
  * FTP_TAKE0 - This command takes no arguments.
  * FTP_TAKE1 - This command takes a single argument.
+ * FTP_KEEP_WHITESPACE  - leading and trailing spaces are significant.
  * FTP_NEED_LOGIN - The user needs to be logged in to execute this command.
  * FTP_DATA_INTR - The current data transfer is interrupted by this command.
  * FTP_NEW_FEAT - This command was introduced following RFC 2389, show in FEAT list.
@@ -217,6 +218,8 @@
  *               a given FTP command from the HELP listing.
  * FTP_EXTENSIBLE - Upon all cmd handlers declining, reply is COMMAND_NOT_IMPL_PARAM
  *                  rather than COMMAND_NOT_IMPLEMENTED
+ * FTP_TAKE1_PATH - This command takes a single argument, delimited by a single
+ *                  space, with leading and trailing spaces significant.
  */
 #define FTP_TAKE0            (1 << 0)
 #define FTP_TAKE1            (1 << 1)
@@ -225,6 +228,9 @@
 #define FTP_NEW_FEAT         (1 << 7)
 #define FTP_NO_HELP          (1 << 8)
 #define FTP_EXTENSIBLE       (1 << 9)
+#define FTP_KEEP_WHITESPACE  (1 << 10)
+
+#define FTP_TAKE1_PATH       (FTP_TAKE1 | FTP_KEEP_WHITESPACE)
 
 /* FTP command handler ordering */
 #define FTP_HOOK_FIRST  10

Modified: httpd/mod_ftp/trunk/modules/ftp/ftp_commands.c
URL: http://svn.apache.org/viewvc/httpd/mod_ftp/trunk/modules/ftp/ftp_commands.c?rev=794522&r1=794521&r2=794522&view=diff
==============================================================================
--- httpd/mod_ftp/trunk/modules/ftp/ftp_commands.c (original)
+++ httpd/mod_ftp/trunk/modules/ftp/ftp_commands.c Thu Jul 16 04:43:02 2009
@@ -261,12 +261,29 @@
  * Returns: 1 on error or 0 on success, a1 and a2 are modified to point
  *          to the correct values.
  */
-static int ftp_parse2(apr_pool_t *pool, const char *cmd, char **a1, char **a2)
+static int ftp_parse2(apr_pool_t *pool, const char *cmd,
+                      char **a1, char **a2, int keepws)
 {
-    *a1 = ap_getword_white(pool, &cmd);
-    *a2 = apr_pstrdup(pool, cmd);
-    if (!*a1 || !*a2) {
-        return 1;
+    if (keepws)
+    {
+        const char *save = cmd;
+        while (*cmd && *cmd != ' ') ++cmd;
+        *a1 = apr_pstrndup(pool, save, cmd - save);
+        if (*cmd && *cmd == ' ') ++cmd;
+        *a2 = apr_pstrdup(pool, cmd);
+        if (!*a1 || !*a2)
+            return 1;
+    }
+    else
+    {
+        char *fix;
+        *a1 = ap_getword(pool, &cmd, ' ');
+        *a2 = apr_pstrdup(pool, cmd);
+        if (!*a1 || !*a2)
+            return 1;
+        fix = strchr(*a2, '\0');
+        while (fix > *a2 && *(fix - 1) == ' ')
+            *(--fix) = '\0';
     }
     return 0;
 }
@@ -293,7 +310,7 @@
          * Note: recursive aliases are unsupported
          */
         if (cmd->pf == NULL) {
-            return (cmd->next->flags & FTP_EXTENSIBLE)
+            return (cmd->flags & FTP_EXTENSIBLE)
                     ? FTP_REPLY_COMMAND_NOT_IMPL_PARAM
                     : FTP_REPLY_COMMAND_NOT_IMPLEMENTED;
         }
@@ -305,9 +322,10 @@
         }
         else {
             res = ftp_parse2(r->pool, r->the_request,
-                             &method, &arg);
-            if ((!(cmd->flags & FTP_TAKE0) && !arg) || res)
-                return FTP_REPLY_LOCAL_ERROR;
+                             &method, &arg, cmd->flags & FTP_KEEP_WHITESPACE);
+            if (res || (!(cmd->flags & FTP_TAKE0) && !*arg)
+                    || (!(cmd->flags & FTP_TAKE1) && *arg))
+                return FTP_REPLY_SYNTAX_ERROR;
 
             return ftp_run_handler(r, cmd, arg);
         }
@@ -2914,11 +2932,11 @@
                  "(specify account)");
 
     ftp_hook_cmd("ALLO", NULL, FTP_HOOK_LAST,
-                 FTP_NEED_LOGIN | FTP_TAKE1,
+                 FTP_NEED_LOGIN | FTP_TAKE1_PATH,
                  "allocate storage (vacuously)");
 
     ftp_hook_cmd("APPE", ftp_cmd_stor, FTP_HOOK_LAST,
-                 FTP_NEED_LOGIN | FTP_TAKE1,
+                 FTP_NEED_LOGIN | FTP_TAKE1_PATH,
                  "<sp> file-name");
 
     /* AUTH does not require a user to be logged in */
@@ -2934,11 +2952,11 @@
                  "change to parent directory");
 
     ftp_hook_cmd("CWD", ftp_cmd_cwd, FTP_HOOK_LAST,
-                 FTP_NEED_LOGIN | FTP_TAKE1,
+                 FTP_NEED_LOGIN | FTP_TAKE1_PATH,
                  "[ <sp> directory-name ]");
 
     ftp_hook_cmd("DELE", ftp_cmd_dele, FTP_HOOK_LAST,
-                 FTP_NEED_LOGIN | FTP_TAKE1,
+                 FTP_NEED_LOGIN | FTP_TAKE1_PATH,
                  "<sp> file-name");
 
     ftp_hook_cmd("EPRT", ftp_cmd_eprt, FTP_HOOK_LAST,
@@ -2962,15 +2980,15 @@
                  "<sp> lang-code");
 
     ftp_hook_cmd("LIST", ftp_cmd_list, FTP_HOOK_LAST,
-                 FTP_NEED_LOGIN | FTP_TAKE0 | FTP_TAKE1,
+                 FTP_NEED_LOGIN | FTP_TAKE0 | FTP_TAKE1_PATH,
                  "[ <sp> path-name ]");
 
     ftp_hook_cmd("MDTM", ftp_cmd_mdtm, FTP_HOOK_LAST,
-                 FTP_NEED_LOGIN | FTP_TAKE1 | FTP_NEW_FEAT,
+                 FTP_NEED_LOGIN | FTP_TAKE1_PATH | FTP_NEW_FEAT,
                  "<sp> path-name");
 
     ftp_hook_cmd("MKD", ftp_cmd_mkd, FTP_HOOK_LAST,
-                 FTP_NEED_LOGIN | FTP_TAKE1,
+                 FTP_NEED_LOGIN | FTP_TAKE1_PATH,
                  "<sp> path-name");
 
     ftp_hook_cmd("MODE", ftp_cmd_mode, FTP_HOOK_LAST,
@@ -2978,7 +2996,7 @@
                  "<sp> [ S | B | C ] (specify transfer mode)");
 
     ftp_hook_cmd("NLST", ftp_cmd_nlst, FTP_HOOK_LAST,
-                 FTP_NEED_LOGIN | FTP_TAKE0 | FTP_TAKE1,
+                 FTP_NEED_LOGIN | FTP_TAKE0 | FTP_TAKE1_PATH,
                  "[ <sp> path-name ]");
 
     ftp_hook_cmd("NOOP", ftp_cmd_noop, FTP_HOOK_LAST,
@@ -3030,19 +3048,19 @@
     ftp_feat_advert("REST STREAM");
 
     ftp_hook_cmd("RETR", ftp_cmd_retr, FTP_HOOK_LAST,
-                 FTP_NEED_LOGIN | FTP_TAKE1,
+                 FTP_NEED_LOGIN | FTP_TAKE1_PATH,
                  "<sp> file-name");
 
     ftp_hook_cmd("RMD", ftp_cmd_rmd, FTP_HOOK_LAST,
-                 FTP_NEED_LOGIN | FTP_TAKE1,
+                 FTP_NEED_LOGIN | FTP_TAKE1_PATH,
                  "<sp> path-name");
 
     ftp_hook_cmd("RNFR", ftp_cmd_rnfr, FTP_HOOK_LAST,
-                 FTP_NEED_LOGIN | FTP_TAKE1,
+                 FTP_NEED_LOGIN | FTP_TAKE1_PATH,
                  "<sp> file-name");
 
     ftp_hook_cmd("RNTO", ftp_cmd_rnto, FTP_HOOK_LAST,
-                 FTP_NEED_LOGIN | FTP_TAKE1,
+                 FTP_NEED_LOGIN | FTP_TAKE1_PATH,
                  "<sp> file-name");
 
     ftp_hook_cmd("SITE", NULL, FTP_HOOK_LAST,
@@ -3050,7 +3068,7 @@
                  "<sp> site-cmd [ <sp> arguments ]");
 
     ftp_hook_cmd("SIZE", ftp_cmd_size, FTP_HOOK_LAST,
-                 FTP_NEED_LOGIN | FTP_TAKE1 | FTP_NEW_FEAT,
+                 FTP_NEED_LOGIN | FTP_TAKE1_PATH | FTP_NEW_FEAT,
                  "<sp> path-name");
 
     ftp_hook_cmd("SMNT", NULL, FTP_HOOK_LAST,
@@ -3058,15 +3076,15 @@
                  "(structure mount)");
 
     ftp_hook_cmd("STAT", NULL, FTP_HOOK_LAST,
-                 FTP_NEED_LOGIN | FTP_TAKE1,
+                 FTP_NEED_LOGIN | FTP_TAKE1_PATH,
                  "[ <sp> path-name ]");
 
     ftp_hook_cmd("STOR", ftp_cmd_stor, FTP_HOOK_LAST,
-                 FTP_NEED_LOGIN | FTP_TAKE1,
+                 FTP_NEED_LOGIN | FTP_TAKE1_PATH,
                  "<sp> file-name");
 
     ftp_hook_cmd("STOU", NULL, FTP_HOOK_LAST,
-                 FTP_NEED_LOGIN | FTP_TAKE1,
+                 FTP_NEED_LOGIN | FTP_TAKE1_PATH,
                  "<sp> file-name");
 
     ftp_hook_cmd("STRU", ftp_cmd_stru, FTP_HOOK_LAST,
@@ -3077,6 +3095,12 @@
                  FTP_NEED_LOGIN | FTP_TAKE0,
                  "(get type of operating system)");
 
+    /* RFC3659 TVFS advertising simply insists that '/' is not a filename
+     * character, but a path delimiter in a tree structured filesystem.
+     * That's us, advertise it;
+     */
+    ftp_feat_advert("TVFS");
+
     ftp_hook_cmd("TYPE", ftp_cmd_type, FTP_HOOK_LAST,
                  FTP_NEED_LOGIN | FTP_TAKE1,
                  "<sp> [ A [ fmt ] | E [ fmt ] | I | L size ]");

Modified: httpd/mod_ftp/trunk/modules/ftp/ftp_protocol.c
URL: http://svn.apache.org/viewvc/httpd/mod_ftp/trunk/modules/ftp/ftp_protocol.c?rev=794522&r1=794521&r2=794522&view=diff
==============================================================================
--- httpd/mod_ftp/trunk/modules/ftp/ftp_protocol.c (original)
+++ httpd/mod_ftp/trunk/modules/ftp/ftp_protocol.c Thu Jul 16 04:43:02 2009
@@ -212,24 +212,57 @@
             }
 
             /* If we got a full line of input, stop reading */
-            if (last_char && (*last_char == APR_ASCII_LF)) {
+            if ((last_char > pbuf) && (*(last_char - 1) == APR_ASCII_CR)
+                                   && (*last_char == APR_ASCII_LF))
+            {
+                char *ssrc, *sdst;
 
-                /* Trim the trailing spaces or tabs, LF and CR */
-                while ((last_char > pbuf)
-                       && apr_isspace(*(last_char - 1))) {
-                    --last_char;
-                }
+                /* Since we want to remove the CRLF from the line, we'll go
+                 * ahead and NULL term the string;
+                 */
+                *(--last_char) = '\0';
 
                 /*
-                 * Since we want to remove the LF from the line, we'll go
-                 * ahead and set this last character to be the term NULL
+                 * We ignore claims in draft-ietf-ftpext-utf-8-option-00 which
+                 * suggested RFC2640 is wrong with respect to RFC1123.  RFC1123
+                 * is quite clear in stating;
+                 *
+                 *   The Telnet end-of-line sequence CR LF MUST be used to send
+                 *   Telnet data that is not terminal-to-computer (e.g., for Server
+                 *   Telnet sending output, or the Telnet protocol incorporated
+                 *   another application protocol)
+                 *
+                 * ergo CR NUL is not a valid command completion sequence, as
+                 * FTP is not a terminal protocol, but an application protocol.
+                 *
+                 * Collapse <CR><NULL> to <CR> and <IAC><IAC>
to a single 0xFF
+                 * as documented in RFC854 and clarified by RFC2640 and RFC3659,
+                 * discarding all extranious <IAC> sequences.  As negotiation
+                 * is explicitly not allowed, we make no effort to catch such.
+                 */
+                for (ssrc = sdst = pbuf; ssrc < last_char; ++ssrc, ++sdst) {
+                    if ((ssrc[0] == 0xff)  
+                            || (ssrc[0] == APR_ASCII_CR && ssrc[1] == 0)) {
+                        if (ssrc[0] == 0xff && ssrc[1] != 0xff)
+                            --sdst;
+                        break;
+                    }
+                }
+                /* Jumping from the parse-only loop above, into this parse w/copy
                  */
-                *last_char = '\0';
+                for (ssrc += 2, ++sdst; ssrc < last_char; ++ssrc) {
+                    *(sdst++) = *ssrc;
+                    if ((ssrc[0] == 0xff)  
+                            || (ssrc[0] == APR_ASCII_CR && ssrc[1] == 0)) {
+                        ++ssrc;
+                        if (ssrc[0] == 0xff && ssrc[1] != 0xff)
+                            --sdst;
+                    }
+                }
 
                 /*
                  * Return the result string, and the actual bytes read from
-                 * the network (before we truncated trailing whitespace and
-                 * newlines)
+                 * the network (before we truncated characters)
                  * 
                  * We may have moved from a pool to another pool, or to a heap
                  * bucket.  Reallocate from the current pool in these cases.
@@ -362,32 +395,6 @@
     apr_brigade_destroy(fc->next_bb);
     fc->next_bb = NULL;
 
-    ll = fc->next_request;
-
-#ifdef FTP_TRACE
-    ap_log_error(APLOG_MARK, APLOG_NOERRNO | APLOG_DEBUG, 0,
-                 fc->orig_server, "FTP frar: raw command: %s", ll);
-#endif
-
-    /*
-     * FIXME: For some reason we miss seeing the OOB mark,
-     * at which point the various telnet sequences are
-     * still in the command stream. We assume that we
-     * should strip out anything before, and including,
-     * the DM (value xf2). Because of strangeness with
-     * how some clients do this, we also strip out
-     * the IAC (xff) and IP (xf4) chars.
-     */
-    if ((method = ap_strrchr_c(ll, '\xff')))
-        ll = ++method;
-    if ((method = ap_strrchr_c(ll, '\xf4')))
-        ll = ++method;
-    if ((method = ap_strrchr_c(ll, '\xf2')))
-        ll = ++method;
-
-    /* XXX gross hack till we straighten out constness */
-    fc->next_request = (char *) ll;     /* store away "cleaned" command */
-
     method = ftp_toupper(fc->next_pool, ap_getword_white(fc->next_pool, &ll));
 
 #ifdef FTP_TRACE



Mime
View raw message