Ensure initClient MasterXactions have listening ports (#993)

MasterXaction creation code that forgets to supply a port for an
initClient transaction will now be rejected by the compiler.

This safety improvement hit two problems described below.

The TcpAcceptor class was incorrectly creating a MasterXaction object
for FTP DATA connections. We moved MasterXaction creation to TcpAcceptor
callbacks because only they know whether accepting a TCP connection
implies a new master transaction start (it does not for FTP DATA cases).

The Downloader class was implemented to spawn its own MasterXaction
using XactionInitiator info supplied by the download requestor. That
design is incompatible with the new static assertion because each
MasterXaction creator must now hard-code XactionInitiator value for the
assertion to work at _compile_ time. All other contexts naturally
hard-code that value.

We saw two ways to resolve this conflict:

a) Let the download requestor create the MasterXaction object for the
   Downloader. The primary advantage of this (implemented) approach is
   that it allows (future) client request processing code to satisfy
   client requests using Downloader. Such request satisfaction requires
   sharing the master transaction between the client transaction and the
   downloader transaction, and this change enables such sharing. This
   change moves the "Which master transaction does this download belong
   to?" question from the Downloader into download requestors.

b) Move initClient-has-port enforcement (and squidPort) from
   MasterXaction into XactionInitiator. The primary advantage of this
   (not implemented) approach is that it places that enforcement inside
   the type it is meant to police (essentially) -- XactionInitiator.

The two changes are complementary. We did not implement (b) because it
requires significant XactionInitiator redesign, moving _all_ originating
client information from MasterXaction to XactionInitiator (currently
squidPort and tcpClient).

diff --git a/src/CommCalls.cc b/src/CommCalls.cc
index 7933a50d471306ba77526625740c70f51051567a..519219bd12a29532bcf5d4a8c70b7734eb49487d 100644 (file)
--- a/src/CommCalls.cc
+++ b/src/CommCalls.cc
@@ -49,7 +49,7 @@ CommCommonCbParams::print(std::ostream &os) const
 /* CommAcceptCbParams */
 
 CommAcceptCbParams::CommAcceptCbParams(void *aData):
-    CommCommonCbParams(aData), xaction()
+    CommCommonCbParams(aData)
 {
 }
 
@@ -61,8 +61,8 @@ CommAcceptCbParams::print(std::ostream &os) const
 {
     CommCommonCbParams::print(os);
 
-    if (xaction != NULL)
-        os << ", " << xaction->id;
+    if (port && port->listenConn)
+        os << ", " << port->listenConn->codeContextGist();
 }
 
 /* CommConnectCbParams */

diff --git a/src/CommCalls.h b/src/CommCalls.h
index c2adfcf34b5f1dab7cd33a144d589328dff3c661..e56a79196f857ebe20307b1863d4d2abfb8f8970 100644 (file)
--- a/src/CommCalls.h
+++ b/src/CommCalls.h
@@ -101,8 +101,8 @@ public:
 
     void print(std::ostream &os) const;
 
-    /// Transaction which this call is part of.
-    MasterXaction::Pointer xaction;
+    /// the configuration listening port this call relates to (may be nil)
+    AnyP::PortCfgPointer port;
 };
 
 // connect parameters

diff --git a/src/Downloader.cc b/src/Downloader.cc
index b7f8553e6b0d9c41d7badac9becee1831209cd57..c1295e04764641c9baa1c3d842961314799c3a64 100644 (file)
--- a/src/Downloader.cc
+++ b/src/Downloader.cc
@@ -64,12 +64,12 @@ Downloader::CbDialer::print(std::ostream &os) const
     os << " Http Status:" << status << Raw("body data", object.rawContent(), 64).hex();
 }
 
-Downloader::Downloader(SBuf &url, AsyncCall::Pointer &aCallback, const XactionInitiator initiator, unsigned int level):
+Downloader::Downloader(const SBuf &url, const AsyncCall::Pointer &aCallback, const MasterXactionPointer &masterXaction, unsigned int level):
     AsyncJob("Downloader"),
     url_(url),
     callback_(aCallback),
     level_(level),
-    initiator_(initiator)
+    masterXaction_(masterXaction)
 {
 }
 
