------------------------------------------------------------
revno: 13895
revision-id: squid3@treenet.co.nz-20150821012552-2qv5m6a8ez0j0ful
parent: squid3@treenet.co.nz-20150821005450-yzh2ft4yuwad2mt5
author: Christos Tsantilas <chtsanti@users.sourceforge.net>
committer: Amos Jeffries <squid3@treenet.co.nz>
branch nick: 3.5
timestamp: Thu 2015-08-20 18:25:52 -0700
message:
  FtpServer.cc:1024: "reply != NULL" assertion
  
  Handle nil HttpReply pointer inside various handlers called from
  Ftp::Server::handleReply(). For example, when the related StoreEntry
  object is aborted, the client_side_reply.cc code may call the 
  Ftp::Server::handleReply() method with a nil reply pointer.
  
  The Ftp::Server::handleReply() methods itself cannot handle nil replies
  because they are valid in many states. Only state-specific handlers know
  whether they need the reply.
  
  The Ftp::Server::handleReply() method is called [via Store] from Client code.
  Thus, exceptions in handleReply() are handled by the Ftp::Client job. That job
  does not have enough information to know whether the client-to-Squid connection
  should be closed; the job keeps the connection open. When the reply is nil,
  that open connection becomes unusable, leading to more problems.
  
  This patch fixes the Ftp::Server::handleReply() to handle exceptions,
  including closing the connections in the case of an exception. It also
  adds Must(reply) checks to check for nil HttpReply pointers where the 
  reply is required. Eventually, Store should start using async calls to
  protect jobs waiting for Store updates. Meanwhile, this should help.
  
  This is a Measurement Factory project.
------------------------------------------------------------
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: squid3@treenet.co.nz-20150821012552-2qv5m6a8ez0j0ful
# target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# testament_sha1: 5a6159ee1f214a3a0ee2b4fb9e949be284d3ef01
# timestamp: 2015-08-21 01:51:01 +0000
# source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# base_revision_id: squid3@treenet.co.nz-20150821005450-\
#   yzh2ft4yuwad2mt5
# 
# Begin patch
=== modified file 'src/servers/FtpServer.cc'
--- src/servers/FtpServer.cc	2015-01-13 09:13:49 +0000
+++ src/servers/FtpServer.cc	2015-08-21 01:25:52 +0000
@@ -785,11 +785,16 @@
         NULL, // fssHandleCdup
         &Ftp::Server::handleErrorReply // fssError
     };
-    const Server &server = dynamic_cast<const Ftp::Server&>(*context->getConn());
-    if (const ReplyHandler handler = handlers[server.master->serverState])
-        (this->*handler)(reply, data);
-    else
-        writeForwardedReply(reply);
+    try {
+        const Server &server = dynamic_cast<const Ftp::Server&>(*context->getConn());
+        if (const ReplyHandler handler = handlers[server.master->serverState])
+            (this->*handler)(reply, data);
+        else
+            writeForwardedReply(reply);
+    } catch (const std::exception &e) {
+        callException(e);
+        throw TexcHere(e.what());
+    }
 }
 
 void
@@ -800,6 +805,7 @@
         return;
     }
 
+    Must(reply);
     HttpReply::Pointer featReply = Ftp::HttpReplyWrapper(211, "End", Http::scNoContent, 0);
     HttpHeader const &serverReplyHeader = reply->header;
 
@@ -1021,7 +1027,8 @@
 void
 Ftp::Server::writeForwardedReply(const HttpReply *reply)
 {
-    assert(reply != NULL);
+    Must(reply);
+
     const HttpHeader &header = reply->header;
     // adaptation and forwarding errors lack HDR_FTP_STATUS
     if (!header.has(HDR_FTP_STATUS)) {
@@ -1102,7 +1109,7 @@
     }
 #endif
 
-    assert(reply != NULL);
+    Must(reply);
     const char *reason = reply->header.has(HDR_FTP_REASON) ?
                          reply->header.getStr(HDR_FTP_REASON):
                          reply->sline.reason();
@@ -1676,6 +1683,17 @@
     http->storeEntry()->replaceHttpReply(reply);
 }
 
+void
+Ftp::Server::callException(const std::exception &e)
+{
+    debugs(33, 2, "FTP::Server job caught: " << e.what());
+    closeDataConnection();
+    unpinConnection(true);
+    if (Comm::IsConnOpen(clientConnection))
+        clientConnection->close();
+    AsyncJob::callException(e);
+}
+
 /// Whether Squid FTP Relay supports a named feature (e.g., a command).
 static bool
 Ftp::SupportedCommand(const SBuf &name)

=== modified file 'src/servers/FtpServer.h'
--- src/servers/FtpServer.h	2015-01-13 09:13:49 +0000
+++ src/servers/FtpServer.h	2015-08-21 01:25:52 +0000
@@ -55,6 +55,8 @@
 public:
     explicit Server(const MasterXaction::Pointer &xact);
     virtual ~Server();
+    /* AsyncJob API */
+    virtual void callException(const std::exception &e);
 
     // This is a pointer in hope to minimize future changes when MasterState
     // becomes a part of MasterXaction. Guaranteed not to be nil.