Merge branch 'master' into arc-autoconf-ng-13-b

.github/workflows/default.yaml

@@ -159,6 +159,21 @@ jobs:
           export CPPFLAGS="-I$HOMEBREW_PREFIX/include${CPPFLAGS:+ $CPPFLAGS}"
           export LDFLAGS="-L$HOMEBREW_PREFIX/lib${LDFLAGS:+ $LDFLAGS}"
           export CFLAGS="-Wno-compound-token-split-by-macro${CFLAGS:+ $CFLAGS}" # needed fir ltdl with Xcode
+
+          # libtool package referenced below fails to copy its configure*
+          # files, possibly due to a packaging/brewing bug. The following sed
+          # command restores installed libtoolize code to its earlier (and
+          # working) variation.
+          echo "brew libtool package details:"
+          brew info libtool --json | grep -E 'rebuild|tap_git_head'
+          # This hack was tested on libtoolize package with the following output:
+          # "rebuild": 2,
+          # "tap_git_head": "5cede8ea3b7b12c7f68215f75a951430b38d945f",
+          #
+          editable=$HOMEBREW_CELLAR/libtool/2.4.7/bin/glibtoolize
+          sed -i.bak 's@ltdl_ac_aux_dir=""@ltdl_ac_aux_dir="../build-aux"@' $editable || true
+          diff -u $editable.bak $editable || true
+
           ./test-builds.sh ${{ matrix.layer.name }}
 
       - name: Publish build logs

Makefile.am

@@ -7,13 +7,23 @@
 
 AUTOMAKE_OPTIONS = dist-bzip2 1.5 foreign
 
-SUBDIRS = compat contrib doc errors icons
+SUBDIRS = \
+	compat \
+	contrib \
+	doc \
+	errors \
+	icons
 
 if ENABLE_LOADABLE_MODULES
 SUBDIRS += libltdl
 endif
 
-SUBDIRS += lib scripts src tools test-suite
+SUBDIRS += \
+	lib \
+	scripts \
+	src \
+	tools \
+	test-suite
 
 DISTCLEANFILES = include/stamp-h include/stamp-h[0-9]*
 DEFAULT_PINGER = $(libexecdir)/`echo pinger | sed '$(transform);s/$$/$(EXEEXT)/'`

acinclude/squid-util.m4

