[PATCH] gvfsd-ftp: fix codepath for receiving lines split in chunks.

[Andreas Henriksson <andreas fatal se>]

Hello Benjamin!

Judging from your domain name on irc, it seems you're back from your vacation.
I hope you had a really nice time!
I'm sending this mail to remind you of looking at my patch for bug 525283,
available at http://bugzilla.gnome.org/attachment.cgi?id=126356

Problem description:

In function ftp_connection_receive() there's a number of different code-paths.
You seem to have thought about them all, but made a small mistake in one
of the possible paths. This path is triggered by (broken?) ftp servers
sending small chunks which does not contain a full line.
(pure-ftpd on FreeBSD seems to be a common trigger for many.)

Example:

I'll use the example "foobarquux\n" which is sent in chunks ("foo", "bar" and
"quux\n") by the server and we'll need to call function
soup_socket_read_until() three times to received the full row.
In each iteration of the "while (reply_state != DONE)" the "last_line"
pointer gets updated to point to the place in buffer where we'll insert
the last chunk. When we're finally "DONE", the buffer will contain
"foobarquux\n", but "last_line" will point to "quux\n" rather then the
beginning of the buffer.

Solution:

I've solved this by introducing a "last_chunk" pointer and only updating
the "last_line" pointer whenever the last chunk actually contained a newline.
(This also meant starting out with "got_boundary" set to true. The debug
message also got adjusted to only add the prefix if the last chunk was a
new line.)

This has been tested on two sites (including the original site in the report)
and confirmed to fix the problem.

Thanks:

Wong Yong Lie and Chris Jones for digging into the code and spotting the
problematic area.
Savvas Radević, who reported the bug, provided access to the original
problematic site for testing!



.... on a related note:

Do you mind adding my name to gvfsd-ftp? I've sent a bunch of fixes now
that maybe is enough to be copyright worthy. I don't really care all that
much but it would be nice to get some credit for the work in case anyone
would possibly look at the source some day. :)
I've attached a patch for that as well, for your convenience.... 

Both attachments are also available (for a short time) on:
http://fatal.se/tmp/gvfsd-ftp-chunked-lines.diff
http://fatal.se/tmp/gvfsd-ftp-andreas.diff

-- 
Regards,
Andreas Henriksson

commit becf25e2336c5c0e8b00dee9f06856b076224b03
Author: Andreas Henriksson <andreas fatal se>
Date:   Mon Feb 16 19:38:34 2009 +0100

    gvfsd-ftp: fix codepath for receiving lines split in chunks.
    
    Apparently pure-ftpd on FreeBSD likes to split stuff up in
    silly tiny chunks, making several reads needed to get a single line.
    Fix bug w.r.t. "last_line" pointer in gvfsd-ftp to handle this correctly.

diff --git a/daemon/gvfsbackendftp.c b/daemon/gvfsbackendftp.c
index 8df8efe..dd20153 100644
--- a/daemon/gvfsbackendftp.c
+++ b/daemon/gvfsbackendftp.c
@@ -348,7 +348,7 @@ ftp_connection_receive (FtpConnection *conn,
   SoupSocketIOStatus status;
   gsize n_bytes;
   gboolean got_boundary;
-  char *last_line;
+  char *last_line, *last_chunk;
   enum {
     FIRST_LINE,
     MULTILINE,
@@ -361,6 +361,7 @@ ftp_connection_receive (FtpConnection *conn,
   if (ftp_connection_in_error (conn))
     return 0;
 
+  got_boundary = TRUE;
   conn->read_bytes = 0;
   while (reply_state != DONE)
     {
@@ -381,9 +382,11 @@ ftp_connection_receive (FtpConnection *conn,
 	      return 0;
 	    }
 	}
-      last_line = conn->read_buffer + conn->read_bytes;
+      last_chunk = conn->read_buffer + conn->read_bytes;
+      if (got_boundary)
+        last_line = last_chunk;
       status = soup_socket_read_until (conn->commands,
-				       last_line,
+				       last_chunk,
 				       /* -1 byte for nul-termination */
 				       conn->read_buffer_size - conn->read_bytes - 1,
 				       "\r\n",
@@ -395,7 +398,10 @@ ftp_connection_receive (FtpConnection *conn,
 
       conn->read_bytes += n_bytes;
       conn->read_buffer[conn->read_bytes] = 0;
-      DEBUG ("<-- %s", last_line);
+      if (last_line == last_chunk)
+        DEBUG ("<-- %s", last_line);
+      else
+        DEBUG ("%s", last_chunk);
 
       switch (status)
 	{

commit 245ab63538b6d6536fbf1794bc38fc3b013d2494
Author: Andreas Henriksson <andreas fatal se>
Date:   Mon Feb 16 19:42:43 2009 +0100

    gvfsd-ftp: add copyright notice.

diff --git a/daemon/gvfsbackendftp.c b/daemon/gvfsbackendftp.c
index dd20153..80a5c2f 100644
--- a/daemon/gvfsbackendftp.c
+++ b/daemon/gvfsbackendftp.c
@@ -1,6 +1,7 @@
 /* GIO - GLib Input, Output and Streaming Library
  * 
  * Copyright (C) 2008 Benjamin Otte <otte gnome org>
+ * Copyright (C) 2008,2009 Andreas Henriksson <andreas fatal se>
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -18,6 +19,7 @@
  * Boston, MA 02111-1307, USA.
  *
  * Author: Benjmain Otte <otte gnome org>
+ *         Andreas Henriksson <andreas fatal se>
  */