]> git.ipfire.org Git - thirdparty/squid.git/blobdiff - src/client_side_reply.cc
Source Format Enforcement (#763)
[thirdparty/squid.git] / src / client_side_reply.cc
index 0c977b26abe06b01b4b1b429509b2784e85d43a6..1512772704ef526b913050eaf457db08016872d0 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 1996-2020 The Squid Software Foundation and contributors
+ * Copyright (C) 1996-2021 The Squid Software Foundation and contributors
  *
  * Squid software is distributed under GPLv2+ license and includes
  * contributions from numerous individuals and organizations.
@@ -53,7 +53,7 @@ CBDATA_CLASS_INIT(clientReplyContext);
 
 /* Local functions */
 extern "C" CSS clientReplyStatus;
-ErrorState *clientBuildError(err_type, Http::StatusCode, char const *, Ip::Address &, HttpRequest *, const AccessLogEntry::Pointer &);
+ErrorState *clientBuildError(err_type, Http::StatusCode, char const *, const ConnStateData *, HttpRequest *, const AccessLogEntry::Pointer &);
 
 /* privates */
 
@@ -74,16 +74,12 @@ clientReplyContext::~clientReplyContext()
 
 clientReplyContext::clientReplyContext(ClientHttpRequest *clientContext) :
     purgeStatus(Http::scNone),
-    lookingforstore(0),
     http(cbdataReference(clientContext)),
     headers_sz(0),
     sc(NULL),
     old_reqsize(0),
     reqsize(0),
     reqofs(0),
-#if USE_CACHE_DIGESTS
-    lookup_type(NULL),
-#endif
     ourNode(NULL),
     reply(NULL),
     old_entry(NULL),
@@ -103,7 +99,7 @@ clientReplyContext::clientReplyContext(ClientHttpRequest *clientContext) :
 void
 clientReplyContext::setReplyToError(
     err_type err, Http::StatusCode status, const HttpRequestMethod& method, char const *uri,
-    Ip::Address &addr, HttpRequest * failedrequest, const char *unparsedrequest,
+    const ConnStateData *conn, HttpRequest *failedrequest, const char *unparsedrequest,
 #if USE_AUTH
     Auth::UserRequest::Pointer auth_user_request
 #else
@@ -111,7 +107,7 @@ clientReplyContext::setReplyToError(
 #endif
 )
 {
-    auto errstate = clientBuildError(err, status, uri, addr, failedrequest, http->al);
+    auto errstate = clientBuildError(err, status, uri, conn, failedrequest, http->al);
 
     if (unparsedrequest)
         errstate->request_hdrs = xstrdup(unparsedrequest);
@@ -377,6 +373,10 @@ clientReplyContext::sendClientUpstreamResponse()
 {
     StoreIOBuffer tempresult;
     removeStoreReference(&old_sc, &old_entry);
+
+    if (collapsedRevalidation)
+        http->storeEntry()->clearPublicKeyScope();
+
     /* here the data to send is the data we just received */
     tempBuffer.offset = 0;
     old_reqsize = 0;
@@ -447,12 +447,16 @@ clientReplyContext::handleIMSReply(StoreIOBuffer result)
         debugs(88, 3, "request to origin aborted '" << http->storeEntry()->url() << "', sending old entry to client");
         http->logType.update(LOG_TCP_REFRESH_FAIL_OLD);
         sendClientOldEntry();
+        return;
     }
 
     const auto oldStatus = old_entry->mem().freshestReply().sline.status();
     const auto &new_rep = http->storeEntry()->mem().freshestReply();
     const auto status = new_rep.sline.status();
 
+    // XXX: Disregard stale incomplete (i.e. still being written) borrowed (i.e.
+    // not caused by our request) IMS responses. That new_rep may be very old!
+
     // origin replied 304
     if (status == Http::scNotModified) {
         http->logType.update(LOG_TCP_REFRESH_UNMODIFIED);
@@ -463,54 +467,48 @@ clientReplyContext::handleIMSReply(StoreIOBuffer result)
         Store::Root().updateOnNotModified(old_entry, *http->storeEntry());
 
         // if client sent IMS
-
         if (http->request->flags.ims && !old_entry->modifiedSince(http->request->ims, http->request->imslen)) {
             // forward the 304 from origin
-            debugs(88, 3, "origin replied 304, revalidating existing entry and forwarding 304 to client");
+            debugs(88, 3, "origin replied 304, revalidated existing entry and forwarding 304 to client");
             sendClientUpstreamResponse();
-        } else {
-            // send existing entry, it's still valid
-            debugs(88, 3, "origin replied 304, revalidating existing entry and sending " <<
-                   oldStatus << " to client");
-            sendClientOldEntry();
+            return;
         }
+
+        // send existing entry, it's still valid
+        debugs(88, 3, "origin replied 304, revalidated existing entry and sending " << oldStatus << " to client");
+        sendClientOldEntry();
+        return;
     }
 
     // origin replied with a non-error code
-    else if (status > Http::scNone && status < Http::scInternalServerError) {
+    if (status > Http::scNone && status < Http::scInternalServerError) {
         // RFC 7234 section 4: a cache MUST use the most recent response
         // (as determined by the Date header field)
         if (new_rep.olderThan(&old_entry->mem().freshestReply())) {
             http->logType.err.ignored = true;
-            debugs(88, 3, "origin replied " << status <<
-                   " but with an older date header, sending old entry (" <<
-                   oldStatus << ") to client");
+            debugs(88, 3, "origin replied " << status << " but with an older date header, sending old entry (" << oldStatus << ") to client");
             sendClientOldEntry();
-        } else {
-            http->logType.update(LOG_TCP_REFRESH_MODIFIED);
-            debugs(88, 3, "origin replied " << status <<
-                   ", replacing existing entry and forwarding to client");
-
-            if (collapsedRevalidation)
-                http->storeEntry()->clearPublicKeyScope();
-
-            sendClientUpstreamResponse();
+            return;
         }
+
+        http->logType.update(LOG_TCP_REFRESH_MODIFIED);
+        debugs(88, 3, "origin replied " << status << ", forwarding to client");
+        sendClientUpstreamResponse();
+        return;
     }
 
     // origin replied with an error
-    else if (http->request->flags.failOnValidationError) {
+    if (http->request->flags.failOnValidationError) {
         http->logType.update(LOG_TCP_REFRESH_FAIL_ERR);
-        debugs(88, 3, "origin replied with error " << status <<
-               ", forwarding to client due to fail_on_validation_err");
+        debugs(88, 3, "origin replied with error " << status << ", forwarding to client due to fail_on_validation_err");
         sendClientUpstreamResponse();
-    } else {
-        // ignore and let client have old entry
-        http->logType.update(LOG_TCP_REFRESH_FAIL_OLD);
-        debugs(88, 3, "origin replied with error " <<
-               status << ", sending old entry (" << oldStatus << ") to client");
-        sendClientOldEntry();
+        return;
     }
+
+    // ignore and let client have old entry
+    http->logType.update(LOG_TCP_REFRESH_FAIL_OLD);
+    debugs(88, 3, "origin replied with error " << status << ", sending old entry (" << oldStatus << ") to client");
+    sendClientOldEntry();
 }
 
 SQUIDCEXTERN CSR clientGetMoreData;
@@ -761,13 +759,10 @@ clientReplyContext::processMiss()
         return;
     }
 
-    Comm::ConnectionPointer conn = http->getConn() != nullptr ? http->getConn()->clientConnection : nullptr;
     /// Deny loops
     if (r->flags.loopDetected) {
         http->al->http.code = Http::scForbidden;
-        Ip::Address tmp_noaddr;
-        tmp_noaddr.setNoAddr();
-        err = clientBuildError(ERR_ACCESS_DENIED, Http::scForbidden, nullptr, conn ? conn->remote : tmp_noaddr, http->request, http->al);
+        err = clientBuildError(ERR_ACCESS_DENIED, Http::scForbidden, nullptr, http->getConn(), http->request, http->al);
         createStoreEntry(r->method, RequestFlags());
         errorAppendEntry(http->storeEntry(), err);
         triggerInitialStoreRead();
@@ -789,6 +784,7 @@ clientReplyContext::processMiss()
 
         assert(r->clientConnectionManager == http->getConn());
 
+        Comm::ConnectionPointer conn = http->getConn() != nullptr ? http->getConn()->clientConnection : nullptr;
         /** Start forwarding to get the new object from network */
         FwdState::Start(conn, http->storeEntry(), r, http->al);
     }
@@ -805,11 +801,8 @@ clientReplyContext::processOnlyIfCachedMiss()
 {
     debugs(88, 4, http->request->method << ' ' << http->uri);
     http->al->http.code = Http::scGatewayTimeout;
-    Ip::Address tmp_noaddr;
-    tmp_noaddr.setNoAddr();
     ErrorState *err = clientBuildError(ERR_ONLY_IF_CACHED_MISS, Http::scGatewayTimeout, NULL,
-                                       http->getConn() ? http->getConn()->clientConnection->remote : tmp_noaddr,
-                                       http->request, http->al);
+                                       http->getConn(), http->request, http->al);
     removeClientStoreReference(&sc, http);
     startError(err);
 }
@@ -887,18 +880,6 @@ clientReplyContext::blockedHit() const
     }
 }
 
