From: Amos Jeffries Date: Sat, 7 May 2022 17:56:16 +0000 (+0000) Subject: Cleanup ClientHttpRequest-related code (#1045) X-Git-Tag: SQUID_6_0_1~191 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1d1457f2ff9b0c8e7951e0fc87b1eb9805d770d6;p=thirdparty%2Fsquid.git Cleanup ClientHttpRequest-related code (#1045) No logic changes. --- diff --git a/src/ClientRequestContext.h b/src/ClientRequestContext.h index 55a7a43822..a0c891107d 100644 --- a/src/ClientRequestContext.h +++ b/src/ClientRequestContext.h @@ -33,26 +33,26 @@ public: bool httpStateIsValid(); void hostHeaderVerify(); - void hostHeaderIpVerify(const ipcache_addrs* ia, const Dns::LookupDetails &dns); + void hostHeaderIpVerify(const ipcache_addrs *, const Dns::LookupDetails &); void hostHeaderVerifyFailed(const char *A, const char *B); void clientAccessCheck(); void clientAccessCheck2(); - void clientAccessCheckDone(const Acl::Answer &answer); + void clientAccessCheckDone(const Acl::Answer &); void clientRedirectStart(); - void clientRedirectDone(const Helper::Reply &reply); + void clientRedirectDone(const Helper::Reply &); void clientStoreIdStart(); - void clientStoreIdDone(const Helper::Reply &reply); + void clientStoreIdDone(const Helper::Reply &); void checkNoCache(); - void checkNoCacheDone(const Acl::Answer &answer); + void checkNoCacheDone(const Acl::Answer &); #if USE_ADAPTATION - void adaptationAccessCheck(); #endif #if USE_OPENSSL /** - * Initiates and start the acl checklist to check if the a CONNECT + * Initiates and start the acl checklist to check if the CONNECT * request must be bumped. - \retval true if the acl check scheduled, false if no ssl-bump required + * \retval true if the acl check scheduled + * \retval false if no ssl-bump required */ bool sslBumpAccessCheck(); /// The callback function for ssl-bump access check list @@ -60,26 +60,26 @@ public: #endif ClientHttpRequest *http; - ACLChecklist *acl_checklist; /* need ptr back so we can unreg if needed */ - int redirect_state; - int store_id_state; + ACLChecklist *acl_checklist = nullptr; ///< need ptr back so we can unregister if needed + int redirect_state = REDIRECT_NONE; + int store_id_state = REDIRECT_NONE; - bool host_header_verify_done; - bool http_access_done; - bool adapted_http_access_done; + bool host_header_verify_done = false; + bool http_access_done = false; + bool adapted_http_access_done = false; #if USE_ADAPTATION - bool adaptation_acl_check_done; + bool adaptation_acl_check_done = false; #endif - bool redirect_done; - bool store_id_done; - bool no_cache_done; - bool interpreted_req_hdrs; - bool toClientMarkingDone; + bool redirect_done = false; + bool store_id_done = false; + bool no_cache_done = false; + bool interpreted_req_hdrs = false; + bool toClientMarkingDone = false; #if USE_OPENSSL - bool sslBumpCheckDone; + bool sslBumpCheckDone = false; #endif - ErrorState *error; ///< saved error page for centralized/delayed processing - bool readNextRequest; ///< whether Squid should read after error handling + ErrorState *error = nullptr; ///< saved error page for centralized/delayed processing + bool readNextRequest = false; ///< whether Squid should read after error handling }; #endif /* SQUID_CLIENTREQUESTCONTEXT_H */ diff --git a/src/client_side_request.cc b/src/client_side_request.cc index e7e6cd0131..c2cc0baf6b 100644 --- a/src/client_side_request.cc +++ b/src/client_side_request.cc @@ -115,26 +115,7 @@ ClientRequestContext::~ClientRequestContext() } ClientRequestContext::ClientRequestContext(ClientHttpRequest *anHttp) : - http(cbdataReference(anHttp)), - acl_checklist(NULL), - redirect_state(REDIRECT_NONE), - store_id_state(REDIRECT_NONE), - host_header_verify_done(false), - http_access_done(false), - adapted_http_access_done(false), -#if USE_ADAPTATION - adaptation_acl_check_done(false), -#endif - redirect_done(false), - store_id_done(false), - no_cache_done(false), - interpreted_req_hdrs(false), - toClientMarkingDone(false), -#if USE_OPENSSL - sslBumpCheckDone(false), -#endif - error(NULL), - readNextRequest(false) + http(cbdataReference(anHttp)) { debugs(85, 3, "ClientRequestContext constructed, this=" << this); } @@ -145,29 +126,13 @@ ClientHttpRequest::ClientHttpRequest(ConnStateData * aConn) : #if USE_ADAPTATION AsyncJob("ClientHttpRequest"), #endif - request(NULL), - uri(NULL), - log_uri(NULL), - req_sz(0), al(new AccessLogEntry()), - calloutContext(NULL), - maxReplyBodySize_(0), - entry_(NULL), - loggingEntry_(NULL), conn_(cbdataReference(aConn)) -#if USE_OPENSSL - , sslBumpNeed_(Ssl::bumpEnd) -#endif -#if USE_ADAPTATION - , receivedWholeAdaptedReply(false) - , request_satisfaction_mode(false) - , request_satisfaction_offset(0) -#endif { CodeContext::Reset(al); al->cache.start_time = current_time; if (aConn) { - al->tcpClient = clientConnection = aConn->clientConnection; + al->tcpClient = aConn->clientConnection; al->cache.port = aConn->port; al->cache.caddr = aConn->log_addr; al->proxyProtocolHeader = aConn->proxyProtocolHeader(); @@ -292,10 +257,7 @@ ClientHttpRequest::~ClientHttpRequest() stopConsumingFrom(adaptedBodySource); #endif - if (calloutContext) - delete calloutContext; - - clientConnection = NULL; + delete calloutContext; if (conn_) cbdataReferenceDone(conn_); diff --git a/src/client_side_request.h b/src/client_side_request.h index bb84835e97..f5eee5f6d0 100644 --- a/src/client_side_request.h +++ b/src/client_side_request.h @@ -39,11 +39,9 @@ class ClientHttpRequest CBDATA_CLASS(ClientHttpRequest); public: - ClientHttpRequest(ConnStateData *csd); + ClientHttpRequest(ConnStateData *); + ClientHttpRequest(ClientHttpRequest &&) = delete; ~ClientHttpRequest(); - /* Not implemented - present to prevent synthetic operations */ - ClientHttpRequest(ClientHttpRequest const &); - ClientHttpRequest& operator=(ClientHttpRequest const &); String rangeBoundaryStr() const; void freeResources(); @@ -62,7 +60,7 @@ public: StoreEntry *loggingEntry() const { return loggingEntry_; } void loggingEntry(StoreEntry *); - ConnStateData * getConn() const { + ConnStateData *getConn() const { return (cbdataReferenceValid(conn_) ? conn_ : nullptr); } @@ -82,152 +80,146 @@ public: /// the processing tags associated with this request transaction. const LogTags &loggingTags() const { return al->cache.code; } - /** Details of the client socket which produced us. - * Treat as read-only for the lifetime of this HTTP request. - */ - Comm::ConnectionPointer clientConnection; + int64_t mRangeCLen() const; + + void doCallouts(); + + // The three methods below prepare log_uri and friends for future logging. + // Call the best-fit method whenever the current request or its URI changes. + + /// sets log_uri when we know the current request + void setLogUriToRequestUri(); + + /// sets log_uri to a parsed request URI when Squid fails to parse or + /// validate other request components, yielding no current request + void setLogUriToRawUri(const char *, const HttpRequestMethod &); + + /// sets log_uri and uri to an internally-generated "error:..." URI when + /// neither the current request nor the parsed request URI are known + void setErrorUri(const char *); + + /// Prepares to satisfy a Range request with a generated HTTP 206 response. + /// Initializes range_iter state to allow raw range_iter access. + /// \returns Content-Length value for the future response; never negative + int64_t prepPartialResponseGeneration(); + + /// Build an error reply. For use with the callouts. + void calloutsError(const err_type, const ErrorDetail::Pointer &); + /// if necessary, stores new error information (if any) + void updateError(const Error &); + +public: /// Request currently being handled by ClientHttpRequest. /// Usually remains nil until the virgin request header is parsed or faked. /// Starts as a virgin request; see initRequest(). /// Adaptation and redirections replace it; see resetRequest(). - HttpRequest * const request; + HttpRequest * const request = nullptr; /// Usually starts as a URI received from the client, with scheme and host /// added if needed. Is used to create the virgin request for initRequest(). /// URIs of adapted/redirected requests replace it via resetRequest(). - char *uri; + char *uri = nullptr; // TODO: remove this field and store the URI directly in al->url /// Cleaned up URI of the current (virgin or adapted/redirected) request, /// computed URI of an internally-generated requests, or /// one of the hard-coded "error:..." URIs. - char * const log_uri; + char * const log_uri = nullptr; String store_id; /* StoreID for transactions where the request member is nil */ struct Out { - Out() : offset(0), size(0), headers_sz(0) {} - /// Roughly speaking, this offset points to the next body byte we want /// to receive from Store. Without Ranges (and I/O errors), we should /// have received (and written to the client) all the previous bytes. /// XXX: The offset is updated by various receive-write steps, making /// its exact meaning illusive. Its Out class placement is confusing. - int64_t offset; + int64_t offset = 0; /// Response header and body bytes written to the client connection. - uint64_t size; + uint64_t size = 0; /// Response header bytes written to the client connection. /// Not to be confused with clientReplyContext::headers_sz. - size_t headers_sz; + size_t headers_sz = 0; } out; HttpHdrRangeIter range_iter; /* data for iterating thru range specs */ - size_t req_sz; /* raw request size on input, not current request size */ + size_t req_sz = 0; ///< raw request size on input, not current request size const AccessLogEntry::Pointer al; ///< access.log entry struct Flags { - Flags() : accel(false), internal(false), done_copying(false) {} - - bool accel; - bool internal; - bool done_copying; + bool accel = false; + bool internal = false; + bool done_copying = false; } flags; struct Redirect { - Redirect() : status(Http::scNone), location(NULL) {} - - Http::StatusCode status; - char *location; + Http::StatusCode status = Http::scNone; + char *location = nullptr; } redirect; dlink_node active; dlink_list client_stream; - int64_t mRangeCLen() const; - - ClientRequestContext *calloutContext; - void doCallouts(); - // The three methods below prepare log_uri and friends for future logging. - // Call the best-fit method whenever the current request or its URI changes. - - /// sets log_uri when we know the current request - void setLogUriToRequestUri(); - /// sets log_uri to a parsed request URI when Squid fails to parse or - /// validate other request components, yielding no current request - void setLogUriToRawUri(const char *rawUri, const HttpRequestMethod &); - /// sets log_uri and uri to an internally-generated "error:..." URI when - /// neither the current request nor the parsed request URI are known - void setErrorUri(const char *errorUri); - - /// Prepares to satisfy a Range request with a generated HTTP 206 response. - /// Initializes range_iter state to allow raw range_iter access. - /// \returns Content-Length value for the future response; never negative - int64_t prepPartialResponseGeneration(); - - /// Build an error reply. For use with the callouts. - void calloutsError(const err_type error, const ErrorDetail::Pointer &errDetail); - - /// if necessary, stores new error information (if any) - void updateError(const Error &error); - -#if USE_ADAPTATION - // AsyncJob virtual methods - virtual bool doneAll() const { - return Initiator::doneAll() && - BodyConsumer::doneAll() && false; - } - virtual void callException(const std::exception &ex); -#endif + ClientRequestContext *calloutContext = nullptr; private: /// assigns log_uri with aUri without copying the entire C-string - void absorbLogUri(char *aUri); + void absorbLogUri(char *); /// resets the current request and log_uri to nil void clearRequest(); /// initializes the current unassigned request to the virgin request /// sets the current request, asserting that it was unset - void assignRequest(HttpRequest *aRequest); + void assignRequest(HttpRequest *); - int64_t maxReplyBodySize_; - StoreEntry *entry_; - StoreEntry *loggingEntry_; - ConnStateData * conn_; + int64_t maxReplyBodySize_ = 0; + StoreEntry *entry_ = nullptr; + StoreEntry *loggingEntry_ = nullptr; + ConnStateData * conn_ = nullptr; #if USE_OPENSSL - /// whether (and how) the request needs to be bumped - Ssl::BumpMode sslBumpNeed_; - public: /// returns raw sslBump mode value Ssl::BumpMode sslBumpNeed() const { return sslBumpNeed_; } /// returns true if and only if the request needs to be bumped bool sslBumpNeeded() const { return sslBumpNeed_ == Ssl::bumpServerFirst || sslBumpNeed_ == Ssl::bumpClientFirst || sslBumpNeed_ == Ssl::bumpBump || sslBumpNeed_ == Ssl::bumpPeek || sslBumpNeed_ == Ssl::bumpStare; } /// set the sslBumpNeeded state - void sslBumpNeed(Ssl::BumpMode mode); + void sslBumpNeed(Ssl::BumpMode); void sslBumpStart(); - void sslBumpEstablish(Comm::Flag errflag); + void sslBumpEstablish(Comm::Flag); + +private: + /// whether (and how) the request needs to be bumped + Ssl::BumpMode sslBumpNeed_ = Ssl::bumpEnd; #endif #if USE_ADAPTATION - public: - void startAdaptation(const Adaptation::ServiceGroupPointer &g); + void startAdaptation(const Adaptation::ServiceGroupPointer &); bool requestSatisfactionMode() const { return request_satisfaction_mode; } + /* AsyncJob API */ + virtual bool doneAll() const { + return Initiator::doneAll() && + BodyConsumer::doneAll() && + false; // TODO: Refactor into a proper AsyncJob + } + virtual void callException(const std::exception &); + private: /// Handles an adaptation client request failure. /// Bypasses the error if possible, or build an error reply. - void handleAdaptationFailure(const ErrorDetail::Pointer &errDetail, bool bypassable = false); + void handleAdaptationFailure(const ErrorDetail::Pointer &, bool bypassable = false); - // Adaptation::Initiator API - virtual void noteAdaptationAnswer(const Adaptation::Answer &answer); - void handleAdaptedHeader(Http::Message *msg); - void handleAdaptationBlock(const Adaptation::Answer &answer); - virtual void noteAdaptationAclCheckDone(Adaptation::ServiceGroupPointer group); + void handleAdaptedHeader(Http::Message *); + void handleAdaptationBlock(const Adaptation::Answer &); - // BodyConsumer API, called by BodyPipe + /* Adaptation::Initiator API */ + virtual void noteAdaptationAclCheckDone(Adaptation::ServiceGroupPointer); + virtual void noteAdaptationAnswer(const Adaptation::Answer &); + + /* BodyConsumer API */ virtual void noteMoreBodyDataAvailable(BodyPipe::Pointer); virtual void noteBodyProductionEnded(BodyPipe::Pointer); virtual void noteBodyProducerAborted(BodyPipe::Pointer); @@ -241,17 +233,17 @@ private: BodyPipe::Pointer adaptedBodySource; /// noteBodyProductionEnded() was called - bool receivedWholeAdaptedReply; + bool receivedWholeAdaptedReply = false; - bool request_satisfaction_mode; - int64_t request_satisfaction_offset; + bool request_satisfaction_mode = false; + int64_t request_satisfaction_offset = 0; #endif }; /* client http based routines */ char *clientConstructTraceEcho(ClientHttpRequest *); -ACLFilledChecklist *clientAclChecklistCreate(const acl_access * acl,ClientHttpRequest * http); +ACLFilledChecklist *clientAclChecklistCreate(const acl_access *, ClientHttpRequest *); void clientAclChecklistFill(ACLFilledChecklist &, ClientHttpRequest *); void clientAccessCheck(ClientHttpRequest *); @@ -259,4 +251,3 @@ void clientAccessCheck(ClientHttpRequest *); void tunnelStart(ClientHttpRequest *); #endif /* SQUID_CLIENTSIDEREQUEST_H */ -