Re: bug 4906 issue

[Alex Rousskov <rousskov@xxxxxxxxxxxxxxxxxxxxxxx>]

On 2023-09-26 07:39, Matus UHLAR - fantomas wrote:

> I have just encountered bug 4906 with squid-4.13 (Debian 11)
>
> I could upgrade system fo Debian 12 with squid-5.7 but this issue 
> doesn't seem to be resolved in it, at least:
> http://www.squid-cache.org/Versions/v5/changesets/
>
> does not mention that.

We have not (knowingly) fixed bug 4906 yet. Moreover, there are many 
other places in Squid code that would result in problems similar to 
those reported in bug 4906. A comprehensive unofficial fix that 
addresses the root cause got stuck in my unofficial review queue since 
December 2019.


> I could try to test patch provided in that bugreport:
> https://patch-diff.githubusercontent.com/raw/measurement-factory/squid/pull/20.patch
> ...but I find trying such complicated patch too risky.

You can also try a small old unofficial v4.3 workaround patch attached 
to this email. I do not know whether it will apply to and work with 
4.13. If you are lucky, this workaround will address the immediate 
problem in your environment.


HTH,

Alex.

Fix src ACL when access logging TCP probes

TCP probe transactions lack HttpRequest object, which is used when
checking ACLs before logging. For this reason, src ACL did not work. To
fix this, we need to supply ACLFilledChecklist with available address
information.

diff --git a/src/client_side.cc b/src/client_side.cc
index d61e278..368c607 100644
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -407,91 +407,98 @@ ClientHttpRequest::logRequest()
 
     tvSub(al->cache.trTime, al->cache.start_time, current_time);
 
     if (request)
         prepareLogWithRequestDetails(request, al);
 
 #if USE_OPENSSL && 0
 
     /* This is broken. Fails if the connection has been closed. Needs
      * to snarf the ssl details some place earlier..
      */
     if (getConn() != NULL)
         al->cache.ssluser = sslGetUserEmail(fd_table[getConn()->fd].ssl);
 
 #endif
 
     /* Add notes (if we have a request to annotate) */
     if (request) {
         // The al->notes and request->notes must point to the same object.
         (void)SyncNotes(*al, *request);
         for (auto i = Config.notes.begin(); i != Config.notes.end(); ++i) {
             if (const char *value = (*i)->match(request, al->reply, NULL)) {
                 NotePairs &notes = SyncNotes(*al, *request);
                 notes.add((*i)->key.termedBuf(), value);
                 debugs(33, 3, (*i)->key.termedBuf() << " " << value);
             }
         }
     }
 
     ACLFilledChecklist checklist(NULL, request, NULL);
+    const auto clientConn = getConn() ? getConn()->clientConnection : nullptr;
+    if (!request && clientConn) {
+        // because could not obtain them from request
+        checklist.src_addr = clientConn->remote;
+        checklist.my_addr = clientConn->local;
+    }
+
     if (al->reply) {
         checklist.reply = al->reply;
         HTTPMSGLOCK(checklist.reply);
     }
 
     if (request) {
         HTTPMSGUNLOCK(al->adapted_request);
         al->adapted_request = request;
         HTTPMSGLOCK(al->adapted_request);
     }
     // no need checklist.syncAle(): already synced
     checklist.al = al;
     accessLogLog(al, &checklist);
 
     bool updatePerformanceCounters = true;
     if (Config.accessList.stats_collection) {
         ACLFilledChecklist statsCheck(Config.accessList.stats_collection, request, NULL);
         statsCheck.al = al;
         if (al->reply) {
             statsCheck.reply = al->reply;
             HTTPMSGLOCK(statsCheck.reply);
         }
         updatePerformanceCounters = statsCheck.fastCheck().allowed();
     }
 
     if (updatePerformanceCounters) {
         if (request)
             updateCounters();
 
-        if (getConn() != NULL && getConn()->clientConnection != NULL)
-            clientdbUpdate(getConn()->clientConnection->remote, logType, AnyP::PROTO_HTTP, out.size);
+        if (clientConn)
+            clientdbUpdate(clientConn->remote, logType, AnyP::PROTO_HTTP, out.size);
     }
 }
 
 void
 ClientHttpRequest::freeResources()
 {
     safe_free(uri);
     safe_free(redirect.location);
     range_iter.boundary.clean();
     clearRequest();
 
     if (client_stream.tail)
         clientStreamAbort((clientStreamNode *)client_stream.tail->data, this);
 }
 
 void
 httpRequestFree(void *data)
 {
     ClientHttpRequest *http = (ClientHttpRequest *)data;
     assert(http != NULL);
     delete http;
 }
 
 /* This is a handler normally called by comm_close() */
 void ConnStateData::connStateClosed(const CommCloseCbParams &)
 {
     deleteThis("ConnStateData::connStateClosed");
 }
 
 #if USE_AUTH

_______________________________________________
squid-users mailing list
squid-users@xxxxxxxxxxxxxxxxxxxxx
https://lists.squid-cache.org/listinfo/squid-users