From: Amos Jeffries <> Date: Sun, 2 Apr 2017 12:06:21 +0000 (+1200) Subject: Cleanup: make Client::request a Pointer X-Git-Tag: M-staged-PR71~206 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=d603e3c2c65ce4707d29c1bdf2f8c305237e29be;p=thirdparty%2Fsquid.git Cleanup: make Client::request a Pointer --- diff --git a/src/clients/Client.cc b/src/clients/Client.cc index 8706157252..4f5e8ba620 100644 --- a/src/clients/Client.cc +++ b/src/clients/Client.cc @@ -57,7 +57,6 @@ Client::Client(FwdState *theFwdState): AsyncJob("Client"), entry->lock("Client"); request = fwd->request; - HTTPMSGLOCK(request); } Client::~Client() @@ -71,7 +70,6 @@ Client::~Client() entry->unlock("Client"); - HTTPMSGUNLOCK(request); HTTPMSGUNLOCK(theVirginReply); HTTPMSGUNLOCK(theFinalReply); @@ -179,9 +177,7 @@ Client::serverComplete() } completed = true; - - HttpRequest *r = originalRequest(); - r->hier.stopPeerClock(true); + originalRequest()->hier.stopPeerClock(true); if (requestBodySource != NULL) stopConsumingFrom(requestBodySource); @@ -232,7 +228,7 @@ Client::completeForwarding() // Register to receive request body bool Client::startRequestBodyFlow() { - HttpRequest *r = originalRequest(); + HttpRequestPointer r(originalRequest()); assert(r->body_pipe != NULL); requestBodySource = r->body_pipe; if (requestBodySource->setConsumerIfNotLate(this)) { @@ -511,9 +507,9 @@ Client::maybePurgeOthers() SBuf tmp(request->effectiveRequestUri()); const char *reqUrl = tmp.c_str(); debugs(88, 5, "maybe purging due to " << request->method << ' ' << tmp); - purgeEntriesByUrl(request, reqUrl); - purgeEntriesByHeader(request, reqUrl, theFinalReply, Http::HdrType::LOCATION); - purgeEntriesByHeader(request, reqUrl, theFinalReply, Http::HdrType::CONTENT_LOCATION); + purgeEntriesByUrl(request.getRaw(), reqUrl); + purgeEntriesByHeader(request.getRaw(), reqUrl, theFinalReply, Http::HdrType::LOCATION); + purgeEntriesByHeader(request.getRaw(), reqUrl, theFinalReply, Http::HdrType::CONTENT_LOCATION); } /// called when we have final (possibly adapted) reply headers; kids extend @@ -536,7 +532,7 @@ Client::blockCaching() if (const Acl::Tree *acl = Config.accessList.storeMiss) { // This relatively expensive check is not in StoreEntry::checkCachable: // That method lacks HttpRequest and may be called too many times. - ACLFilledChecklist ch(acl, originalRequest(), NULL); + ACLFilledChecklist ch(acl, originalRequest().getRaw()); ch.reply = const_cast(entry->getReply()); // ACLFilledChecklist API bug HTTPMSGLOCK(ch.reply); if (ch.fastCheck() != ACCESS_ALLOWED) { // when in doubt, block @@ -547,7 +543,7 @@ Client::blockCaching() return false; } -HttpRequest * +HttpRequestPointer Client::originalRequest() { return request; @@ -872,7 +868,7 @@ Client::handledEarlyAdaptationAbort() { if (entry->isEmpty()) { debugs(11,8, "adaptation failure with an empty entry: " << *entry); - ErrorState *err = new ErrorState(ERR_ICAP_FAILURE, Http::scInternalServerError, request); + ErrorState *err = new ErrorState(ERR_ICAP_FAILURE, Http::scInternalServerError, request.getRaw()); err->detailError(ERR_DETAIL_ICAP_RESPMOD_EARLY); fwd->fail(err); fwd->dontRetry(true); @@ -909,7 +905,7 @@ Client::handleAdaptationBlocked(const Adaptation::Answer &answer) if (page_id == ERR_NONE) page_id = ERR_ACCESS_DENIED; - ErrorState *err = new ErrorState(page_id, Http::scForbidden, request); + ErrorState *err = new ErrorState(page_id, Http::scForbidden, request.getRaw()); err->detailError(ERR_DETAIL_RESPMOD_BLOCK_EARLY); fwd->fail(err); fwd->dontRetry(true); @@ -940,7 +936,7 @@ Client::noteAdaptationAclCheckDone(Adaptation::ServiceGroupPointer group) return; } - startAdaptation(group, originalRequest()); + startAdaptation(group, originalRequest().getRaw()); processReplyBody(); } #endif @@ -948,7 +944,7 @@ Client::noteAdaptationAclCheckDone(Adaptation::ServiceGroupPointer group) void Client::sendBodyIsTooLargeError() { - ErrorState *err = new ErrorState(ERR_TOO_BIG, Http::scForbidden, request); + ErrorState *err = new ErrorState(ERR_TOO_BIG, Http::scForbidden, request.getRaw()); fwd->fail(err); fwd->dontRetry(true); abortOnData("Virgin body too large."); @@ -964,7 +960,7 @@ Client::adaptOrFinalizeReply() // The callback can be called with a NULL service if adaptation is off. adaptationAccessCheckPending = Adaptation::AccessCheck::Start( Adaptation::methodRespmod, Adaptation::pointPreCache, - originalRequest(), virginReply(), fwd->al, this); + originalRequest().getRaw(), virginReply(), fwd->al, this); debugs(11,5, HERE << "adaptationAccessCheckPending=" << adaptationAccessCheckPending); if (adaptationAccessCheckPending) return; diff --git a/src/clients/Client.h b/src/clients/Client.h index 89d538e643..d6f1548a92 100644 --- a/src/clients/Client.h +++ b/src/clients/Client.h @@ -60,7 +60,7 @@ public: virtual bool abortOnData(const char *reason); /// a hack to reach HttpStateData::orignal_request - virtual HttpRequest *originalRequest(); + virtual HttpRequestPointer originalRequest(); #if USE_ADAPTATION // Adaptation::Initiator API: start an ICAP transaction and receive adapted headers. @@ -163,7 +163,7 @@ protected: public: // should not be StoreEntry *entry; FwdState::Pointer fwd; - HttpRequest *request; + HttpRequestPointer request; protected: BodyPipe::Pointer requestBodySource; /**< to consume request body */ diff --git a/src/clients/FtpGateway.cc b/src/clients/FtpGateway.cc index aa4d97aa87..8e83f22a26 100644 --- a/src/clients/FtpGateway.cc +++ b/src/clients/FtpGateway.cc @@ -1163,7 +1163,7 @@ Ftp::Gateway::start() if (!checkAuth(&request->header)) { /* create appropriate reply */ SBuf realm(ftpRealm()); // local copy so SBuf wont disappear too early - HttpReply *reply = ftpAuthRequired(request, realm); + HttpReply *reply = ftpAuthRequired(request.getRaw(), realm); entry->replaceHttpReply(reply); serverComplete(); return; @@ -2276,12 +2276,12 @@ Ftp::Gateway::completedListing() { assert(entry); entry->lock("Ftp::Gateway"); - ErrorState ferr(ERR_DIR_LISTING, Http::scOkay, request); + ErrorState ferr(ERR_DIR_LISTING, Http::scOkay, request.getRaw()); ferr.ftp.listing = &listing; ferr.ftp.cwd_msg = xstrdup(cwd_message.size()? cwd_message.termedBuf() : ""); ferr.ftp.server_msg = ctrl.message; ctrl.message = NULL; - entry->replaceHttpReply( ferr.BuildHttpReply() ); + entry->replaceHttpReply(ferr.BuildHttpReply()); EBIT_CLR(entry->flags, ENTRY_FWD_HDR_WAIT); entry->flush(); entry->unlock("Ftp::Gateway"); @@ -2519,7 +2519,7 @@ ftpSendReply(Ftp::Gateway * ftpState) http_code = Http::scInternalServerError; } - ErrorState err(err_code, http_code, ftpState->request); + ErrorState err(err_code, http_code, ftpState->request.getRaw()); if (ftpState->old_request) err.ftp.request = xstrdup(ftpState->old_request); @@ -2536,7 +2536,7 @@ ftpSendReply(Ftp::Gateway * ftpState) // TODO: interpret as FTP-specific error code err.detailError(code); - ftpState->entry->replaceHttpReply( err.BuildHttpReply() ); + ftpState->entry->replaceHttpReply(err.BuildHttpReply()); ftpSendQuit(ftpState); } diff --git a/src/http.cc b/src/http.cc index d3a3517640..3d7fb5c814 100644 --- a/src/http.cc +++ b/src/http.cc @@ -823,7 +823,7 @@ HttpStateData::handle1xx(HttpReply *reply) #if USE_HTTP_VIOLATIONS // check whether the 1xx response forwarding is allowed by squid.conf if (Config.accessList.reply) { - ACLFilledChecklist ch(Config.accessList.reply, originalRequest(), NULL); + ACLFilledChecklist ch(Config.accessList.reply, originalRequest().getRaw()); ch.reply = reply; HTTPMSGLOCK(ch.reply); if (ch.fastCheck() != ACCESS_ALLOWED) { // TODO: support slow lookups? @@ -939,7 +939,7 @@ HttpStateData::haveParsedReplyHeaders() || rep->header.has(Http::HdrType::HDR_X_ACCELERATOR_VARY) #endif ) { - const SBuf vary(httpMakeVaryMark(request, rep)); + const SBuf vary(httpMakeVaryMark(request.getRaw(), rep)); if (vary.isEmpty()) { entry->makePrivate(); @@ -1484,7 +1484,7 @@ HttpStateData::processReplyBody() } if (ispinned && request->clientConnectionManager.valid()) { - request->clientConnectionManager->pinConnection(serverConnection, request, _peer, + request->clientConnectionManager->pinConnection(serverConnection, request.getRaw(), _peer, (request->flags.connectionAuth)); } else { fwd->pconnPush(serverConnection, request->url.host()); @@ -2159,7 +2159,7 @@ HttpStateData::buildRequestPrefix(MemBuf * mb) /* build and pack headers */ { HttpHeader hdr(hoRequest); - httpBuildRequestHeader(request, entry, fwd->al, &hdr, flags); + httpBuildRequestHeader(request.getRaw(), entry, fwd->al, &hdr, flags); if (request->flags.pinned && request->flags.connectionAuth) request->flags.authSent = true; @@ -2332,7 +2332,7 @@ HttpStateData::finishingBrokenPost() return false; } - ACLFilledChecklist ch(Config.accessList.brokenPosts, originalRequest(), NULL); + ACLFilledChecklist ch(Config.accessList.brokenPosts, originalRequest().getRaw()); if (ch.fastCheck() != ACCESS_ALLOWED) { debugs(11, 5, HERE << "didn't match brokenPosts"); return false;