Fixes and polishing in response to Amos' squid-dev review dated 2014/08/10.

Fixed handling of FTP commands terminated with LF instead of CRLF (regression
introduced during migration to Tokenizer in r12812).

Many polishing touches.

diff --git a/src/Server.h b/src/Server.h
index 589f76a5db9e2ce3fa515fa7c1582330fa98a97d..fbaff202bcba17eefd6f68e64ade7b9a072e0166 100644 (file)
--- a/src/Server.h
+++ b/src/Server.h
@@ -128,7 +128,7 @@ protected:
     virtual void closeServer() = 0;            /**< end communication with the server */
     virtual bool doneWithServer() const = 0;   /**< did we end communication? */
     /// whether we may receive more virgin response body bytes
-    virtual bool mayReadVirginReplyBody() const { return !doneWithServer(); }
+    virtual bool mayReadVirginReplyBody() const = 0;
 
     /// Entry-dependent callbacks use this check to quit if the entry went bad
     bool abortOnBadEntry(const char *abortReason);

diff --git a/src/cache_cf.cc b/src/cache_cf.cc
index a012e9351ecc527ec7ab4d2cbd418425042ee3e4..a96c3269fe08d9ea65cfe747ac9c1f7d05798f2c 100644 (file)
--- a/src/cache_cf.cc
+++ b/src/cache_cf.cc
@@ -3582,8 +3582,7 @@ parsePortProtocol(const SBuf &value)
         return AnyP::ProtocolVersion(AnyP::PROTO_HTTPS, 1,1);
 
     if (value.cmp("FTP") == 0)
-        return AnyP::ProtocolVersion(AnyP::PROTO_FTP,
-                                     Ftp::ProtocolVersion().major, Ftp::ProtocolVersion().minor);
+        return Ftp::ProtocolVersion();
 
     fatalf("%s directive does not support protocol=" SQUIDSBUFPH "\n", cfg_directive, SQUIDSBUFPRINT(value));
     return AnyP::ProtocolVersion(); // not reached

diff --git a/src/cf.data.pre b/src/cf.data.pre
index 84203db66303f14a8214e5c94db7ea6de99fbee3..42b41789c134f8498405356bfd2ee57ced1767cb 100644 (file)
--- a/src/cf.data.pre
+++ b/src/cf.data.pre
@@ -1914,11 +1914,11 @@ DOC_START
           name=token   Specifies an internal name for the port. Defaults to
                        the port address. Usable with myportname ACL.
 
-          ftp-track-dirs=on|off
+          ftp-track-dirs
                        Enables tracking of FTP directories by injecting extra
                        PWD commands and adjusting Request-URI (in wrapping
                        HTTP requests) to reflect the current FTP server
-                       directory. Disabled by default.
+                       directory. Tracking is disabled by default.
 
           protocol=FTP Protocol to reconstruct accelerated and intercepted
                        requests with. Defaults to FTP. No other accepted

diff --git a/src/clients/FtpClient.cc b/src/clients/FtpClient.cc
index 01a2f37dc602b3721fa22d0aa98dd322d1ac514b..65ca12a92bfcffe6c6dedb4c7ac25918db244844 100644 (file)
--- a/src/clients/FtpClient.cc
+++ b/src/clients/FtpClient.cc
@@ -858,7 +858,7 @@ Ftp::Client::maybeReadVirginBody()
 
     const int read_sz = replyBodySpace(*data.readBuf, 0);
 
-    debugs(11,9, "FTP may read up to " << read_sz << " bytes");
+    debugs(9, 9, "FTP may read up to " << read_sz << " bytes");
 
     if (read_sz < 2) // see http.cc
         return;

diff --git a/src/clients/FtpGateway.cc b/src/clients/FtpGateway.cc
index c827390153a3858dd4af488b08d7f5a8edb467d6..42e4baa51d228ea1e5fe7d644f7b51fb4520abfc 100644 (file)
--- a/src/clients/FtpGateway.cc
+++ b/src/clients/FtpGateway.cc
@@ -189,6 +189,7 @@ protected:
     virtual void dataClosed(const CommCloseCbParams &io);
 
 private:
+    virtual bool mayReadVirginReplyBody() const;
     // BodyConsumer for HTTP: consume request body.
     virtual void handleRequestBodyProducerAborted();
 
@@ -2759,6 +2760,13 @@ Ftp::Gateway::haveControlChannel(const char *caller_name) const
     return true;
 }
 
