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

Jump to bottom

Supply ALE with HttpReply before checking http_reply_access #398

Closed
wants to merge 27 commits into from
Closed

Supply ALE with HttpReply before checking http_reply_access #398

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
be6c958
Supply ALE with HttpReply before checking http_reply_access
eduard-bagdasaryan May 2, 2019
1ce7574
Use ALE::setReply() for ALE::reply assigment
eduard-bagdasaryan May 3, 2019
3461984
Fixed unit tests
eduard-bagdasaryan May 4, 2019
57aabda
Check ALE for nil before setting the reply field
eduard-bagdasaryan May 4, 2019
a79cdca
A couple of comments added
eduard-bagdasaryan Jul 3, 2019
c5aaabc
Converted ALE::reply to HttpReplyPointer
eduard-bagdasaryan Jul 3, 2019
43f7d5f
Revert "Fixed unit tests"
eduard-bagdasaryan Jul 3, 2019
be1da33
Merge branch 'master' into SQUID-446-http-reply-access-misses-httpreply
eduard-bagdasaryan Jul 3, 2019
67448b6
Fixed the terminology for reply-related format codes.
eduard-bagdasaryan Jul 17, 2019
854c634
Added a wiki page reference explaining the response terminology
eduard-bagdasaryan Jul 17, 2019
53a8826
Added a TODO about possibly unreachable code
eduard-bagdasaryan Jul 22, 2019
824da98
Several fixes
eduard-bagdasaryan Jul 25, 2019
0eda4cf
Supply ALE with HttpReply as early as possible
eduard-bagdasaryan Jul 31, 2019
01703cf
Supply ALE for send_hit ACL
eduard-bagdasaryan Aug 1, 2019
fdf6662
Fix '<h' for control (1xx) responses
eduard-bagdasaryan Aug 1, 2019
d513678
Log with '<h' actual reply headers instead of a cached header string
eduard-bagdasaryan Aug 1, 2019
dc453ae
Polishing
eduard-bagdasaryan Aug 5, 2019
7a96f97
Partially undone fdf6662
eduard-bagdasaryan Aug 5, 2019
cc860b2
Fixed ALE::reply description
eduard-bagdasaryan Aug 6, 2019
1e7eb49
Added XXX about an inconsistency between '<h' and '>h'
eduard-bagdasaryan Aug 6, 2019
5f8055f
Use 'SeeSquidWiki' keyword as terminology reference
eduard-bagdasaryan Aug 12, 2019
78ca607
Each affected format code should refer to 'SeeSquidWiki'
eduard-bagdasaryan Aug 12, 2019
7bd1b13
Removed controversal 'SeeSquidWiki' documentation labels
eduard-bagdasaryan Aug 21, 2019
3318fe2
Polished
eduard-bagdasaryan Aug 21, 2019
064bf97
Review request: undone documentation changes (as controversal)
eduard-bagdasaryan Aug 22, 2019
8cd9b08
Polishing
eduard-bagdasaryan Aug 23, 2019
766df68
Synced with master
eduard-bagdasaryan Sep 9, 2019
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
8 changes: 8 additions & 0 deletions src/AccessLogEntry.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,14 @@ AccessLogEntry::syncNotes(HttpRequest *req)
assert(notes == req->notes());
}

void
AccessLogEntry::setReply(HttpReply *rep)
yadij marked this conversation as resolved.
Show resolved Hide resolved
{
HTTPMSGUNLOCK(reply);
reply = rep;
HTTPMSGLOCK(reply);
}

const char *
AccessLogEntry::getClientIdent() const
{
Expand Down
2 changes: 2 additions & 0 deletions src/AccessLogEntry.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ class AccessLogEntry: public RefCountable

void syncNotes(HttpRequest *request);

void setReply(HttpReply *);

SBuf url;

/// TCP/IP level details about the client connection
Expand Down
2 changes: 2 additions & 0 deletions src/client_side_reply.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1656,6 +1656,8 @@ clientReplyContext::cloneReply()

/* do header conversions */
buildReplyHeader();

http->al->setReply(reply);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This setReply() argument points to a to-be-sent response rather than the (adapted) received response, right? Is that what we want in ALE? Your new ALE::reply member documentation seems to imply that we want the received (and possibly adapted) response instead. Please check and adjust this code or the new data member description.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It points to a 'to-be-sent' response. I fixed the documentation with this terminology.

}

/// Safely disposes of an entry pointing to a cache hit that we do not want.
Expand Down
2 changes: 2 additions & 0 deletions src/clients/FtpRelay.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,8 @@ Ftp::Relay::forwardPreliminaryReply(const PreliminaryCb cb)

const HttpReply::Pointer reply = createHttpReply(Http::scContinue);

fwd->al->setReply(reply.getRaw());

// the Sink will use this to call us back after writing 1xx to the client
typedef NullaryMemFunT<Relay> CbDialer;
const AsyncCall::Pointer call = JobCallback(11, 3, CbDialer, this,
Expand Down
2 changes: 2 additions & 0 deletions src/http.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -787,6 +787,8 @@ HttpStateData::processReplyHeader()
void
HttpStateData::handle1xx(HttpReply *reply)
{
fwd->al->setReply(reply);

HttpReply::Pointer msg(reply); // will destroy reply if unused

// one 1xx at a time: we must not be called while waiting for previous 1xx
Expand Down
6 changes: 0 additions & 6 deletions src/servers/FtpServer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -777,12 +777,6 @@ Ftp::Server::handleReply(HttpReply *reply, StoreIOBuffer data)
Http::StreamPointer context = pipeline.front();
assert(context != nullptr);

if (context->http && context->http->al != NULL &&
!context->http->al->reply && reply) {
context->http->al->reply = reply;
HTTPMSGLOCK(context->http->al->reply);
}

static ReplyHandler handlers[] = {
NULL, // fssBegin
NULL, // fssConnected
Expand Down
2 changes: 0 additions & 2 deletions src/servers/Http1Server.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,8 +312,6 @@ Http::One::Server::handleReply(HttpReply *rep, StoreIOBuffer receivedData)
}

assert(rep);
http->al->reply = rep;
HTTPMSGLOCK(http->al->reply);
context->sendStartOfMessage(rep, receivedData);
}

Expand Down