]> git.ipfire.org Git - thirdparty/squid.git/commitdiff
Cleanup ClientHttpRequest-related code (#1045)
authorAmos Jeffries <yadij@users.noreply.github.com>
Sat, 7 May 2022 17:56:16 +0000 (17:56 +0000)
committerSquid Anubis <squid-anubis@squid-cache.org>
Mon, 9 May 2022 11:01:05 +0000 (11:01 +0000)
No logic changes.

src/ClientRequestContext.h
src/client_side_request.cc
src/client_side_request.h

index 55a7a4382288e075923072437893673d6bbce387..a0c891107d3a96bc9439056b3c7365942746989d 100644 (file)
@@ -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 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 */
index e7e6cd0131f5e15f76dde9f5b193033796c2c030..c2cc0baf6bc88dd534b0a6808e38529ceb48e61c 100644 (file)
@@ -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_);
index bb84835e971d2ca6161e3ed55b4bf29963509db5..f5eee5f6d0fce3e11f6d74bd34abe1916c353c95 100644 (file)
@@ -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 */
-