+bool
+Ftp::Gateway::mayReadVirginReplyBody() const
+{
+    // TODO: Can we do what Ftp::Relay::mayReadVirginReplyBody() does instead?
+    return !doneWithServer();
+}
+
 AsyncJob::Pointer
 Ftp::StartGateway(FwdState *const fwdState)
 {

diff --git a/src/clients/FtpRelay.cc b/src/clients/FtpRelay.cc
index 5c27b715fc031ea7512446ab4774b5f684bac58a..6a7cd53d7e7098a64ed4b331d5123037bf81f9f3 100644 (file)
--- a/src/clients/FtpRelay.cc
+++ b/src/clients/FtpRelay.cc
@@ -200,7 +200,7 @@ Ftp::Relay::updateMaster()
     CbcPointer<ConnStateData> &mgr = fwd->request->clientConnectionManager;
     if (mgr.valid()) {
         if (Ftp::Server *srv = dynamic_cast<Ftp::Server*>(mgr.get()))
-            return srv->master;
+            return *srv->master;
     }
     // this code will not be necessary once the master is inside MasterXaction
     debugs(9, 3, "our server side is gone: " << mgr);

diff --git a/src/clients/Makefile.am b/src/clients/Makefile.am
index af867511fd06fc150e9a8339689d486cb8561cc1..89f06a3899787be9ea0e0de5c30d35975edb15f6 100644 (file)
--- a/src/clients/Makefile.am
+++ b/src/clients/Makefile.am
@@ -1,4 +1,5 @@
 include $(top_srcdir)/src/Common.am
+include $(top_srcdir)/src/TestHeaders.am
 
 noinst_LTLIBRARIES = libclients.la
 
@@ -7,5 +8,4 @@ libclients_la_SOURCES = \
        FtpClient.h \
        FtpGateway.cc \
        FtpRelay.cc \
-       \
        forward.h

diff --git a/src/ftp/Makefile.am b/src/ftp/Makefile.am
index 6117116d9c65e9865bcb8f3374ce5835042661ab..622099313cc495ac8490260cb0bd23cf69f45c78 100644 (file)
--- a/src/ftp/Makefile.am
+++ b/src/ftp/Makefile.am
@@ -1,4 +1,5 @@
 include $(top_srcdir)/src/Common.am
+include $(top_srcdir)/src/TestHeaders.am
 
 noinst_LTLIBRARIES = libftp.la
 

diff --git a/src/http.cc b/src/http.cc
index 04bb1ef385a294e4c709cb1d77a6f7ebc4c859c5..708c6c12d62e20493970246150e51ad441373703 100644 (file)
--- a/src/http.cc
+++ b/src/http.cc
@@ -1494,6 +1494,15 @@ HttpStateData::processReplyBody()
     maybeReadVirginBody();
 }
 