@@ -133,8 +133,7 @@ Downloader::buildRequest()
 {
     const HttpRequestMethod method = Http::METHOD_GET;
 
-    const MasterXaction::Pointer mx = new MasterXaction(initiator_);
-    auto * const request = HttpRequest::FromUrl(url_, mx, method);
+    const auto request = HttpRequest::FromUrl(url_, masterXaction_, method);
     if (!request) {
         debugs(33, 5, "Invalid URI: " << url_);
         return false; //earlyError(...)

diff --git a/src/Downloader.h b/src/Downloader.h
index 3dd24d2cc126aef25269afba2de315db34c804c2..957afdc21fbdbcae943ad3e6045892ee08da5382 100644 (file)
--- a/src/Downloader.h
+++ b/src/Downloader.h
@@ -14,13 +14,14 @@
 #include "http/forward.h"
 #include "http/StatusCode.h"
 #include "sbuf/SBuf.h"
-#include "XactionInitiator.h"
 
 class ClientHttpRequest;
 class StoreIOBuffer;
 class clientStreamNode;
 class DownloaderContext;
 typedef RefCount<DownloaderContext> DownloaderContextPointer;
+class MasterXaction;
+using MasterXactionPointer = RefCount<MasterXaction>;
 
 /// The Downloader class fetches SBuf-storable things for other Squid
 /// components/transactions using internal requests. For example, it is used
@@ -46,7 +47,7 @@ public:
         Http::StatusCode status;
     };
 
-    Downloader(SBuf &url, AsyncCall::Pointer &aCallback, const XactionInitiator initiator, unsigned int level = 0);
+    Downloader(const SBuf &url, const AsyncCall::Pointer &aCallback, const MasterXactionPointer &, unsigned int level = 0);
     virtual ~Downloader();
     virtual void swanSong();
 
@@ -76,8 +77,7 @@ private:
     AsyncCall::Pointer callback_; ///< callback to call when download finishes
     SBuf object_; ///< the object body data
     const unsigned int level_; ///< holds the nested downloads level
-    /// The initiator of the download request.
-    XactionInitiator initiator_;
+    MasterXactionPointer masterXaction_; ///< download transaction context
 
     /// Pointer to an object that stores the clientStream required info
     DownloaderContextPointer context_;

diff --git a/src/MasterXaction.h b/src/MasterXaction.h
index 277a1b8b97918bef9d2ad491c59f2ffd36eed5f4..9dcc8ecb7eda9439cd26b20b798003a2fb4848fc 100644 (file)
--- a/src/MasterXaction.h
+++ b/src/MasterXaction.h
@@ -41,7 +41,20 @@ class MasterXaction : public RefCountable
 public:
     typedef RefCount<MasterXaction> Pointer;
 
-    explicit MasterXaction(const XactionInitiator anInitiator) : initiator(anInitiator) {};
+    /// Create a master transaction not associated with a AnyP::PortCfg port.
+    template <XactionInitiator::Initiator anInitiator>
+    static Pointer MakePortless()
+    {
+        static_assert(anInitiator != XactionInitiator::initClient, "not an HTTP or FTP client");
+        return new MasterXaction(anInitiator, nullptr);
+    }
+
+    /// Create a master transaction associated with a AnyP::PortCfg port.
+    /// \param aPort may be nil if port information was lost
+    static Pointer MakePortful(const AnyP::PortCfgPointer &aPort)
+    {
+        return new MasterXaction(XactionInitiator::initClient, aPort);
+    }
 
     /// transaction ID.
     InstanceId<MasterXaction, uint64_t> id;
@@ -59,6 +72,13 @@ public:
     bool generatingConnect = false;
 
     // TODO: add state from other Jobs in the transaction
+
+private:
+    // use public Make() functions instead
+    MasterXaction(const XactionInitiator anInitiator, const AnyP::PortCfgPointer &aPort):
+        squidPort(aPort),
+        initiator(anInitiator)
+    {}
 };
 
 #endif /* SQUID_SRC_MASTERXACTION_H */

