On Thursday 04 November 2010 21:44:11 Daniel Stenberg wrote:
> Yes: The original bug report was about a server getting "stuck" if we
> didn't send ABOR, so theoretically it still hangs on the connection if we
> just close it. So sending ABOR is the nice thing to do to the server.
>
> No: That occasional hang will not bother us somce we just proceed and close
> down the entire thing anyway without trying to read anything further from
> the command connection.
>
> As we now send ABOR my only concern is if there's another case that we
> haven't yet thought of or ran into that makes sending it a bad idea, as
> otherwise I think sending it makes sense.

Daniel, would the attached patch be acceptable from your point of view?

Simon, I'll prepare a new updates image for Anaconda if you are willing
to test it.

Kamil
From 4123e05d41234203826cee0ebd3070718a4b9c0c Mon Sep 17 00:00:00 2001
From: Kamil Dudka <[email protected]>
Date: Thu, 4 Nov 2010 21:44:02 +0100
Subject: [PATCH] ftp: close connection as soon as ABOR has been sent

... and do not send ABOR unless really necessary.

Bug: https://bugzilla.redhat.com/649347
Reported by: Simon H.
---
 lib/ftp.c           |   14 ++++++++++----
 tests/data/test1036 |    1 -
 tests/data/test1057 |    1 -
 tests/data/test110  |    1 -
 tests/data/test122  |    1 -
 tests/data/test135  |    1 -
 6 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/lib/ftp.c b/lib/ftp.c
index 679f233..82c1e6e 100644
--- a/lib/ftp.c
+++ b/lib/ftp.c
@@ -3083,10 +3083,9 @@ static CURLcode ftp_done(struct connectdata *conn, CURLcode status,
 #endif
 
   if(conn->sock[SECONDARYSOCKET] != CURL_SOCKET_BAD) {
-    if(!result && ftpc->dont_check)
-      /* prevent some FTP servers (namely Pure-ftpd) from hanging if we close
-       * the data channel before transferring all data */
-      result = Curl_pp_sendf(&ftpc->pp, "ABOR");
+    if(!result && ftpc->dont_check && data->req.maxdownload > 0)
+      /* partial download completed */
+      result = Curl_pp_sendf(pp, "ABOR");
 
     if(conn->ssl[SECONDARYSOCKET].use) {
       /* The secondary socket is using SSL so we must close down that part
@@ -3128,6 +3127,13 @@ static CURLcode ftp_done(struct connectdata *conn, CURLcode status,
     if(result)
       return result;
 
+    if(ftpc->dont_check && data->req.maxdownload > 0) {
+      infof(data, "partial download completed, closing connection\n");
+      ftpc->ctl_valid = FALSE; /* mark control connection as bad */
+      conn->bits.close = TRUE; /* mark for closure */
+      return result;
+    }
+
     if(!ftpc->dont_check) {
       /* 226 Transfer complete, 250 Requested file action okay, completed. */
       if((ftpcode != 226) && (ftpcode != 250)) {
diff --git a/tests/data/test1036 b/tests/data/test1036
index a31cb45..b8ebc4f 100644
--- a/tests/data/test1036
+++ b/tests/data/test1036
@@ -50,7 +50,6 @@ TYPE I
 SIZE 1036
 REST 20
 RETR 1036
-ABOR
 QUIT
 </protocol>
 <file name="log/curl1036.out">
diff --git a/tests/data/test1057 b/tests/data/test1057
index b4ef20c..aa594c4 100644
--- a/tests/data/test1057
+++ b/tests/data/test1057
@@ -46,7 +46,6 @@ SIZE 1057
 REST 52
 RETR 1057
 ABOR
-QUIT
 </protocol>
 </verify>
 </testcase>
diff --git a/tests/data/test110 b/tests/data/test110
index 4a094e7..b63ba8a 100644
--- a/tests/data/test110
+++ b/tests/data/test110
@@ -46,7 +46,6 @@ TYPE I
 SIZE 110
 REST 20
 RETR 110
-ABOR
 QUIT
 </protocol>
 </verify>
diff --git a/tests/data/test122 b/tests/data/test122
index ac60672..fb1dd05 100644
--- a/tests/data/test122
+++ b/tests/data/test122
@@ -39,7 +39,6 @@ EPSV
 PASV
 TYPE I
 SIZE 122
-ABOR
 QUIT
 </protocol>
 </verify>
diff --git a/tests/data/test135 b/tests/data/test135
index 10eb0ea..b943e4c 100644
--- a/tests/data/test135
+++ b/tests/data/test135
@@ -48,7 +48,6 @@ SIZE 135
 REST 4
 RETR 135
 ABOR
-QUIT
 </protocol>
 </verify>
 </testcase>
-- 
1.7.2.3

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette:  http://curl.haxx.se/mail/etiquette.html

Reply via email to