+bool
+HttpStateData::mayReadVirginReplyBody() const
+{
+    // TODO: Be more precise here. For example, if/when reading trailer, we may
+    // not be doneWithServer() yet, but we should return false. Similarly, we
+    // could still be writing the request body after receiving the whole reply.
+    return !doneWithServer();
+}
+
 void
 HttpStateData::maybeReadVirginBody()
 {

diff --git a/src/http.h b/src/http.h
index 6cf6982e7ce45f861dc0228c38b0306885656ebd..d042fe750f64bcbacf90b8f13fc97d429f1a5a68 100644 (file)
--- a/src/http.h
+++ b/src/http.h
@@ -110,6 +110,7 @@ private:
     virtual void closeServer(); // end communication with the server
     virtual bool doneWithServer() const; // did we end communication?
     virtual void abortTransaction(const char *reason); // abnormal termination
+    virtual bool mayReadVirginReplyBody() const;
 
     // consuming request body
     virtual void handleMoreRequestBodyAvailable();

diff --git a/src/servers/FtpServer.cc b/src/servers/FtpServer.cc
index 3fd2837ce35eaf31800ac896a5d89a9e1136088e..b7e0e73b8b24f9acc8096505511e0a4f95b96dd1 100644 (file)
--- a/src/servers/FtpServer.cc
+++ b/src/servers/FtpServer.cc
@@ -4,6 +4,7 @@
 
 #include "squid.h"
 #include "base/CharacterSet.h"
+#include "base/RefCount.h"
 #include "base/Subscription.h"
 #include "client_side_reply.h"
 #include "client_side_request.h"
@@ -42,6 +43,7 @@ static bool CommandHasPathParameter(const SBuf &cmd);
 Ftp::Server::Server(const MasterXaction::Pointer &xact):
         AsyncJob("Ftp::Server"),
         ConnStateData(xact),
+        master(new MasterState),
         uri(),
         host(),
         gotEpsvAll(false),
@@ -315,7 +317,7 @@ void
 Ftp::Server::resetLogin(const char *reason)
 {
     debugs(33, 5, "will need to re-login due to " << reason);
-    master.clientReadGreeting = false;
+    master->clientReadGreeting = false;
     changeState(fssBegin, reason);
 }
 
@@ -325,10 +327,10 @@ Ftp::Server::calcUri(const SBuf *file)
 {
     uri = "ftp://";
     uri.append(host);
-    if (port->ftp_track_dirs && master.workingDir.length()) {
-        if (master.workingDir[0] != '/')
+    if (port->ftp_track_dirs && master->workingDir.length()) {
+        if (master->workingDir[0] != '/')
             uri.append("/");
-        uri.append(master.workingDir);
+        uri.append(master->workingDir);
     }
 
     if (uri[uri.length() - 1] != '/')
@@ -407,10 +409,10 @@ Ftp::Server::acceptDataConnection(const CommAcceptCbParams &params)
             AsyncCall::Pointer call = onDataAcceptCall;
             onDataAcceptCall = NULL;
             // If we got an upload request, start reading data from the client.
-            if (master.serverState == fssHandleUploadRequest)
+            if (master->serverState == fssHandleUploadRequest)
                 maybeReadUploadData();
             else
-                Must(master.serverState == fssHandleDataRequest);
+                Must(master->serverState == fssHandleDataRequest);
             MemBuf mb;
             mb.init();
             mb.Printf("150 Data connection opened.\r\n");
@@ -472,8 +474,8 @@ Ftp::Server::writeEarlyReply(const int code, const char *msg)
 void
 Ftp::Server::writeReply(MemBuf &mb)
 {
-    debugs(11, 2, "FTP Client " << clientConnection);
-    debugs(11, 2, "FTP Client REPLY:\n---------\n" << mb.buf <<
+    debugs(9, 2, "FTP Client " << clientConnection);
+    debugs(9, 2, "FTP Client REPLY:\n---------\n" << mb.buf <<
            "\n----------");
 
     typedef CommCbMemFunT<Server, CommIoCbParams> Dialer;
@@ -506,14 +508,14 @@ Ftp::Server::writeCustomReply(const int code, const char *msg, const HttpReply *
 void
 Ftp::Server::changeState(const ServerState newState, const char *reason)
 {
-    if (master.serverState == newState) {
-        debugs(33, 3, "client state unchanged at " << master.serverState <<
+    if (master->serverState == newState) {
+        debugs(33, 3, "client state unchanged at " << master->serverState <<
                " because " << reason);
-        master.serverState = newState;
+        master->serverState = newState;
     } else {
-        debugs(33, 3, "client state was " << master.serverState <<
+        debugs(33, 3, "client state was " << master->serverState <<
                ", now " << newState << " because " << reason);
-        master.serverState = newState;
+        master->serverState = newState;
     }
 }
 
@@ -549,14 +551,18 @@ ClientSocketContext *
 Ftp::Server::parseOneRequest(Http::ProtocolVersion &ver)
 {
     // OWS <command> [ RWS <parameter> ] OWS LF
-    static const CharacterSet WhiteSpace = CharacterSet("Ftp::WS", " \f\r\t\v");
-    static const CharacterSet BlackSpace = WhiteSpace.complement("!Ftp::WS");
-    static const CharacterSet NewLine = CharacterSet("NL", "\n");
-    static const CharacterSet OldLine = NewLine.complement("!NL");
+
+    // InlineSpaceChars are isspace(3) or RFC 959 Section 3.1.1.5.2, except
+    // for the LF character that we must exclude here (but see FullWhiteSpace).
+    static const char * const InlineSpaceChars = " \f\r\t\v";
+    static const CharacterSet InlineSpace = CharacterSet("Ftp::Inline", InlineSpaceChars);
+    static const CharacterSet FullWhiteSpace = (InlineSpace + CharacterSet::LF).rename("Ftp::FWS");
+    static const CharacterSet CommandChars = FullWhiteSpace.complement("Ftp::Command");
+    static const CharacterSet TailChars = CharacterSet::LF.complement("Ftp::Tail");
 
     // This set is used to ignore empty commands without allowing an attacker
     // to keep us endlessly busy by feeding us whitespace or empty commands.
-    static const CharacterSet LeadingSpace = (WhiteSpace + NewLine).rename("Ftp::LeadingSpace");
+    static const CharacterSet &LeadingSpace = FullWhiteSpace;
 
     SBuf cmd;
     SBuf params;
@@ -564,18 +570,18 @@ Ftp::Server::parseOneRequest(Http::ProtocolVersion &ver)
     Parser::Tokenizer tok(in.buf);
 
     (void)tok.skipAll(LeadingSpace); // leading OWS and empty commands
-    const bool parsed = tok.prefix(cmd, BlackSpace); // required command
+    const bool parsed = tok.prefix(cmd, CommandChars); // required command
 
     // note that the condition below will eat either RWS or trailing OWS
-    if (parsed && tok.skipAll(WhiteSpace) && tok.prefix(params, OldLine)) {
+    if (parsed && tok.skipAll(InlineSpace) && tok.prefix(params, TailChars)) {
         // now params may include trailing OWS
         // TODO: Support right-trimming using CharacterSet in Tokenizer instead
-        static const SBuf bufWhiteSpace("\r\t ");
+        static const SBuf bufWhiteSpace(InlineSpaceChars);
         params.trim(bufWhiteSpace, false, true);
     }
 
     // technically, we may skip multiple NLs below, but that is OK
-    if (!parsed || !tok.skipAll(NewLine)) { // did not find terminating LF yet
+    if (!parsed || !tok.skipAll(CharacterSet::LF)) { // did not find terminating LF yet
         // we need more data, but can we buffer more?
         if (in.buf.length() >= Config.maxRequestHeaderSize) {
             changeState(fssError, "huge req");
@@ -597,7 +603,7 @@ Ftp::Server::parseOneRequest(Http::ProtocolVersion &ver)
 
     // interception cases do not need USER to calculate the uri
     if (!transparent()) {
-        if (!master.clientReadGreeting) {
+        if (!master->clientReadGreeting) {
             // the first command must be USER
             if (!pinning.pinned && cmd != cmdUser()) {
                 writeEarlyReply(530, "Must login first");
@@ -619,7 +625,7 @@ Ftp::Server::parseOneRequest(Http::ProtocolVersion &ver)
         cmd == cmdAppe() || cmd == cmdStor() || cmd == cmdStou() ?
         Http::METHOD_PUT : Http::METHOD_GET;
 
-    const SBuf *path = params.length() && CommandHasPathParameter(cmd) ?
+    const SBuf *path = (params.length() && CommandHasPathParameter(cmd)) ?
                        &params : NULL;
     calcUri(path);
     char *newUri = xstrdup(uri.c_str());
@@ -702,14 +708,14 @@ Ftp::Server::handleReply(HttpReply *reply, StoreIOBuffer data)
         &Ftp::Server::handleErrorReply // fssError
     };
     const Server &server = dynamic_cast<const Ftp::Server&>(*context->getConn());
-    if (const ReplyHandler handler = handlers[server.master.serverState])
+    if (const ReplyHandler handler = handlers[server.master->serverState])
         (this->*handler)(reply, data);
     else
         writeForwardedReply(reply);
 }
 
 void
-Ftp::Server::handleFeatReply(const HttpReply *reply, StoreIOBuffer data)
+Ftp::Server::handleFeatReply(const HttpReply *reply, StoreIOBuffer)
 {
     if (getCurrentContext()->http->request->errType != ERR_NONE) {
         writeCustomReply(502, "Server does not support FEAT", reply);
@@ -779,7 +785,7 @@ Ftp::Server::handleFeatReply(const HttpReply *reply, StoreIOBuffer data)
 }
 
 void
-Ftp::Server::handlePasvReply(const HttpReply *reply, StoreIOBuffer data)
+Ftp::Server::handlePasvReply(const HttpReply *reply, StoreIOBuffer)
 {
     ClientSocketContext::Pointer context = getCurrentContext();
     assert(context != NULL);
@@ -814,12 +820,12 @@ Ftp::Server::handlePasvReply(const HttpReply *reply, StoreIOBuffer data)
               addr,
               static_cast<int>(localPort / 256),
               static_cast<int>(localPort % 256));
-    debugs(11, 3, Raw("writing", mb.buf, mb.size));
+    debugs(9, 3, Raw("writing", mb.buf, mb.size));
     writeReply(mb);
 }
 
 void
-Ftp::Server::handlePortReply(const HttpReply *reply, StoreIOBuffer data)
+Ftp::Server::handlePortReply(const HttpReply *reply, StoreIOBuffer)
 {
     if (getCurrentContext()->http->request->errType != ERR_NONE) {
         writeCustomReply(502, "Server does not support PASV (converted from PORT)", reply);
@@ -832,7 +838,7 @@ Ftp::Server::handlePortReply(const HttpReply *reply, StoreIOBuffer data)
 }
 
 void
-Ftp::Server::handleErrorReply(const HttpReply *reply, StoreIOBuffer data)
+Ftp::Server::handleErrorReply(const HttpReply *reply, StoreIOBuffer)
 {
     if (!pinning.pinned) // we failed to connect to server
         uri.clear();
@@ -890,8 +896,7 @@ Ftp::Server::wroteReplyData(const CommIoCbParams &io)
         return;
 
     if (io.flag != Comm::OK) {
-        debugs(33, 3, "FTP reply data writing failed: " <<
-               xstrerr(io.xerrno));
+        debugs(33, 3, "FTP reply data writing failed: " << xstrerr(io.xerrno));
         closeDataConnection();
         writeCustomReply(426, "Data connection error; transfer aborted");
         return;
@@ -931,7 +936,7 @@ Ftp::Server::replyDataWritingCheckpoint()
 }
 
 void
-Ftp::Server::handleUploadReply(const HttpReply *reply, StoreIOBuffer data)
+Ftp::Server::handleUploadReply(const HttpReply *reply, StoreIOBuffer)
 {
     writeForwardedReply(reply);
     // note that the client data connection may already be closed by now
@@ -954,7 +959,7 @@ Ftp::Server::writeForwardedReply(const HttpReply *reply)
 }
 
 void
-Ftp::Server::handleEprtReply(const HttpReply *reply, StoreIOBuffer data)
+Ftp::Server::handleEprtReply(const HttpReply *reply, StoreIOBuffer)
 {
     if (getCurrentContext()->http->request->errType != ERR_NONE) {
         writeCustomReply(502, "Server does not support PASV (converted from EPRT)", reply);
@@ -967,7 +972,7 @@ Ftp::Server::handleEprtReply(const HttpReply *reply, StoreIOBuffer data)
 }
 
 void
-Ftp::Server::handleEpsvReply(const HttpReply *reply, StoreIOBuffer data)
+Ftp::Server::handleEpsvReply(const HttpReply *reply, StoreIOBuffer)
 {
     if (getCurrentContext()->http->request->errType != ERR_NONE) {
         writeCustomReply(502, "Cannot connect to server", reply);
@@ -978,13 +983,13 @@ Ftp::Server::handleEpsvReply(const HttpReply *reply, StoreIOBuffer data)
     if (!localPort)
         return;
 
-    // In interception setups, we combine remote server address with a
-    // local port number and hope that traffic will be redirected to us.
+    // In interception setups, we use a local port number and hope that data
+    // traffic will be redirected to us.
     MemBuf mb;
     mb.init();
     mb.Printf("229 Entering Extended Passive Mode (|||%u|)\r\n", localPort);
 
-    debugs(11, 3, Raw("writing", mb.buf, mb.size));
+    debugs(9, 3, Raw("writing", mb.buf, mb.size));
     writeReply(mb);
 }
 
@@ -1066,13 +1071,13 @@ Ftp::Server::writeForwardedReplyAndCall(const HttpReply *reply, AsyncCall::Point
     // Status 125 or 150 implies upload or data request, but we still check
     // the state in case the server is buggy.
     if ((scode == 125 || scode == 150) &&
-            (master.serverState == fssHandleUploadRequest ||
-             master.serverState == fssHandleDataRequest)) {
+            (master->serverState == fssHandleUploadRequest ||
+             master->serverState == fssHandleDataRequest)) {
         if (checkDataConnPost()) {
             // If the data connection is ready, start reading data (here)
             // and forward the response to client (further below).
             debugs(33, 7, "data connection established, start data transfer");
-            if (master.serverState == fssHandleUploadRequest)
+            if (master->serverState == fssHandleUploadRequest)
                 maybeReadUploadData();
         } else {
             // If we are waiting to accept the data connection, keep waiting.
@@ -1096,8 +1101,8 @@ Ftp::Server::writeForwardedReplyAndCall(const HttpReply *reply, AsyncCall::Point
     mb.init();
     Ftp::PrintReply(mb, reply);
 
-    debugs(11, 2, "FTP Client " << clientConnection);
-    debugs(11, 2, "FTP Client REPLY:\n---------\n" << mb.buf <<
+    debugs(9, 2, "FTP Client " << clientConnection);
+    debugs(9, 2, "FTP Client REPLY:\n---------\n" << mb.buf <<
            "\n----------");
 
     Comm::Write(clientConnection, &mb, call);
@@ -1163,7 +1168,7 @@ Ftp::Server::wroteReply(const CommIoCbParams &io)
     context->http->out.size += io.size;
     context->http->out.headers_sz += io.size;
 
-    if (master.serverState == fssError) {
+    if (master->serverState == fssError) {
         debugs(33, 5, "closing on FTP server error");
         io.conn->close();
         return;
@@ -1194,7 +1199,7 @@ Ftp::Server::handleRequest(String &cmd, String &params)
     HttpRequest *request = getCurrentContext()->http->request;
     Must(request);
 
-    if (do_debug(11, 2)) {
+    if (do_debug(9, 2)) {
         MemBuf mb;
         Packer p;
         mb.init();
@@ -1202,8 +1207,8 @@ Ftp::Server::handleRequest(String &cmd, String &params)
         request->pack(&p);
         packerClean(&p);
 
-        debugs(11, 2, "FTP Client " << clientConnection);
-        debugs(11, 2, "FTP Client REQUEST:\n---------\n" << mb.buf <<
+        debugs(9, 2, "FTP Client " << clientConnection);
+        debugs(9, 2, "FTP Client REQUEST:\n---------\n" << mb.buf <<
                "\n----------");
     }
 
@@ -1235,7 +1240,7 @@ Ftp::Server::handleRequest(String &cmd, String &params)
     }
 
     if (!handler) {
-        debugs(11, 7, "forwarding " << cmd << " as is, no post-processing");
+        debugs(9, 7, "forwarding " << cmd << " as is, no post-processing");
         return true;
     }
 
@@ -1276,18 +1281,18 @@ Ftp::Server::handleUserRequest(const SBuf &cmd, SBuf &params)
     params.chop(0, eou); // leave just the login part for the peer
 
     SBuf oldUri;
-    if (master.clientReadGreeting)
+    if (master->clientReadGreeting)
         oldUri = uri;
 
-    master.workingDir.clear();
+    master->workingDir.clear();
     calcUri(NULL);
 
-    if (!master.clientReadGreeting) {
-        debugs(11, 3, "set URI to " << uri);
+    if (!master->clientReadGreeting) {
+        debugs(9, 3, "set URI to " << uri);
     } else if (oldUri.caseCmp(uri) == 0) {
-        debugs(11, 5, "kept URI as " << oldUri);
+        debugs(9, 5, "kept URI as " << oldUri);
     } else {
-        debugs(11, 3, "reset URI from " << oldUri << " to " << uri);
+        debugs(9, 3, "reset URI from " << oldUri << " to " << uri);
         closeDataConnection();
         unpinConnection(true); // close control connection to peer
         resetLogin("URI reset");
@@ -1340,17 +1345,16 @@ Ftp::Server::createDataConnection(Ip::Address cltAddr)
     closeDataConnection();
 
     Comm::ConnectionPointer conn = new Comm::Connection();
-    conn->remote = cltAddr;
+    conn->flags |= COMM_DOBIND;
 
     // Use local IP address of the control connection as the source address
     // of the active data connection, or some clients will refuse to accept.
-    conn->flags |= COMM_DOBIND;
-    conn->local = clientConnection->local;
+    conn->setAddrs(clientConnection->local, cltAddr);
     // RFC 959 requires active FTP connections to originate from port 20
     // but that would preclude us from supporting concurrent transfers! (XXX?)
     conn->local.port(0);
 
-    debugs(11, 3, "will actively connect from " << conn->local << " to " <<
+    debugs(9, 3, "will actively connect from " << conn->local << " to " <<
            conn->remote);
 
     dataConn = conn;
@@ -1412,7 +1416,7 @@ Ftp::Server::handleUploadRequest(String &cmd, String &params)
 bool
 Ftp::Server::handleEprtRequest(String &cmd, String &params)
 {
-    debugs(11, 3, "Process an EPRT " << params);
+    debugs(9, 3, "Process an EPRT " << params);
 
     if (gotEpsvAll) {
         setReply(500, "Rejecting EPRT after EPSV ALL");
@@ -1441,7 +1445,7 @@ Ftp::Server::handleEprtRequest(String &cmd, String &params)
 bool
 Ftp::Server::handleEpsvRequest(String &cmd, String &params)
 {
-    debugs(11, 3, "Process an EPSV command with params: " << params);
+    debugs(9, 3, "Process an EPSV command with params: " << params);
     if (params.size() <= 0) {
         // treat parameterless EPSV as "use the protocol of the ctrl conn"
     } else if (params.caseCmp("ALL") == 0) {
@@ -1498,7 +1502,7 @@ Ftp::Server::setDataCommand()
     header.putStr(HDR_FTP_COMMAND, "PASV");
     header.delById(HDR_FTP_ARGUMENTS);
     header.putStr(HDR_FTP_ARGUMENTS, "");
-    debugs(11, 5, "client data command converted to fake PASV");
+    debugs(9, 5, "client data command converted to fake PASV");
 }
 
 /// check that client data connection is ready for future I/O or at least

diff --git a/src/servers/FtpServer.h b/src/servers/FtpServer.h
index 574e26efc30031290429b86b02b6d9bd1bc5cceb..cdc5fa85af05d88cef8f7523dabbc2142631e1ad 100644 (file)
--- a/src/servers/FtpServer.h
+++ b/src/servers/FtpServer.h
@@ -6,6 +6,7 @@
 #define SQUID_SERVERS_FTP_SERVER_H
 
 #include "client_side.h"
+#include "base/Lock.h"
 
 namespace Ftp
 {
@@ -29,15 +30,17 @@ typedef enum {
 // TODO: This should become a part of MasterXaction when we start sending
 // master transactions to the clients/ code.
 /// Transaction information shared among our FTP client and server jobs.
-class MasterState
+class MasterState: public RefCountable
 {
 public:
+    typedef RefCount<MasterState> Pointer;
+
+    MasterState(): serverState(fssBegin), clientReadGreeting(false) {}
+
     Ip::Address clientDataAddr; ///< address of our FTP client data connection
     SBuf workingDir; ///< estimated current working directory for URI formation
     ServerState serverState; ///< what our FTP server is doing
     bool clientReadGreeting; ///< whether our FTP client read their FTP server greeting
-
-    MasterState(): serverState(fssBegin), clientReadGreeting(false) {}
 };
 
 /// Manages a control connection from an FTP client.
@@ -47,7 +50,9 @@ public:
     explicit Server(const MasterXaction::Pointer &xact);
     virtual ~Server();
 
-    MasterState master; ///< info shared among our FTP client and server jobs
+    // This is a pointer in hope to minimize future changes when MasterState
+    // becomes a part of MasterXaction. Guaranteed not to be nil.
+    MasterState::Pointer master; ///< info shared among our FTP client and server jobs
 
 protected:
     friend void StartListening();

diff --git a/src/servers/Makefile.am b/src/servers/Makefile.am
index ae0fc2d05e09f1bdc32295bc10148ecb9884a058..d59b63f29fd336d983198af0e1be9b575fa6f133 100644 (file)
--- a/src/servers/Makefile.am
+++ b/src/servers/Makefile.am
@@ -1,4 +1,5 @@
 include $(top_srcdir)/src/Common.am
+include $(top_srcdir)/src/TestHeaders.am
 
 noinst_LTLIBRARIES = libservers.la
 
@@ -6,5 +7,4 @@ libservers_la_SOURCES = \
        FtpServer.cc \
        FtpServer.h \
        HttpServer.cc \
-       \
        forward.h

diff --git a/src/servers/forward.h b/src/servers/forward.h
index 47def1d67b4f936d6d9bb5763c64e27dbef6dde9..5cf84ae2fe912eaffe3256d18ea79135898eda64 100644 (file)
--- a/src/servers/forward.h
+++ b/src/servers/forward.h
@@ -5,6 +5,8 @@ class MasterXaction;
 template <class C> class RefCount;
 typedef RefCount<MasterXaction> MasterXactionPointer;
 
+class ConnStateData;
+
 namespace Http
 {