diff --git a/src/PeerPoolMgr.cc b/src/PeerPoolMgr.cc
index 60d7343f40701936099a83d9f355544614f6b2c7..974f7a0648b93fa17f47610aafd7ab4d58283823 100644 (file)
--- a/src/PeerPoolMgr.cc
+++ b/src/PeerPoolMgr.cc
@@ -58,7 +58,7 @@ PeerPoolMgr::start()
 {
     AsyncJob::start();
 
-    const MasterXaction::Pointer mx = new MasterXaction(XactionInitiator::initPeerPool);
+    const auto mx = MasterXaction::MakePortless<XactionInitiator::initPeerPool>();
     // ErrorState, getOutgoingAddress(), and other APIs may require a request.
     // We fake one. TODO: Optionally send this request to peers?
     request = new HttpRequest(Http::METHOD_OPTIONS, AnyP::PROTO_HTTP, "http", "*", mx);

diff --git a/src/acl/Asn.cc b/src/acl/Asn.cc
index f0719671c82806e8be42ea09c561532e13f21936..5f2372e716d9d015a058ba23a8baf0e521acc6e1 100644 (file)
--- a/src/acl/Asn.cc
+++ b/src/acl/Asn.cc
@@ -237,7 +237,7 @@ asnCacheStart(int as)
     debugs(53, 3, "AS " << as);
     ASState *asState = new ASState;
     asState->as_number = as;
-    const MasterXaction::Pointer mx = new MasterXaction(XactionInitiator::initAsn);
+    const auto mx = MasterXaction::MakePortless<XactionInitiator::initAsn>();
     asState->request = new HttpRequest(mx);
     asState->request->url = whoisUrl;
     asState->request->method = Http::METHOD_GET;

diff --git a/src/adaptation/ecap/Host.cc b/src/adaptation/ecap/Host.cc
index edb55de0b3a9ea403b00d4a0598e5431ea49fd50..6202a5f14d38e5c01d6c738d9c287b264aad55e9 100644 (file)
--- a/src/adaptation/ecap/Host.cc
+++ b/src/adaptation/ecap/Host.cc
@@ -163,7 +163,7 @@ Adaptation::Ecap::Host::closeDebug(std::ostream *debug)
 Adaptation::Ecap::Host::MessagePtr
 Adaptation::Ecap::Host::newRequest() const
 {
-    static const MasterXaction::Pointer mx = new MasterXaction(XactionInitiator::initAdaptationOrphan_);
+    static const auto mx = MasterXaction::MakePortless<XactionInitiator::initAdaptationOrphan_>();
     return MessagePtr(new Adaptation::Ecap::MessageRep(new HttpRequest(mx)));
 }
 

diff --git a/src/adaptation/icap/Xaction.cc b/src/adaptation/icap/Xaction.cc
index 499c618dd0b5f6bd37297dcd6221feb6efd3e5a9..33d2dc9eda0add4ba0eab56720d0e03ffed13a3f 100644 (file)
--- a/src/adaptation/icap/Xaction.cc
+++ b/src/adaptation/icap/Xaction.cc
@@ -94,7 +94,7 @@ Adaptation::Icap::Xaction::Xaction(const char *aTypeName, Adaptation::Icap::Serv
 {
     debugs(93,3, typeName << " constructed, this=" << this <<
            " [icapx" << id << ']'); // we should not call virtual status() here
-    const MasterXaction::Pointer mx = new MasterXaction(XactionInitiator::initAdaptation);
+    const auto mx = MasterXaction::MakePortless<XactionInitiator::initAdaptation>();
     icapRequest = new HttpRequest(mx);
     HTTPMSGLOCK(icapRequest);
     icap_tr_start = current_time;

diff --git a/src/client_side.cc b/src/client_side.cc
index a1c683e6d08962c2d6e113be1f76149846482581..f85594fe51ce1c1c3b966e72c6533ad981e45e98 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -2299,22 +2299,24 @@ ConnStateData::acceptTls()
 void
 httpAccept(const CommAcceptCbParams &params)
 {
-    MasterXaction::Pointer xact = params.xaction;
-    AnyP::PortCfgPointer s = xact->squidPort;
+    Assure(params.port);
 
     // NP: it is possible the port was reconfigured when the call or accept() was queued.
 
     if (params.flag != Comm::OK) {
         // Its possible the call was still queued when the client disconnected
-        debugs(33, 2, s->listenConn << ": accept failure: " << xstrerr(params.xerrno));
+        debugs(33, 2, params.port->listenConn << ": accept failure: " << xstrerr(params.xerrno));
         return;
     }
 
     debugs(33, 4, params.conn << ": accepted");
     fd_note(params.conn->fd, "client http connect");
+    const auto xact = MasterXaction::MakePortful(params.port);
+    xact->tcpClient = params.conn;
 
     // Socket is ready, setup the connection manager to start using it
     auto *srv = Http::NewServer(xact);
+    // XXX: do not abandon the MasterXaction object
     AsyncJob::Start(srv); // usually async-calls readSomeData()
 }
 
@@ -2499,22 +2501,25 @@ httpsSslBumpAccessCheckDone(Acl::Answer answer, void *data)
 static void
 httpsAccept(const CommAcceptCbParams &params)
 {
-    MasterXaction::Pointer xact = params.xaction;
-    const AnyP::PortCfgPointer s = xact->squidPort;
+    Assure(params.port);
 
     // NP: it is possible the port was reconfigured when the call or accept() was queued.
 
     if (params.flag != Comm::OK) {
         // Its possible the call was still queued when the client disconnected
-        debugs(33, 2, "httpsAccept: " << s->listenConn << ": accept failure: " << xstrerr(params.xerrno));
+        debugs(33, 2, "httpsAccept: " << params.port->listenConn << ": accept failure: " << xstrerr(params.xerrno));
         return;
     }
 
+    const auto xact = MasterXaction::MakePortful(params.port);
+    xact->tcpClient = params.conn;
+
     debugs(33, 4, params.conn << " accepted, starting SSL negotiation.");
     fd_note(params.conn->fd, "client https connect");
 
     // Socket is ready, setup the connection manager to start using it
     auto *srv = Https::NewServer(xact);
+    // XXX: do not abandon the MasterXaction object
     AsyncJob::Start(srv); // usually async-calls postHttpsAccept()
 }
 
@@ -2530,7 +2535,7 @@ ConnStateData::postHttpsAccept()
             return;
         }
 
-        MasterXaction::Pointer mx = new MasterXaction(XactionInitiator::initClient);
+        const auto mx = MasterXaction::MakePortful(port);
         mx->tcpClient = clientConnection;
         // Create a fake HTTP request and ALE for the ssl_bump ACL check,
         // using tproxy/intercept provided destination IP and port.
@@ -3236,7 +3241,7 @@ ConnStateData::buildFakeRequest(SBuf &useHost, unsigned short usePort, const SBu
     extendLifetime();
     stream->registerWithConn();
 
-    MasterXaction::Pointer mx = new MasterXaction(XactionInitiator::initClient);
+    const auto mx = MasterXaction::MakePortful(port);
     mx->tcpClient = clientConnection;
     // Setup Http::Request object. Maybe should be replaced by a call to (modified)
     // clientProcessRequest

diff --git a/src/client_side_reply.cc b/src/client_side_reply.cc
index ca6334ac8e147cc8d767b4681273e4df8eb19fbc..dc293c556d0c6f463743bc3bc054495e90096f39 100644 (file)
--- a/src/client_side_reply.cc
+++ b/src/client_side_reply.cc
@@ -2139,7 +2139,8 @@ clientReplyContext::createStoreEntry(const HttpRequestMethod& m, RequestFlags re
      */
 
     if (http->request == NULL) {
-        const MasterXaction::Pointer mx = new MasterXaction(XactionInitiator::initClient);
+        const auto connManager = http->getConn();
+        const auto mx = MasterXaction::MakePortful(connManager ? connManager->port : nullptr);
         // XXX: These fake URI parameters shadow the real (or error:...) URI.
         // TODO: Either always set the request earlier and assert here OR use
         // http->uri (converted to Anyp::Uri) to create this catch-all request.

diff --git a/src/comm/TcpAcceptor.cc b/src/comm/TcpAcceptor.cc
index b93991ef8bbe498dd0062eac0a06ef9429c72f59..134e10f525be70e9d41bf95896694268792e215e 100644 (file)
--- a/src/comm/TcpAcceptor.cc
+++ b/src/comm/TcpAcceptor.cc
@@ -322,10 +322,9 @@ Comm::TcpAcceptor::notify(const Comm::Flag flag, const Comm::ConnectionPointer &
     if (theCallSub != NULL) {
         AsyncCall::Pointer call = theCallSub->callback();
         CommAcceptCbParams &params = GetCommParams<CommAcceptCbParams>(call);
-        params.xaction = new MasterXaction(XactionInitiator::initClient);
-        params.xaction->squidPort = listenPort_;
+        params.port = listenPort_;
         params.fd = conn->fd;
-        params.conn = params.xaction->tcpClient = newConnDetails;
+        params.conn = newConnDetails;
         params.flag = flag;
         params.xerrno = errcode;
         ScheduleCallHere(call);

diff --git a/src/esi/Include.cc b/src/esi/Include.cc
index 1dc93481e945fb00642887005d0e5cfb3cf84cd5..9c2f28af3a04c0822cecd79ee2a212090bab6652 100644 (file)
--- a/src/esi/Include.cc
+++ b/src/esi/Include.cc
@@ -287,7 +287,7 @@ ESIInclude::Start (ESIStreamContext::Pointer stream, char const *url, ESIVarStat
     char const *tempUrl = vars->extractChar ();
 
     debugs(86, 5, "ESIIncludeStart: Starting subrequest with url '" << tempUrl << "'");
-    const MasterXaction::Pointer mx = new MasterXaction(XactionInitiator::initEsi);
+    const auto mx = MasterXaction::MakePortless<XactionInitiator::initEsi>();
     if (clientBeginRequest(Http::METHOD_GET, tempUrl, esiBufferRecipient, esiBufferDetach, stream.getRaw(), &tempheaders, stream->localbuffer->buf, HTTP_REQBUF_SZ, mx)) {
         debugs(86, DBG_CRITICAL, "ERROR: starting new ESI subrequest failed");
     }

diff --git a/src/htcp.cc b/src/htcp.cc
index c1eef39122eeaed1d1a355eb6e44d04f0798909d..391b620562e5d28fcbed1777a430c4c774729698 100644 (file)
--- a/src/htcp.cc
+++ b/src/htcp.cc
@@ -701,7 +701,7 @@ htcpUnpackSpecifier(char *buf, int sz)
     // Parse the request
     method.HttpRequestMethodXXX(s->method);
 
-    const MasterXaction::Pointer mx = new MasterXaction(XactionInitiator::initHtcp);
+    const auto mx = MasterXaction::MakePortless<XactionInitiator::initHtcp>();
     s->request = HttpRequest::FromUrlXXX(s->uri, mx, method == Http::METHOD_NONE ? HttpRequestMethod(Http::METHOD_GET) : method);
     if (!s->request) {
         debugs(31, 3, "failed to create request. Invalid URI?");

diff --git a/src/icmp/net_db.cc b/src/icmp/net_db.cc
index 52a5fa2cba57aab4baeca222e36a922a9792cfb2..a5da2f7808f8152b04e7b4b22dacf2e93e88a651 100644 (file)
--- a/src/icmp/net_db.cc
+++ b/src/icmp/net_db.cc
@@ -1296,7 +1296,7 @@ netdbExchangeStart(void *data)
     static const SBuf netDB("netdb");
     char *uri = internalRemoteUri(p->secure.encryptTransport, p->host, p->http_port, "/squid-internal-dynamic/", netDB);
     debugs(38, 3, "Requesting '" << uri << "'");
-    const MasterXaction::Pointer mx = new MasterXaction(XactionInitiator::initIcmp);
+    const auto mx = MasterXaction::MakePortless<XactionInitiator::initIcmp>();
     HttpRequestPointer req(HttpRequest::FromUrlXXX(uri, mx));
 
     if (!req) {

diff --git a/src/icp_v2.cc b/src/icp_v2.cc
index 5d6d639e9b590cc4f2863fa18dfea2aa1638dfc7..740b6ec7cafc9888fb41c43ccf6fdba2fe7ea18d 100644 (file)
--- a/src/icp_v2.cc
+++ b/src/icp_v2.cc
@@ -461,7 +461,7 @@ icpGetRequest(char *url, int reqnum, int fd, Ip::Address &from)
         return NULL;
     }
 
-    const MasterXaction::Pointer mx = new MasterXaction(XactionInitiator::initIcp);
+    const auto mx = MasterXaction::MakePortless<XactionInitiator::initIcp>();
     auto *result = HttpRequest::FromUrlXXX(url, mx);
     if (!result)
         icpCreateAndSend(ICP_ERR, 0, url, reqnum, 0, fd, from, nullptr);

diff --git a/src/mgr/Inquirer.cc b/src/mgr/Inquirer.cc
index 6a2d610fc1d9d11fea8f38c5dd906404d4e2c2a7..45fdf1afa593636b9534e7d93ab4a196e550e84a 100644 (file)
--- a/src/mgr/Inquirer.cc
+++ b/src/mgr/Inquirer.cc
@@ -76,7 +76,7 @@ Mgr::Inquirer::start()
     std::unique_ptr<MemBuf> replyBuf;
     if (strands.empty()) {
         const char *url = aggrAction->command().params.httpUri.termedBuf();
-        const MasterXaction::Pointer mx = new MasterXaction(XactionInitiator::initIpc);
+        const auto mx = MasterXaction::MakePortless<XactionInitiator::initIpc>();
         auto *req = HttpRequest::FromUrlXXX(url, mx);
         ErrorState err(ERR_INVALID_URL, Http::scNotFound, req, nullptr);
         std::unique_ptr<HttpReply> reply(err.BuildHttpReply());

diff --git a/src/mime.cc b/src/mime.cc
index 83da9fba0013ca70c41a6039fcb9c3ad35911e6e..5db0c46281ac601cd8ece5736157ef21bfb9cc04 100644 (file)
--- a/src/mime.cc
+++ b/src/mime.cc
@@ -400,7 +400,7 @@ MimeIcon::load()
 
     /* fill `e` with a canned 2xx response object */
 
-    const MasterXaction::Pointer mx = new MasterXaction(XactionInitiator::initIcon);
+    const auto mx = MasterXaction::MakePortless<XactionInitiator::initIcon>();
     HttpRequestPointer r(HttpRequest::FromUrlXXX(url_, mx));
     if (!r)
         fatalf("mimeLoadIcon: cannot parse internal URL: %s", url_);

diff --git a/src/neighbors.cc b/src/neighbors.cc
index acc97465ff25689fe30e26ea12cc6c3b9cca33fe..64c6c4d0227d7009413cb128ed4f563a644933d9 100644 (file)
--- a/src/neighbors.cc
+++ b/src/neighbors.cc
@@ -1418,7 +1418,7 @@ peerCountMcastPeersCreateAndSend(CachePeer * const p)
     snprintf(url, MAX_URL, "http://");
     p->in_addr.toUrl(url+7, MAX_URL -8 );
     strcat(url, "/");
-    const MasterXaction::Pointer mx = new MasterXaction(XactionInitiator::initPeerMcast);
+    const auto mx = MasterXaction::MakePortless<XactionInitiator::initPeerMcast>();
     auto *req = HttpRequest::FromUrlXXX(url, mx);
     assert(req != nullptr);
     const AccessLogEntry::Pointer ale = new AccessLogEntry;

diff --git a/src/peer_digest.cc b/src/peer_digest.cc
index 4bbb95fb632047cc4bb3012b3d9eea2705ead079..ebae875930121dc4efe004989c422911827fe576 100644 (file)
--- a/src/peer_digest.cc
+++ b/src/peer_digest.cc
@@ -306,7 +306,7 @@ peerDigestRequest(PeerDigest * pd)
         url = xstrdup(internalRemoteUri(p->secure.encryptTransport, p->host, p->http_port, "/squid-internal-periodic/", SBuf(StoreDigestFileName)));
     debugs(72, 2, url);
 
-    const MasterXaction::Pointer mx = new MasterXaction(XactionInitiator::initCacheDigest);
+    const auto mx = MasterXaction::MakePortless<XactionInitiator::initCacheDigest>();
     req = HttpRequest::FromUrlXXX(url, mx);
 
     assert(req);

diff --git a/src/security/PeerConnector.cc b/src/security/PeerConnector.cc
index 7e96485fc788cd8654fe06d81eccad0fad8c8315..cfebd9a94d7a8de849ad9eb4f12dd3cc88f4a0ba 100644 (file)
--- a/src/security/PeerConnector.cc
+++ b/src/security/PeerConnector.cc
@@ -639,7 +639,9 @@ Security::PeerConnector::startCertDownloading(SBuf &url)
                                       "Security::PeerConnector::certDownloadingDone",
                                       PeerConnectorCertDownloaderDialer(&Security::PeerConnector::certDownloadingDone, this));
 
-    const auto dl = new Downloader(url, certCallback, XactionInitiator::initCertFetcher, certDownloadNestingLevel() + 1);
+    const auto dl = new Downloader(url, certCallback,
+        MasterXaction::MakePortless<XactionInitiator::initCertFetcher>(),
+        certDownloadNestingLevel() + 1);
     certDownloadWait.start(dl, certCallback);
 }
 

diff --git a/src/servers/FtpServer.cc b/src/servers/FtpServer.cc
index 472b8b242d8c5f4d3b4d6c9563a812436978b0d3..7068bfd6faea9009c6e389561c22010c59ef6ccb 100644 (file)
--- a/src/servers/FtpServer.cc
+++ b/src/servers/FtpServer.cc
@@ -241,21 +241,24 @@ Ftp::Server::noteBodyConsumerAborted(BodyPipe::Pointer ptr)
 void
 Ftp::Server::AcceptCtrlConnection(const CommAcceptCbParams &params)
 {
-    MasterXaction::Pointer xact = params.xaction;
-    AnyP::PortCfgPointer s = xact->squidPort;
+    Assure(params.port);
 
     // NP: it is possible the port was reconfigured when the call or accept() was queued.
 
     if (params.flag != Comm::OK) {
         // Its possible the call was still queued when the client disconnected
-        debugs(33, 2, s->listenConn << ": FTP accept failure: " << xstrerr(params.xerrno));
+        debugs(33, 2, params.port->listenConn << ": FTP accept failure: " << xstrerr(params.xerrno));
         return;
     }
 
     debugs(33, 4, params.conn << ": accepted");
     fd_note(params.conn->fd, "client ftp connect");
 
+    const auto xact = MasterXaction::MakePortful(params.port);
+    xact->tcpClient = params.conn;
+
     AsyncJob::Start(new Server(xact));
+    // XXX: do not abandon the MasterXaction object
 }
 
 void
@@ -720,7 +723,7 @@ Ftp::Server::parseOneRequest()
     const SBuf *path = (params.length() && CommandHasPathParameter(cmd)) ?
                        &params : NULL;
     calcUri(path);
-    MasterXaction::Pointer mx = new MasterXaction(XactionInitiator::initClient);
+    const auto mx = MasterXaction::MakePortful(port);
     mx->tcpClient = clientConnection;
     auto * const request = HttpRequest::FromUrl(uri, mx, method);
     if (!request) {

diff --git a/src/servers/Http1Server.cc b/src/servers/Http1Server.cc
index 9b6e56f83cbbe64f50b43b9371a836199863e239..38599df0be141f3e25bc019987b04b919f98c9bb 100644 (file)
--- a/src/servers/Http1Server.cc
+++ b/src/servers/Http1Server.cc
@@ -133,7 +133,7 @@ Http::One::Server::buildHttpRequest(Http::StreamPointer &context)
     }
 
     // TODO: move URL parse into Http Parser and INVALID_URL into the above parse error handling
-    MasterXaction::Pointer mx = new MasterXaction(XactionInitiator::initClient);
+    const auto mx = MasterXaction::MakePortful(port);
     mx->tcpClient = clientConnection;
     request = HttpRequest::FromUrlXXX(http->uri, mx, parser_->method());
     if (!request) {
@@ -392,13 +392,13 @@ Http::One::Server::noteTakeServerConnectionControl(ServerConnectionContext serve
 }
 
 ConnStateData *
-Http::NewServer(MasterXactionPointer &xact)
+Http::NewServer(const MasterXaction::Pointer &xact)
 {
     return new Http1::Server(xact, false);
 }
 
 ConnStateData *
-Https::NewServer(MasterXactionPointer &xact)
+Https::NewServer(const MasterXaction::Pointer &xact)
 {
     return new Http1::Server(xact, true);
 }

diff --git a/src/servers/forward.h b/src/servers/forward.h
index 4cdbe85f57744df25dac2e7043c2bbf15bc21ebc..d3ca715fc7d6dbeae0c9b705c25d8af3d91a95a0 100644 (file)
--- a/src/servers/forward.h
+++ b/src/servers/forward.h
@@ -24,7 +24,7 @@ class Server;
 } // namespace One
 
 /// create a new HTTP connection handler; never returns NULL
-ConnStateData *NewServer(MasterXactionPointer &xact);
+ConnStateData *NewServer(const MasterXaction::Pointer &xact);
 
 } // namespace Http
 
@@ -32,7 +32,7 @@ namespace Https
 {
 
 /// create a new HTTPS connection handler; never returns NULL
-ConnStateData *NewServer(MasterXactionPointer &xact);
+ConnStateData *NewServer(const MasterXaction::Pointer &xact);
 
 } // namespace Https
 

diff --git a/src/store_digest.cc b/src/store_digest.cc
index e15197cde2c8d6652a33ef23aac61c01c074845c..a7c20e0888ba98ba8fd7ac4c8a371499afb17826 100644 (file)
--- a/src/store_digest.cc
+++ b/src/store_digest.cc
@@ -417,7 +417,7 @@ storeDigestRewriteStart(void *)
     debugs(71, 2, "storeDigestRewrite: start rewrite #" << sd_state.rewrite_count + 1);
 
     const char *url = internalLocalUri("/squid-internal-periodic/", SBuf(StoreDigestFileName));
-    const MasterXaction::Pointer mx = new MasterXaction(XactionInitiator::initCacheDigest);
+    const auto mx = MasterXaction::MakePortless<XactionInitiator::initCacheDigest>();
     auto req = HttpRequest::FromUrlXXX(url, mx);
 
     RequestFlags flags;

diff --git a/src/tests/testHttpRequest.cc b/src/tests/testHttpRequest.cc
index 047296858c871444e65ba37b01de37e879475364..38d1a0b0df51fbeecf0be9f7aa758607be1bdccc 100644 (file)
--- a/src/tests/testHttpRequest.cc
+++ b/src/tests/testHttpRequest.cc
@@ -46,7 +46,7 @@ testHttpRequest::testCreateFromUrl()
     /* vanilla url, implicit method */
     unsigned short expected_port;
     SBuf url("http://foo:90/bar");
-    const MasterXaction::Pointer mx = new MasterXaction(XactionInitiator::initClient);
+    const auto mx = MasterXaction::MakePortless<XactionInitiator::initHtcp>();
     HttpRequest *aRequest = HttpRequest::FromUrl(url, mx);
     expected_port = 90;
     CPPUNIT_ASSERT(aRequest != nullptr);
@@ -109,7 +109,7 @@ testHttpRequest::testIPv6HostColonBug()
 
     /* valid IPv6 address without port */
     SBuf url("http://[2000:800::45]/foo");
-    const MasterXaction::Pointer mx = new MasterXaction(XactionInitiator::initClient);
+    const auto mx = MasterXaction::MakePortless<XactionInitiator::initHtcp>();
     aRequest = HttpRequest::FromUrl(url, mx, Http::METHOD_GET);
     expected_port = 80;
     CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->url.port());
@@ -143,7 +143,7 @@ void
 testHttpRequest::testSanityCheckStartLine()
 {
     MemBuf input;
-    const MasterXaction::Pointer mx = new MasterXaction(XactionInitiator::initClient);
+    const auto mx = MasterXaction::MakePortless<XactionInitiator::initHtcp>();
     PrivateHttpRequest engine(mx);
     Http::StatusCode error = Http::scNone;
     size_t hdr_len;