Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade ACLFilledChecklist::request to smart Pointer #1501

Closed
wants to merge 2 commits into from
Closed

Upgrade ACLFilledChecklist::request to smart Pointer #1501

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
76c7762
Upgrade AccessLogEntry::request to smart Pointer
yadij Oct 6, 2023
f00569a
Apply suggestions from code review
yadij Oct 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 2 additions & 7 deletions src/acl/FilledChecklist.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ CBDATA_CLASS_INIT(ACLFilledChecklist);

ACLFilledChecklist::ACLFilledChecklist() :
dst_rdns(nullptr),
request (nullptr),
reply (nullptr),
#if USE_AUTH
auth_user_request (nullptr),
Expand All @@ -52,8 +51,6 @@ ACLFilledChecklist::~ACLFilledChecklist()

safe_free(dst_rdns); // created by xstrdup().

HTTPMSGUNLOCK(request);

HTTPMSGUNLOCK(reply);

cbdataReferenceDone(conn_);
Expand Down Expand Up @@ -89,13 +86,13 @@ ACLFilledChecklist::verifyAle() const
showDebugWarning("HttpRequest object");
// XXX: al->request should be original,
// but the request may be already adapted
al->request = request;
al->request = request.getRaw();
HTTPMSGLOCK(al->request);
}

if (!al->adapted_request) {
showDebugWarning("adapted HttpRequest object");
al->adapted_request = request;
al->adapted_request = request.getRaw();
HTTPMSGLOCK(al->adapted_request);
}

Expand Down Expand Up @@ -213,7 +210,6 @@ ACLFilledChecklist::markSourceDomainChecked()
*/
ACLFilledChecklist::ACLFilledChecklist(const acl_access *A, HttpRequest *http_request, const char *ident):
dst_rdns(nullptr),
request(nullptr),
reply(nullptr),
#if USE_AUTH
auth_user_request(nullptr),
Expand Down Expand Up @@ -242,7 +238,6 @@ void ACLFilledChecklist::setRequest(HttpRequest *httpRequest)
assert(!request);
if (httpRequest) {
request = httpRequest;
HTTPMSGLOCK(request);
#if FOLLOW_X_FORWARDED_FOR
if (Config.onoff.acl_uses_indirect_client)
src_addr = request->indirect_client_addr;
Expand Down
5 changes: 2 additions & 3 deletions src/acl/FilledChecklist.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "acl/forward.h"
#include "base/CbcPointer.h"
#include "error/forward.h"
#include "HttpRequest.h"
#include "ip/Address.h"
#if USE_AUTH
#include "auth/UserRequest.h"
Expand All @@ -22,8 +23,6 @@

class CachePeer;
class ConnStateData;
class HttpRequest;
class HttpReply;

/** \ingroup ACLAPI
ACLChecklist filled with specific data, representing Squid and transaction
Expand Down Expand Up @@ -76,7 +75,7 @@ class ACLFilledChecklist: public ACLChecklist
SBuf dst_peer_name;
char *dst_rdns;

HttpRequest *request;
HttpRequestPointer request;
yadij marked this conversation as resolved.
Show resolved Hide resolved
HttpReply *reply;

char rfc931[USER_IDENT_SZ];
Expand Down
6 changes: 3 additions & 3 deletions src/auth/Acl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ Acl::Answer
AuthenticateAcl(ACLChecklist *ch)
{
ACLFilledChecklist *checklist = Filled(ch);
HttpRequest *request = checklist->request;
const auto request = checklist->request;
Http::HdrType headertype;

if (nullptr == request) {
if (!request) {
fatal ("requiresRequest SHOULD have been true for this ACL!!");
return ACCESS_DENIED;
} else if (request->flags.sslBumped) {
Expand All @@ -55,7 +55,7 @@ AuthenticateAcl(ACLChecklist *ch)
/* get authed here */
/* Note: this fills in auth_user_request when applicable */
const AuthAclState result = Auth::UserRequest::tryToAuthenticateAndSetAuthUser(
&checklist->auth_user_request, headertype, request,
&checklist->auth_user_request, headertype, checklist->request.getRaw(),
checklist->conn(), checklist->src_addr, checklist->al);
switch (result) {

Expand Down
2 changes: 1 addition & 1 deletion src/auth/AclProxyAuth.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ ProxyAuthLookup::checkForAsync(ACLChecklist *cl) const
/* make sure someone created auth_user_request for us */
assert(checklist->auth_user_request != nullptr);
assert(checklist->auth_user_request->valid());
checklist->auth_user_request->start(checklist->request, checklist->al, LookupDone, checklist);
checklist->auth_user_request->start(checklist->request.getRaw(), checklist->al, LookupDone, checklist);
}

void
Expand Down
2 changes: 1 addition & 1 deletion src/external_acl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,7 @@ ACLExternal::~ACLExternal()
}

static void
copyResultsFromEntry(HttpRequest *req, const ExternalACLEntryPointer &entry)
copyResultsFromEntry(HttpRequestPointer &req, const ExternalACLEntryPointer &entry)
yadij marked this conversation as resolved.
Show resolved Hide resolved
{
if (req) {
#if USE_AUTH
Expand Down