[PATCH] 1.3: mod_proxy and FTP problems

[Joe Orton <jo...@redhat.com>]

Hi,

In PR 6268 (http://bugs.apache.org/index.cgi/full/6268), Villy Kruse
(vek@pharmapartners.nl) submitted some good fixes for proxying to FTP
servers. There are a couple of other PR's on this, all referenced in 6268.

It would be nice to get these in, since mod_proxy will cause the child
process to hang after accessing many FTP servers at the moment. I've
extracted the least controversial bits of the patch and simplified them
slightly below.

Villy explains the bclose->pclosesocket changes:

 - replace ap_bclose(f) with ap_pclosesocket(p, ap_bfileno(f, B_WR))
   everywhere.
   Reason: ap_bclose does a file close instead of a socket close and
   does not cancel closing the socket later when pool cleanup is done.
   Thus the socket would be closed twice.  Maybe we should not even
   bother closing the control socket and let the general cleanup
   take care of this when the transaction is complete.

Regards,

joe

Index: proxy_ftp.c
===================================================================
RCS file: /home/joe/lib/cvsroot/apache-1.3/src/modules/proxy/proxy_ftp.c,v
retrieving revision 1.84
diff -u -r1.84 proxy_ftp.c
--- proxy_ftp.c	2000/11/14 09:57:16	1.84
+++ proxy_ftp.c	2000/12/22 14:38:16
@@ -810,7 +810,7 @@
     if (dsock == -1) {
 	ap_log_rerror(APLOG_MARK, APLOG_ERR, r,
 		     "proxy: error creating PASV socket");
-	ap_bclose(f);
+	ap_pclosesocket(p, sock);
 	ap_kill_timeout(r);
 	return HTTP_INTERNAL_SERVER_ERROR;
     }
@@ -840,7 +840,7 @@
 	ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, r,
 		     "PASV: control connection is toast");
 	ap_pclosesocket(p, dsock);
-	ap_bclose(f);
+	ap_pclosesocket(p, sock);
 	ap_kill_timeout(r);
 	return HTTP_INTERNAL_SERVER_ERROR;
     }
@@ -895,7 +895,7 @@
 	if (getsockname(sock, (struct sockaddr *) &server, &clen) < 0) {
 	    ap_log_rerror(APLOG_MARK, APLOG_ERR, r,
 			 "proxy: error getting socket address");
-	    ap_bclose(f);
+	    ap_pclosesocket(p, sock);
 	    ap_kill_timeout(r);
 	    return HTTP_INTERNAL_SERVER_ERROR;
 	}
@@ -904,7 +904,7 @@
 	if (dsock == -1) {
 	    ap_log_rerror(APLOG_MARK, APLOG_ERR, r,
 			 "proxy: error creating socket");
-	    ap_bclose(f);
+	    ap_pclosesocket(p, sock);
 	    ap_kill_timeout(r);
 	    return HTTP_INTERNAL_SERVER_ERROR;
 	}
@@ -915,7 +915,7 @@
 	    ap_log_rerror(APLOG_MARK, APLOG_ERR, r,
 			 "proxy: error setting reuseaddr option");
 	    ap_pclosesocket(p, dsock);
-	    ap_bclose(f);
+	    ap_pclosesocket(p, sock);
 	    ap_kill_timeout(r);
 	    return HTTP_INTERNAL_SERVER_ERROR;
 #endif /*_OSD_POSIX*/
@@ -928,7 +928,7 @@
 	    ap_snprintf(buff, sizeof(buff), "%s:%d", inet_ntoa(server.sin_addr), server.sin_port);
 	    ap_log_rerror(APLOG_MARK, APLOG_ERR, r,
 			 "proxy: error binding to ftp data socket %s", buff);
-	    ap_bclose(f);
+	    ap_pclosesocket(p, sock);
 	    ap_pclosesocket(p, dsock);
 	    return HTTP_INTERNAL_SERVER_ERROR;
 	}
@@ -1167,7 +1167,7 @@
 
     if (i != DECLINED) {
 	ap_pclosesocket(p, dsock);
-	ap_bclose(f);
+	ap_pclosesocket(p, sock);
 	return i;
     }
 
@@ -1181,7 +1181,7 @@
 	    ap_log_rerror(APLOG_MARK, APLOG_ERR, r,
 			 "proxy: failed to accept data connection");
 	    ap_pclosesocket(p, dsock);
-	    ap_bclose(f);
+	    ap_pclosesocket(p, sock);
 	    ap_kill_timeout(r);
 	    if (c != NULL)
 		c = ap_proxy_cache_error(c);
@@ -1233,6 +1233,11 @@
 	    ap_proxy_send_fb(data, r, c);
 	} else
 	    send_dir(data, r, c, cwd);
+	
+	/* Must close the data connection here, since some FTP servers
+	 * will wait for the data connection to be closed before
+	 * sending the response down the control connection. */
+	ap_pclosesocket(p, dsock);
 
 	if (rc == 125 || rc == 150)
 	    rc = ftp_getrc(f);
@@ -1246,8 +1251,9 @@
 /* abort the transfer */
 	ap_bputs("ABOR" CRLF, f);
 	ap_bflush(f);
-	if (!pasvmode)
-	    ap_bclose(data);
+	
+	ap_pclosesocket(p, dsock);
+
 	Explain0("FTP: ABOR");
 /* responses: 225, 226, 421, 500, 501, 502 */
     /* 225 Data connection open; no transfer in progress. */
@@ -1273,9 +1279,7 @@
     i = ftp_getrc(f);
     Explain1("FTP: QUIT: status %d", i);
 
-    if (pasvmode)
-	ap_bclose(data);
-    ap_bclose(f);
+    ap_pclosesocket(p, sock);
 
     ap_rflush(r);	/* flush before garbage collection */