-void
-clientReplyContext::purgeRequestFindObjectToPurge()
-{
-    /* Try to find a base entry */
-    http->flags.purging = true;
-    lookingforstore = 1;
-
-    // TODO: can we use purgeAllCached() here instead of doing the
-    // getPublicByRequestMethod() dance?
-    StoreEntry::getPublicByRequestMethod(this, http->request, Http::METHOD_GET);
-}
-
 // Purges all entries with a given url
 // TODO: move to SideAgent parent, when we have one
 /*
@@ -928,23 +909,8 @@ clientReplyContext::purgeAllCached()
     purgeEntriesByUrl(http->request, url.c_str());
 }
 
-void
-clientReplyContext::created(StoreEntry *newEntry)
-{
-    if (lookingforstore == 1)
-        purgeFoundGet(newEntry);
-    else if (lookingforstore == 2)
-        purgeFoundHead(newEntry);
-    else if (lookingforstore == 3)
-        purgeDoPurgeGet(newEntry);
-    else if (lookingforstore == 4)
-        purgeDoPurgeHead(newEntry);
-    else if (lookingforstore == 5)
-        identifyFoundObject(newEntry);
-}
-
 LogTags *
-clientReplyContext::loggingTags()
+clientReplyContext::loggingTags() const
 {
     // XXX: clientReplyContext code assumes that http cbdata is always valid.
     // TODO: Either add cbdataReferenceValid(http) checks in all the relevant
@@ -952,65 +918,6 @@ clientReplyContext::loggingTags()
     return &http->logType;
 }
 
-void
-clientReplyContext::purgeFoundGet(StoreEntry *newEntry)
-{
-    if (!newEntry) {
-        lookingforstore = 2;
-        StoreEntry::getPublicByRequestMethod(this, http->request, Http::METHOD_HEAD);
-    } else
-        purgeFoundObject (newEntry);
-}
-
-void
-clientReplyContext::purgeFoundHead(StoreEntry *newEntry)
-{
-    if (!newEntry)
-        purgeDoMissPurge();
-    else
-        purgeFoundObject (newEntry);
-}
-
-void
-clientReplyContext::purgeFoundObject(StoreEntry *entry)
-{
-    assert (entry);
-
-    if (EBIT_TEST(entry->flags, ENTRY_SPECIAL)) {
-        http->logType.update(LOG_TCP_DENIED);
-        Ip::Address tmp_noaddr;
-        tmp_noaddr.setNoAddr(); // TODO: make a global const
-        ErrorState *err = clientBuildError(ERR_ACCESS_DENIED, Http::scForbidden, NULL,
-                                           http->getConn() ? http->getConn()->clientConnection->remote : tmp_noaddr,
-                                           http->request, http->al);
-        startError(err);
-        return; // XXX: leaking unused entry if some store does not keep it
-    }
-
-    StoreIOBuffer localTempBuffer;
-    /* Swap in the metadata */
-    http->storeEntry(entry);
-
-    http->storeEntry()->lock("clientReplyContext::purgeFoundObject");
-    http->storeEntry()->ensureMemObject(storeId(), http->log_uri,
-                                        http->request->method);
-
-    sc = storeClientListAdd(http->storeEntry(), this);
-
-    http->logType.update(LOG_TCP_HIT);
-
-    reqofs = 0;
-
-    localTempBuffer.offset = http->out.offset;
-
-    localTempBuffer.length = next()->readBuffer.length;
-
-    localTempBuffer.data = next()->readBuffer.data;
-
-    storeClientCopy(sc, http->storeEntry(),
-                    localTempBuffer, CacheHit, this);
-}
-
 void
 clientReplyContext::purgeRequest()
 {
@@ -1019,10 +926,8 @@ clientReplyContext::purgeRequest()
 
     if (!Config2.onoff.enable_purge) {
         http->logType.update(LOG_TCP_DENIED);
-        Ip::Address tmp_noaddr;
-        tmp_noaddr.setNoAddr();
         ErrorState *err = clientBuildError(ERR_ACCESS_DENIED, Http::scForbidden, NULL,
-                                           http->getConn() ? http->getConn()->clientConnection->remote : tmp_noaddr, http->request, http->al);
+                                           http->getConn(), http->request, http->al);
         startError(err);
         return;
     }
