+++ /dev/null
-------------------------------------------------------------
-revno: 14115
-revision-id: squid3@treenet.co.nz-20161130215630-c42qucqar9bi9a1k
-parent: squid3@treenet.co.nz-20161130154205-c9z1bhqzuh3rafl3
-fixes bug: http://bugs.squid-cache.org/show_bug.cgi?id=4004
-author: Christos Tsantilas <chtsanti@users.sourceforge.net>
-committer: Amos Jeffries <squid3@treenet.co.nz>
-branch nick: 3.5
-timestamp: Thu 2016-12-01 10:56:30 +1300
-message:
- Bug 4004 partial: Fix segfault via Ftp::Client::readControlReply
-
- Added nil dereference checks for Ftp::Client::ctrl.conn, including:
- - Ftp::Client::handlePasvReply() and handleEpsvReply() that dereference
- ctrl.conn in DBG_IMPORTANT messages.
- - Many functions inside FtpClient.cc and FtpGateway.cc files.
-
- TODO: We need to find a better way to handle nil ctrl.conn. It is only
- a matter of time when we forget to add another dereference check or
- discover a place we missed during this change.
-
- Also disabled forwarding of EPRT and PORT commands to origin servers.
- Squid support for those commands is broken and their forwarding may
- cause segfaults (bug #4004). Active FTP is still supported, of course.
-
- This is a Measurement Factory project
-------------------------------------------------------------
-# Bazaar merge directive format 2 (Bazaar 0.90)
-# revision_id: squid3@treenet.co.nz-20161130215630-c42qucqar9bi9a1k
-# target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
-# testament_sha1: 345883c1b5a5cd221e9d0e68b254df7d955372ad
-# timestamp: 2016-11-30 22:42:02 +0000
-# source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
-# base_revision_id: squid3@treenet.co.nz-20161130154205-\
-# c9z1bhqzuh3rafl3
-#
-# Begin patch
-=== modified file 'src/clients/FtpClient.cc'
---- src/clients/FtpClient.cc 2016-08-05 14:59:33 +0000
-+++ src/clients/FtpClient.cc 2016-11-30 21:56:30 +0000
-@@ -442,6 +442,11 @@
- char *buf;
- debugs(9, 3, status());
-
-+ if (!Comm::IsConnOpen(ctrl.conn)) {
-+ debugs(9, 5, "The control connection to the remote end is closed");
-+ return false;
-+ }
-+
- if (code != 227) {
- debugs(9, 2, "PASV not supported by remote end");
- return false;
-@@ -473,6 +478,11 @@
- char *buf;
- debugs(9, 3, status());
-
-+ if (!Comm::IsConnOpen(ctrl.conn)) {
-+ debugs(9, 5, "The control connection to the remote end is closed");
-+ return false;
-+ }
-+
- if (code != 229 && code != 522) {
- if (code == 200) {
- /* handle broken servers (RFC 2428 says OK code for EPSV MUST be 229 not 200) */
-@@ -733,6 +743,11 @@
- void
- Ftp::Client::connectDataChannel()
- {
-+ if (!Comm::IsConnOpen(ctrl.conn)) {
-+ debugs(9, 5, "The control connection to the remote end is closed");
-+ return;
-+ }
-+
- safe_free(ctrl.last_command);
-
- safe_free(ctrl.last_reply);
-
-=== modified file 'src/clients/FtpGateway.cc'
---- src/clients/FtpGateway.cc 2016-01-31 05:39:09 +0000
-+++ src/clients/FtpGateway.cc 2016-11-30 21:56:30 +0000
-@@ -212,7 +212,9 @@
- static FTPSM ftpReadMdtm;
- static FTPSM ftpSendSize;
- static FTPSM ftpReadSize;
-+#if 0
- static FTPSM ftpSendEPRT;
-+#endif
- static FTPSM ftpReadEPRT;
- static FTPSM ftpSendPORT;
- static FTPSM ftpReadPORT;
-@@ -450,6 +452,11 @@
- void
- Ftp::Gateway::listenForDataChannel(const Comm::ConnectionPointer &conn)
- {
-+ if (!Comm::IsConnOpen(ctrl.conn)) {
-+ debugs(9, 5, "The control connection to the remote end is closed");
-+ return;
-+ }
-+
- assert(!Comm::IsConnOpen(data.conn));
-
- typedef CommCbMemFunT<Gateway, CommAcceptCbParams> AcceptDialer;
-@@ -1183,7 +1190,7 @@
-
- checkUrlpath();
- buildTitleUrl();
-- debugs(9, 5, HERE << "FD " << ctrl.conn->fd << " : host=" << request->GetHost() <<
-+ debugs(9, 5, "FD " << (ctrl.conn != NULL ? ctrl.conn->fd : -1) << " : host=" << request->GetHost() <<
- ", path=" << request->urlpath << ", user=" << user << ", passwd=" << password);
- state = BEGIN;
- Ftp::Client::start();
-@@ -1750,7 +1757,9 @@
- if (ftpState->handlePasvReply(srvAddr))
- ftpState->connectDataChannel();
- else {
-- ftpSendEPRT(ftpState);
-+ ftpFail(ftpState);
-+ // Currently disabled, does not work correctly:
-+ // ftpSendEPRT(ftpState);
- return;
- }
- }
-@@ -1790,6 +1799,11 @@
- }
- safe_free(ftpState->data.host);
-
-+ if (!Comm::IsConnOpen(ftpState->ctrl.conn)) {
-+ debugs(9, 5, "The control connection to the remote end is closed");
-+ return;
-+ }
-+
- /*
- * Set up a listen socket on the same local address as the
- * control connection.
-@@ -1875,9 +1889,14 @@
- ftpRestOrList(ftpState);
- }
-
-+#if 0
- static void
- ftpSendEPRT(Ftp::Gateway * ftpState)
- {
-+ /* check the server control channel is still available */
-+ if (!ftpState || !ftpState->haveControlChannel("ftpSendEPRT"))
-+ return;
-+
- if (Config.Ftp.epsv_all && ftpState->flags.epsv_all_sent) {
- debugs(9, DBG_IMPORTANT, "FTP does not allow EPRT method after 'EPSV ALL' has been sent.");
- return;
-@@ -1913,6 +1932,7 @@
- ftpState->writeCommand(cbuf);
- ftpState->state = Ftp::Client::SENT_EPRT;
- }
-+#endif
-
- static void
- ftpReadEPRT(Ftp::Gateway * ftpState)
-@@ -1939,10 +1959,8 @@
- {
- debugs(9, 3, HERE);
-
-- if (EBIT_TEST(entry->flags, ENTRY_ABORTED)) {
-- abortAll("entry aborted when accepting data conn");
-- data.listenConn->close();
-- data.listenConn = NULL;
-+ if (!Comm::IsConnOpen(ctrl.conn)) { /*Close handlers will cleanup*/
-+ debugs(9, 5, "The control connection to the remote end is closed");
- return;
- }
-
-@@ -1955,6 +1973,14 @@
- return;
- }
-
-+ if (EBIT_TEST(entry->flags, ENTRY_ABORTED)) {
-+ abortAll("entry aborted when accepting data conn");
-+ data.listenConn->close();
-+ data.listenConn = NULL;
-+ io.conn->close();
-+ return;
-+ }
-+
- /* data listening conn is no longer even open. abort. */
- if (!Comm::IsConnOpen(data.listenConn)) {
- data.listenConn = NULL; // ensure that it's cleared and not just closed.
-@@ -2705,8 +2731,8 @@
- Ftp::Gateway::completeForwarding()
- {
- if (fwd == NULL || flags.completed_forwarding) {
-- debugs(9, 3, HERE << "completeForwarding avoids " <<
-- "double-complete on FD " << ctrl.conn->fd << ", Data FD " << data.conn->fd <<
-+ debugs(9, 3, "avoid double-complete on FD " <<
-+ (ctrl.conn != NULL ? ctrl.conn->fd : -1) << ", Data FD " << data.conn->fd <<
- ", this " << this << ", fwd " << fwd);
- return;
- }
-