@@ -287,22 +287,18 @@ AC_DEFUN([SQUID_EMBED_BUILD_INFO],[
     AS_CASE(["$enableval"],
       [no],[:],
       [yes],[
-        AS_IF([test -d "${srcdir}/.bzr"],[
-          AC_PATH_PROG(BZR,bzr,$FALSE)
-          squid_bzr_branch_nick=`cd ${srcdir} && ${BZR} nick 2>/dev/null`
-          AS_IF([test $? -eq 0 -a "x$squid_bzr_branch_nick" != "x"],[
-            squid_bzr_branch_revno=`cd ${srcdir} && ${BZR} revno 2>/dev/null | sed 's/\"//g'`
-          ])
-          AS_IF([test $? -eq 0 -a "x$squid_bzr_branch_revno" != "x"],[
-            sh -c "cd ${srcdir} && ${BZR} diff 2>&1 >/dev/null"
-            AS_IF([test $? -eq 1],[
-              squid_bzr_branch_revno="$squid_bzr_branch_revno+changes"
+        AC_PATH_PROG(GIT,git,$FALSE)
+        AS_IF([test "x$GIT" != "x$FALSE"],[
+          squid_git_branch="`cd ${srcdir} && ${GIT} branch --show-current 2>/dev/null`"
+          squid_git_revno="`cd ${srcdir} && ${GIT} rev-parse --short HEAD 2>/dev/null`"
+          AS_IF([test "x$squid_git_branch" != "x"], [:], [squid_git_branch="unknown"])
+          AS_IF([test "x$squid_git_revno" != "x"],[
+            AS_IF([cd ${srcdir} && ! ${GIT} diff --quiet HEAD],[ # there are uncommitted changes
+              squid_git_revno="$squid_git_revno plus changes"
             ])
           ])
-          AS_IF([test "x$squid_bzr_branch_revno" != "x"],[
-            squid_build_info="Built branch: ${squid_bzr_branch_nick}-r${squid_bzr_branch_revno}"
-          ])
         ])
+        squid_build_info="Git: branch ${squid_git_branch:-unavailable} revision ${squid_git_revno:-unavailable}"
       ],
       [squid_build_info=$enableval]
     )

bootstrap.sh

@@ -116,38 +116,47 @@ echo "autoconf ($acversion) : autoconf$acver"
 echo "libtool  ($ltversion) : ${LIBTOOL_BIN}${ltver}"
 echo "libtool path : $ltpath"
 
-for dir in \
-	""
-do
-    if [ -z "$dir" ] || [ -d $dir ]; then
-	if (
-	echo "Bootstrapping $dir"
-	cd ./$dir
-	if [ -n "$dir" ] && [ -f bootstrap.sh ]; then
-	    ./bootstrap.sh
-	elif [ ! -f $dir/configure ]; then
-	    # Make sure cfgaux exists
-	    mkdir -p cfgaux
-
-            if test -n "$ltpath"; then
-              acincludeflag="-I $ltpath/../share/aclocal"
-            else
-              acincludeflag=""
-            fi
-
-	    # Bootstrap the autotool subsystems
-	    bootstrap aclocal$amver $acincludeflag
-	    bootstrap autoheader$acver
-	    bootstrap_libtoolize ${LIBTOOL_BIN}ize${ltver}
-	    bootstrap automake$amver --foreign --add-missing --copy -f
-	    bootstrap autoconf$acver --force
-	fi ); then
-	    : # OK
-	else
-	    exit 1
-	fi
+if test -n "$ltpath"; then
+    acincludeflag="-I $ltpath/../share/aclocal"
+else
+    acincludeflag=""
+fi
+
+# bootstrap primary or subproject sources
+bootstrap_dir() {
+    dir="$1"
+    cd $dir || exit $?
+
+    bootstrap aclocal$amver $acincludeflag
+    bootstrap autoheader$acver
+
+    # Do not libtoolize ltdl
+    if grep -q '^LTDL_INIT' configure.ac
+    then
+        bootstrap_libtoolize ${LIBTOOL_BIN}ize${ltver}
     fi
-done
+
+    bootstrap automake$amver --foreign --add-missing --copy --force
+    bootstrap autoconf$acver --force
+
+    cd - > /dev/null
+}
+
+echo "Bootstrapping primary Squid sources"
+mkdir -p cfgaux || exit $?
+bootstrap_dir .
+
+# The above bootstrap_libtoolize step creates or updates libltdl. It copies
+# (with minor adjustments) configure.ac and configure, Makefile.am and
+# Makefile.in from libtool installation, but does not regenerate copied
+# configure from copied configure.ac and copied Makefile.in from Makefile.am.
+# We get libltdl/configure and libltdl/Makefile.in as they were bootstrapped
+# by libtool authors or package maintainers. Low-level idiosyncrasies in those
+# libtool files result in mismatches between copied code expectations and
+# Squid sub-project environment, leading to occasional build failures that
+# this bootstrapping addresses.
+echo "Bootstrapping libltdl sub-project"
+bootstrap_dir libltdl
 
 # Make a copy of SPONSORS we can package
 if test -f SPONSORS.list; then

doc/debug-sections.txt

@@ -130,6 +130,7 @@ section 80    WCCP Support
 section 81    Store HEAP Removal Policies
 section 81    aio_xxx() POSIX emulation on Windows
 section 82    External ACL
+section 82    External ACL Helpers
 section 83    SSL accelerator support
 section 83    SSL-Bump Server/Peer negotiation
 section 83    TLS I/O

doc/release-notes/release-7.sgml.in

@@ -286,6 +286,15 @@ This section gives an account of those changes in three categories:
 
 </descrip>
 
+<sect1>Other changes<label id="otherchanges">
+<p>
+<descrip>
+	<tag>Adjusted configuration and format of ext_time_quota_acl helper debugging</tag>
+	<p>The <em>-l</em> option that enables <em>ext_time_quota_acl</em> to log debug messages
+		to a custom logfile has been removed, and their format has been
+		changed to be in line with Squid's cache.log format.
+</descrip>
+
 <sect>Copyright
 <p>
 Copyright (C) 1996-2023 The Squid Software Foundation and contributors

lib/Makefile.am

@@ -54,7 +54,6 @@ libmisccontainers_la_SOURCES = \
 	hash.cc
 
 libmiscutil_la_SOURCES = \
-	$(SNPRINTFSOURCE) \
 	Splay.cc \
 	heap.c \
 	radix.c \

scripts/source-maintenance.sh

@@ -291,7 +291,7 @@ checkMakeNamedErrorDetails ()
     problems=1 # assume there are problems until proven otherwise
 
     options='-h --only-matching --extended-regexp'
-    git grep $options 'MakeNamedErrorDetail[(]".*?"[)]' src |
+    git grep $options 'MakeNamedErrorDetail[(]"[^"]*"[)]' src |
         sort |
         uniq --count > \
         MakeNamedErrorDetail.tmp

src/CachePeer.cc

@@ -22,7 +22,8 @@ CBDATA_CLASS_INIT(CachePeer);
 
 CachePeer::CachePeer(const char * const hostname):
     name(xstrdup(hostname)),
-    host(xstrdup(hostname))
+    host(xstrdup(hostname)),
+    tlsContext(secure, sslContext)
 {
     Tolower(host); // but .name preserves original spelling
 }
@@ -55,6 +56,14 @@ CachePeer::~CachePeer()
     xfree(domain);
 }
 
+Security::FuturePeerContext *
+CachePeer::securityContext()
+{
+    if (secure.encryptTransport)
+        return &tlsContext;
+    return nullptr;
+}
+
 void
 CachePeer::noteSuccess()
 {

src/CachePeer.h

@@ -44,6 +44,10 @@ class CachePeer
     /// \returns the effective connect timeout for the given peer
     time_t connectTimeout() const;
 
+    /// TLS settings for communicating with this TLS cache_peer (if encryption
+    /// is required; see secure.encryptTransport) or nil (otherwise)
+    Security::FuturePeerContext *securityContext();
+
     /// n-th cache_peer directive, starting with 1
     u_int index = 0;
 
@@ -209,9 +213,12 @@ class CachePeer
 
     char *domain = nullptr; ///< Forced domain
 
+    // TODO: Remove secure and sslContext when FuturePeerContext below becomes PeerContext
     /// security settings for peer connection
     Security::PeerOptions secure;
     Security::ContextPointer sslContext;
+    Security::FuturePeerContext tlsContext;
+
     Security::SessionStatePointer sslSession;
 
     int front_end_https = 0; ///< 0 - off, 1 - on, 2 - auto

src/ConfigParser.cc

@@ -594,13 +594,13 @@ ConfigParser::skipOptional(const char *keyword)
     return false; // no more tokens (i.e. we are at the end of the line)
 }
 
-Acl::Tree *
+ACLList *
 ConfigParser::optionalAclList()
 {
     if (!skipOptional("if"))
         return nullptr; // OK: the directive has no ACLs
 
-    Acl::Tree *acls = nullptr;
+    ACLList *acls = nullptr;
     const auto aclCount = aclParseAclList(*this, &acls, cfg_directive);
     assert(acls);
     if (aclCount <= 0)

src/ConfigParser.h

@@ -72,7 +72,7 @@ class ConfigParser
     bool skipOptional(const char *keyword);
 
     /// parses an [if [!]<acl>...] construct
-    Acl::Tree *optionalAclList();
+    ACLList *optionalAclList();
 
     /// extracts and returns a regex (including any optional flags)
     std::unique_ptr<RegexPattern> regex(const char *expectedRegexDescription);

src/FwdState.cc

@@ -128,7 +128,8 @@ FwdState::FwdState(const Comm::ConnectionPointer &client, StoreEntry * e, HttpRe
     waitingForDispatched(false),
     destinations(new ResolvedPeers()),
     pconnRace(raceImpossible),
-    storedWholeReply_(nullptr)
+    storedWholeReply_(nullptr),
+    peeringTimer(r)
 {
     debugs(17, 2, "Forwarding client request " << client << ", url=" << e->url());
     HTTPMSGLOCK(request);
@@ -186,6 +187,8 @@ FwdState::stopAndDestroy(const char *reason)
 
     cancelStep(reason);
 
+    peeringTimer.stop();
+
     PeerSelectionInitiator::subscribed = false; // may already be false
     self = nullptr; // we hope refcounting destroys us soon; may already be nil
     /* do not place any code here as this object may be gone by now */
@@ -259,8 +262,6 @@ FwdState::completed()
 
     flags.forward_completed = true;
 
-    request->hier.stopPeerClock(false);
-
     if (EBIT_TEST(entry->flags, ENTRY_ABORTED)) {
         debugs(17, 3, "entry aborted");
         return ;
@@ -780,8 +781,6 @@ FwdState::retryOrBail()
     // TODO: should we call completed() here and move doneWithRetries there?
     doneWithRetries();
 
-    request->hier.stopPeerClock(false);
-
     if (self != nullptr && !err && shutting_down && entry->isEmpty()) {
         const auto anErr = new ErrorState(ERR_SHUTTING_DOWN, Http::scServiceUnavailable, request, al);
         errorAppendEntry(entry, anErr);
@@ -1128,8 +1127,6 @@ FwdState::connectStart()
     err = nullptr;
     request->clearError();
 
-    request->hier.startPeerClock();
-
     const auto callback = asyncCallback(17, 5, FwdState::noteConnection, this);
     HttpRequest::Pointer cause = request;
     const auto cs = new HappyConnOpener(destinations, callback, cause, start_t, n_tries, al);
@@ -1572,3 +1569,25 @@ ResetMarkingsToServer(HttpRequest * request, Comm::Connection &conn)
         Ip::Qos::setSockNfmark(&conn, conn.nfmark);
 }
 
+/* PeeringActivityTimer */
+
+// The simple methods below are not inlined to avoid exposing some of the
+// current FwdState.h users to a full HttpRequest definition they do not need.
+
+PeeringActivityTimer::PeeringActivityTimer(const HttpRequestPointer &r): request(r)
+{
+    Assure(request);
+    timer().resume();
+}
+
+PeeringActivityTimer::~PeeringActivityTimer()
+{
+    stop();
+}
+
+Stopwatch &
+PeeringActivityTimer::timer()
+{
+    return request->hier.totalPeeringTime;
+}
+

src/FwdState.h

@@ -49,6 +49,40 @@ void ResetMarkingsToServer(HttpRequest *, Comm::Connection &);
 
 class HelperReply;
 
+/// Eliminates excessive Stopwatch pause() calls in a task with multiple code
+/// locations that pause a stopwatch. Ideally, there would be just one such
+/// location (e.g., a task class destructor), but current code idiosyncrasies
+/// necessitate this state. For simplicity sake, this class currently manages a
+/// Stopwatch at a hard-coded location: HttpRequest::hier.totalPeeringTime.
+class PeeringActivityTimer
+{
+public:
+    PeeringActivityTimer(const HttpRequestPointer &); ///< resumes timer
+    ~PeeringActivityTimer(); ///< \copydoc stop()
+
+    /// pauses timer if stop() has not been called
+    void stop()
+    {
+        if (!stopped) {
+            timer().pause();
+            stopped = true;
+        }
+    }
+
+private:
+    /// managed Stopwatch object within HierarchyLogEntry
+    Stopwatch &timer();
+
+    /// the owner of managed HierarchyLogEntry
+    HttpRequestPointer request;
+
+    // We cannot rely on timer().ran(): This class eliminates excessive calls
+    // within a single task (e.g., an AsyncJob) while the timer (and its ran()
+    // state) may be shared/affected by multiple concurrent tasks.
+    /// Whether the task is done participating in the managed activity.
+    bool stopped = false;
+};
+
 class FwdState: public RefCountable, public PeerSelectionInitiator
 {
     CBDATA_CHILD(FwdState);
@@ -212,6 +246,9 @@ class FwdState: public RefCountable, public PeerSelectionInitiator
     /// Whether the entire reply (including any body) was written to Store.
     /// The string literal value is only used for debugging.
     const char *storedWholeReply_;
+
+    /// Measures time spent on selecting and communicating with peers.
+    PeeringActivityTimer peeringTimer;
 };
 
 class acl_tos;