@@ -1030,47 +935,34 @@ clientReplyContext::purgeRequest()
     /* Release both IP cache */
     ipcacheInvalidate(http->request->url.host());
 
-    if (!http->flags.purging)
-        purgeRequestFindObjectToPurge();
-    else
-        purgeDoMissPurge();
+    // TODO: can we use purgeAllCached() here instead?
+    purgeDoPurge();
 }
 
 void
-clientReplyContext::purgeDoMissPurge()
-{
-    http->logType.update(LOG_TCP_MISS);
-    lookingforstore = 3;
-    StoreEntry::getPublicByRequestMethod(this,http->request, Http::METHOD_GET);
-}
-
-void
-clientReplyContext::purgeDoPurgeGet(StoreEntry *newEntry)
-{
-    if (newEntry) {
-        /* Release the cached URI */
-        debugs(88, 4, "clientPurgeRequest: GET '" << newEntry->url() << "'" );
-#if USE_HTCP
-        neighborsHtcpClear(newEntry, http->request, HttpRequestMethod(Http::METHOD_GET), HTCP_CLR_PURGE);
-#endif
-        newEntry->release(true);
-        purgeStatus = Http::scOkay;
+clientReplyContext::purgeDoPurge()
+{
+    auto firstFound = false;
+    if (const auto entry = storeGetPublicByRequestMethod(http->request, Http::METHOD_GET)) {
+        // special entries are only METHOD_GET entries without variance
+        if (EBIT_TEST(entry->flags, ENTRY_SPECIAL)) {
+            http->logType.update(LOG_TCP_DENIED);
+            const auto err = clientBuildError(ERR_ACCESS_DENIED, Http::scForbidden, nullptr,
+                                              http->getConn(), http->request, http->al);
+            startError(err);
+            entry->abandon(__FUNCTION__);
+            return;
+        }
+        firstFound = true;
+        if (!purgeEntry(*entry, Http::METHOD_GET))
+            return;
     }
 
-    lookingforstore = 4;
-    StoreEntry::getPublicByRequestMethod(this, http->request, Http::METHOD_HEAD);
-}
+    detailStoreLookup(storeLookupString(firstFound));
 
-void
-clientReplyContext::purgeDoPurgeHead(StoreEntry *newEntry)
-{
-    if (newEntry) {
-        debugs(88, 4, "HEAD " << newEntry->url());
-#if USE_HTCP
-        neighborsHtcpClear(newEntry, http->request, HttpRequestMethod(Http::METHOD_HEAD), HTCP_CLR_PURGE);
-#endif
-        newEntry->release(true);
-        purgeStatus = Http::scOkay;
+    if (const auto entry = storeGetPublicByRequestMethod(http->request, Http::METHOD_HEAD)) {
+        if (!purgeEntry(*entry, Http::METHOD_HEAD))
+            return;
     }
 
     /* And for Vary, release the base URI if none of the headers was included in the request */
@@ -1078,26 +970,15 @@ clientReplyContext::purgeDoPurgeHead(StoreEntry *newEntry)
             && http->request->vary_headers.find('=') != SBuf::npos) {
         // XXX: performance regression, c_str() reallocates
         SBuf tmp(http->request->effectiveRequestUri());
-        StoreEntry *entry = storeGetPublic(tmp.c_str(), Http::METHOD_GET);
 
-        if (entry) {
-            debugs(88, 4, "Vary GET " << entry->url());
-#if USE_HTCP
-            neighborsHtcpClear(entry, http->request, HttpRequestMethod(Http::METHOD_GET), HTCP_CLR_PURGE);
-#endif
-            entry->release(true);
-            purgeStatus = Http::scOkay;
+        if (const auto entry = storeGetPublic(tmp.c_str(), Http::METHOD_GET)) {
+            if (!purgeEntry(*entry, Http::METHOD_GET, "Vary "))
+                return;
         }
 
-        entry = storeGetPublic(tmp.c_str(), Http::METHOD_HEAD);
-
-        if (entry) {
-            debugs(88, 4, "Vary HEAD " << entry->url());
-#if USE_HTCP
-            neighborsHtcpClear(entry, http->request, HttpRequestMethod(Http::METHOD_HEAD), HTCP_CLR_PURGE);
-#endif
-            entry->release(true);
-            purgeStatus = Http::scOkay;
+        if (const auto entry = storeGetPublic(tmp.c_str(), Http::METHOD_HEAD)) {
+            if (!purgeEntry(*entry, Http::METHOD_HEAD, "Vary "))
+                return;
         }
     }
 
@@ -1121,6 +1002,18 @@ clientReplyContext::purgeDoPurgeHead(StoreEntry *newEntry)
     http->storeEntry()->complete();
 }
 
+bool
+clientReplyContext::purgeEntry(StoreEntry &entry, const Http::MethodType methodType, const char *descriptionPrefix)
+{
+    debugs(88, 4, descriptionPrefix << Http::MethodStr(methodType) << " '" << entry.url() << "'" );
+#if USE_HTCP
+    neighborsHtcpClear(&entry, http->request, HttpRequestMethod(methodType), HTCP_CLR_PURGE);
+#endif
+    entry.release(true);
+    purgeStatus = Http::scOkay;
+    return true;
+}
+
 void
 clientReplyContext::traceReply(clientStreamNode * node)
 {
@@ -1559,20 +1452,18 @@ clientReplyContext::buildReplyHeader()
         Auth::UserRequest::AddReplyAuthHeader(reply, request->auth_user_request, request, http->flags.accel, 0);
 #endif
 
-    /* Append X-Cache */
-    httpHeaderPutStrf(hdr, Http::HdrType::X_CACHE, "%s from %s",
-                      is_hit ? "HIT" : "MISS", getMyHostname());
-
-#if USE_CACHE_DIGESTS
-    /* Append X-Cache-Lookup: -- temporary hack, to be removed @?@ @?@ */
-    httpHeaderPutStrf(hdr, Http::HdrType::X_CACHE_LOOKUP, "%s from %s:%d",
-                      lookup_type ? lookup_type : "NONE",
-                      getMyHostname(), getMyPort());
-
-#endif
+    SBuf cacheStatus(uniqueHostname());
+    if (const auto hitOrFwd = http->logType.cacheStatusSource())
+        cacheStatus.append(hitOrFwd);
+    if (firstStoreLookup_) {
+        cacheStatus.append(";detail=");
+        cacheStatus.append(firstStoreLookup_);
+    }
+    // TODO: Remove c_str() after converting HttpHeaderEntry::value to SBuf
+    hdr->putStr(Http::HdrType::CACHE_STATUS, cacheStatus.c_str());
 
     const bool maySendChunkedReply = !request->multipartRangeRequest() &&
-                                     reply->sline.protocol == AnyP::PROTO_HTTP && // response is HTTP
+                                     reply->sline.version.protocol == AnyP::PROTO_HTTP && // response is HTTP
                                      (request->http_ver >= Http::ProtocolVersion(1,1));
 
     /* Check whether we should send keep-alive */
@@ -1656,7 +1547,7 @@ clientReplyContext::cloneReply()
 
     http->al->reply = reply;
 
-    if (reply->sline.protocol == AnyP::PROTO_HTTP) {
+    if (reply->sline.version.protocol == AnyP::PROTO_HTTP) {
         /* RFC 2616 requires us to advertise our version (but only on real HTTP traffic) */
         reply->sline.version = Http::ProtocolVersion();
     }
@@ -1690,10 +1581,11 @@ clientReplyContext::identifyStoreObject()
     // client sent CC:no-cache or some other condition has been
     // encountered which prevents delivering a public/cached object.
     if (!r->flags.noCache || r->flags.internal) {
-        lookingforstore = 5;
-        StoreEntry::getPublicByRequest (this, r);
+        const auto e = storeGetPublicByRequest(r);
+        identifyFoundObject(e, storeLookupString(bool(e)));
     } else {
-        identifyFoundObject(nullptr);
+        // "external" no-cache requests skip Store lookups
+        identifyFoundObject(nullptr, "no-cache");
     }
 }
 
@@ -1702,8 +1594,10 @@ clientReplyContext::identifyStoreObject()
  * to see if we can determine the final status of the request.
  */
 void
-clientReplyContext::identifyFoundObject(StoreEntry *newEntry)
+clientReplyContext::identifyFoundObject(StoreEntry *newEntry, const char *detail)
 {
+    detailStoreLookup(detail);
+
     HttpRequest *r = http->request;
     http->storeEntry(newEntry);
     const auto e = http->storeEntry();
@@ -1715,10 +1609,6 @@ clientReplyContext::identifyFoundObject(StoreEntry *newEntry)
     if (r->flags.noCache || r->flags.noCacheHack())
         ipcacheInvalidateNegative(r->url.host());
 
-#if USE_CACHE_DIGESTS
-    lookup_type = e ? "HIT" : "MISS";
-#endif
-
     if (!e) {
         /** \li If no StoreEntry object is current assume this object isn't in the cache set MISS*/
         debugs(85, 3, "StoreEntry is NULL -  MISS");
@@ -1781,6 +1671,18 @@ clientReplyContext::identifyFoundObject(StoreEntry *newEntry)
     doGetMoreData();
 }
 
+/// remembers the very first Store lookup classification, ignoring the rest
+void
+clientReplyContext::detailStoreLookup(const char *detail)
+{
+    if (!firstStoreLookup_) {
+        debugs(85, 7, detail);
+        firstStoreLookup_ = detail;
+    } else {
+        debugs(85, 7, "ignores " << detail << " after " << firstStoreLookup_);
+    }
+}
+
 /**
  * Request more data from the store for the client Stream
  * This is *the* entry point to this module.
@@ -1967,12 +1869,9 @@ clientReplyContext::next() const
 void
 clientReplyContext::sendBodyTooLargeError()
 {
-    Ip::Address tmp_noaddr;
-    tmp_noaddr.setNoAddr(); // TODO: make a global const
     http->logType.update(LOG_TCP_DENIED_REPLY);
     ErrorState *err = clientBuildError(ERR_TOO_BIG, Http::scForbidden, NULL,
-                                       http->getConn() != NULL ? http->getConn()->clientConnection->remote : tmp_noaddr,
-                                       http->request, http->al);
+                                       http->getConn(), http->request, http->al);
     removeClientStoreReference(&(sc), http);
     HTTPMSGUNLOCK(reply);
     startError(err);
@@ -1984,11 +1883,9 @@ void
 clientReplyContext::sendPreconditionFailedError()
 {
     http->logType.update(LOG_TCP_HIT);
-    Ip::Address tmp_noaddr;
-    tmp_noaddr.setNoAddr();
     ErrorState *const err =
         clientBuildError(ERR_PRECONDITION_FAILED, Http::scPreconditionFailed,
-                         NULL, http->getConn() ? http->getConn()->clientConnection->remote : tmp_noaddr, http->request, http->al);
+                         nullptr, http->getConn(), http->request, http->al);
     removeClientStoreReference(&sc, http);
     HTTPMSGUNLOCK(reply);
     startError(err);
@@ -2097,11 +1994,8 @@ clientReplyContext::processReplyAccessResult(const Acl::Answer &accessAllowed)
         if (page_id == ERR_NONE)
             page_id = ERR_ACCESS_DENIED;
 
-        Ip::Address tmp_noaddr;
-        tmp_noaddr.setNoAddr();
         err = clientBuildError(page_id, Http::scForbidden, NULL,
-                               http->getConn() != NULL ? http->getConn()->clientConnection->remote : tmp_noaddr,
-                               http->request, http->al);
+                               http->getConn(), http->request, http->al);
 
         removeClientStoreReference(&sc, http);
 
@@ -2332,10 +2226,10 @@ clientReplyContext::createStoreEntry(const HttpRequestMethod& m, RequestFlags re
 
 ErrorState *
 clientBuildError(err_type page_id, Http::StatusCode status, char const *url,
-                 Ip::Address &src_addr, HttpRequest * request, const AccessLogEntry::Pointer &al)
+                 const ConnStateData *conn, HttpRequest *request, const AccessLogEntry::Pointer &al)
 {
     const auto err = new ErrorState(page_id, status, request, al);
-    err->src_addr = src_addr;
+    err->src_addr = conn && conn->clientConnection ? conn->clientConnection->remote : Ip::Address::NoAddr();
 
     if (url)
         err->url = xstrdup(url);