Remove AclMatchedName from ACL::ParseAclLine()

src/acl/Acl.cc

@@ -66,6 +66,22 @@ Make(TypeName typeName)
     return result;
 }
 
+/// CodeContext of the being-parsed acl directive
+class ParsingContext: public CodeContext
+{
+public:
+    using Pointer = RefCount<ParsingContext>;
+
+    explicit ParsingContext(const SBuf &name): name_(name) {}
+
+    /* CodeContext API */
+    ScopedId codeContextGist() const override;
+    std::ostream &detailCodeContext(std::ostream &os) const override;
+
+private:
+    SBuf name_; ///< the aclname parameter of the being-parsed acl directive
+};
+
 } // namespace Acl
 
 void
@@ -80,9 +96,7 @@ void
 Acl::SetKey(SBuf &keyStorage, const char *keyParameterName, const char *newKey)
 {
     if (!newKey) {
-        throw TextException(ToSBuf("An acl declaration is missing a ", keyParameterName,
-                                   Debug::Extra, "ACL name: ", AclMatchedName),
-                            Here());
+        throw TextException(ToSBuf("An acl declaration is missing a ", keyParameterName), Here());
     }
 
     if (keyStorage.isEmpty()) {
@@ -96,12 +110,27 @@ Acl::SetKey(SBuf &keyStorage, const char *keyParameterName, const char *newKey)
     throw TextException(ToSBuf("Attempt to change the value of the ", keyParameterName, " argument in a subsequent acl declaration:",
                                Debug::Extra, "previously seen value: ", keyStorage,
                                Debug::Extra, "new/conflicting value: ", newKey,
-                               Debug::Extra, "ACL name: ", AclMatchedName,
                                Debug::Extra, "advice: Use a dedicated ACL name for each distinct ", keyParameterName,
                                " (and group those ACLs together using an 'any-of' ACL)."),
                         Here());
 }
 
+/* Acl::ParsingContext */
+
+ScopedId
+Acl::ParsingContext::codeContextGist() const {
+    return ScopedId("acl");
+}
+
+std::ostream &
+Acl::ParsingContext::detailCodeContext(std::ostream &os) const
+{
+    return os << Debug::Extra << "acl name: " << name_ <<
+           Debug::Extra << "configuration context: " << ConfigParser::CurrentLocation();
+}
+
+/* Acl::Node */
+
 void *
 Acl::Node::operator new (size_t)
 {
@@ -192,9 +221,6 @@ Acl::Node::ParseAclLine(ConfigParser &parser, Node ** head)
 {
     /* we're already using strtok() to grok the line */
     char *t = nullptr;
-    Node *A = nullptr;
-    LOCAL_ARRAY(char, aclname, ACL_NAME_SZ);
-    int new_acl = 0;
 
     /* snarf the ACL name */
 
@@ -211,7 +237,16 @@ Acl::Node::ParseAclLine(ConfigParser &parser, Node ** head)
         return;
     }
 
-    xstrncpy(aclname, t, ACL_NAME_SZ);
+    SBuf aclname(t);
+    CallParser(ParsingContext::Pointer::Make(aclname), [&] {
+        ParseNamed(parser, head, aclname.c_str()); // TODO: Convert Node::name to SBuf
+    });
+}
+
+/// parses acl directive parts that follow aclname
+void
+Acl::Node::ParseNamed(ConfigParser &parser, Node ** const head, const char * const aclname)
+{
     /* snarf the ACL type */
     const char *theType;
 
@@ -252,6 +287,8 @@ Acl::Node::ParseAclLine(ConfigParser &parser, Node ** head)
         theType = "client_connection_mark";
     }
 
+    Node *A = nullptr;
+    int new_acl = 0;
     if ((A = FindByName(aclname)) == nullptr) {
         debugs(28, 3, "aclParseAclLine: Creating ACL '" << aclname << "'");
         A = Acl::Make(theType);
@@ -268,22 +305,11 @@ Acl::Node::ParseAclLine(ConfigParser &parser, Node ** head)
         new_acl = 0;
     }
 
-    /*
-     * Here we set AclMatchedName in case we need to use it in a
-     * warning message in Acl::SplayInserter::Merge().
-     */
-    AclMatchedName = A->name;   /* ugly */
-
     A->parseFlags();
 
     /*split the function here */
     A->parse();
 
-    /*
-     * Clear AclMatchedName from our temporary hack
-     */
-    AclMatchedName = nullptr;  /* ugly */
-
     if (!new_acl)
         return;
 

src/acl/Node.h

@@ -90,6 +90,8 @@ class Node
     /// \returns (linked) "line" Options supported by this Acl::Node
     /// \see Acl::Node::options()
     virtual const Acl::Options &lineOptions() { return Acl::NoOptions(); }
+
+    static void ParseNamed(ConfigParser &, Node **head, const char *name);
 };
 
 } // namespace Acl

src/acl/ProtocolData.cc

@@ -57,7 +57,7 @@ ACLProtocolData::parse()
             }
         }
         if (p == AnyP::PROTO_UNKNOWN) {
-            debugs(28, DBG_IMPORTANT, "WARNING: Ignoring unknown protocol '" << t << "' in the ACL named '" << AclMatchedName << "'");
+            debugs(28, DBG_IMPORTANT, "WARNING: Ignoring unknown protocol '" << t << "' in the ACL");
             // XXX: store the text pattern of this protocol name for live comparisons
         }
     }

src/acl/SplayInserter.h

@@ -73,22 +73,22 @@ Acl::SplayInserter<DataValue>::Merge(Splay<Value> &storage, Value &&newItem)
 
         if (IsSubset(newItem, oldItem)) {
             debugs(28, DBG_PARSE_NOTE(DBG_IMPORTANT), "WARNING: Ignoring " << newItem << " because it is already covered by " << oldItem <<
-                   Debug::Extra << "advice: Remove value " << newItem << " from the ACL named " << AclMatchedName);
+                   Debug::Extra << "advice: Remove value " << newItem << " from the ACL");
             DestroyValue(newItem);
             return;
         }
 
         if (IsSubset(oldItem, newItem)) {
             debugs(28, DBG_PARSE_NOTE(DBG_IMPORTANT), "WARNING: Ignoring earlier " << oldItem << " because it is covered by " << newItem <<
-                   Debug::Extra << "advice: Remove value " << oldItem << " from the ACL named " << AclMatchedName);
+                   Debug::Extra << "advice: Remove value " << oldItem << " from the ACL");
             storage.remove(oldItem, comparator);
             DestroyValue(oldItem);
             continue; // still need to insert newItem (and it may conflict with other old items)
         }
 
         const auto combinedItem = MakeCombinedValue(oldItem, newItem);
         debugs(28, DBG_PARSE_NOTE(DBG_IMPORTANT), "WARNING: Merging overlapping " << newItem << " and " << oldItem << " into " << combinedItem <<
-               Debug::Extra << "advice: Replace values " << newItem << " and " << oldItem << " with " << combinedItem << " in the ACL named " << AclMatchedName);
+               Debug::Extra << "advice: Replace values " << newItem << " and " << oldItem << " with " << combinedItem << " in the ACL");
         DestroyValue(newItem);
         newItem = combinedItem;
         storage.remove(oldItem, comparator);

src/base/CodeContext.h

@@ -106,20 +106,38 @@ class CodeContextGuard
     CodeContext::Pointer savedCodeContext;
 };
 
-/// Executes service `callback` in `callbackContext`. If an exception occurs,
-/// the callback context is preserved, so that the exception is associated with
-/// the callback that triggered them (rather than with the service).
-///
+/// A helper that calls the given function in the given call context. If the
+/// function throws, the call context is preserved, so that the exception is
+/// associated with the context that triggered it.
+template <typename Fun>
+inline void
+CallAndRestore_(const CodeContext::Pointer &context, Fun &&fun)
+{
+    const auto savedCodeContext(CodeContext::Current());
+    CodeContext::Reset(context);
+    fun();
+    CodeContext::Reset(savedCodeContext);
+}
+
 /// Service code running in its own service context should use this function.
+/// \sa CallAndRestore_()
 template <typename Fun>
 inline void
 CallBack(const CodeContext::Pointer &callbackContext, Fun &&callback)
 {
     // TODO: Consider catching exceptions and letting CodeContext handle them.
-    const auto savedCodeContext(CodeContext::Current());
-    CodeContext::Reset(callbackContext);
-    callback();
-    CodeContext::Reset(savedCodeContext);
+    CallAndRestore_(callbackContext, callback);
+}
+
+/// To supply error-reporting code with parsing context X (where the error
+/// occurred), parsing code should use this function when initiating parsing
+/// inside that context X.
+/// \sa CallAndRestore_()
+template <typename Fun>
+inline void
+CallParser(const CodeContext::Pointer &parsingContext, Fun &&parse)
+{
+    CallAndRestore_(parsingContext, parse);
 }
 
 /// Executes `service` in `serviceContext` but due to automatic caller context

src/external_acl.cc

@@ -528,7 +528,7 @@ ACLExternal::parse()
 
     // def->name is the name of the external_acl_type.
     // this is the name of the 'acl' directive being tested
-    data->name = xstrdup(AclMatchedName);
+    data->name = xstrdup(name);
 
     while ((token = ConfigParser::strtokFile())) {
         wordlistAdd(&data->arguments, token);

test-suite/squidconf/bad-acl-dstdomain-dupe.conf

@@ -6,7 +6,7 @@
 ##
 
 acl test11 dstdomain .example.com example.com
-acl test12 dstdomain example.com .example.com
+acl test12 dstdomain example.org .example.org
 
 acl test21 dstdomain .example.com a.example.com
 acl test22 dstdomain b.example.com .example.com

test-suite/squidconf/bad-acl-dstdomain-dupe.conf.instructions

@@ -1,25 +1,31 @@
 expect-messages <<END
 WARNING: Ignoring example.com because it is already covered by .example.com
-    advice: Remove value example.com from the ACL named test11
+    advice: Remove value example.com from the ACL
+    acl name: test11
 
-WARNING: Ignoring earlier example.com because it is covered by .example.com
-    advice: Remove value example.com from the ACL named test12
+WARNING: Ignoring earlier example.org because it is covered by .example.org
+    advice: Remove value example.org from the ACL
+    acl name: test12
 
 WARNING: Ignoring a.example.com because it is already covered by .example.com
-    advice: Remove value a.example.com from the ACL named test21
+    advice: Remove value a.example.com from the ACL
+    acl name: test21
 
 WARNING: Ignoring earlier b.example.com because it is covered by .example.com
-    advice: Remove value b.example.com from the ACL named test22
+    advice: Remove value b.example.com from the ACL
+    acl name: test22
 
 WARNING: Ignoring .example.com because it is already covered by .example.com
-    advice: Remove value .example.com from the ACL named test31
-
+    advice: Remove value .example.com from the ACL
 WARNING: Ignoring c.example.com because it is already covered by .example.com
-    advice: Remove value c.example.com from the ACL named test31
+    advice: Remove value c.example.com from the ACL
+    acl name: test31
 
 WARNING: Ignoring earlier .d.example.com because it is covered by .example.com
-    advice: Remove value .d.example.com from the ACL named test41
+    advice: Remove value .d.example.com from the ACL
+    acl name: test41
 
 WARNING: Ignoring .e.example.com because it is already covered by .example.com
-    advice: Remove value .e.example.com from the ACL named test42
+    advice: Remove value .e.example.com from the ACL
+    acl name: test42
 END

test-suite/squidconf/bad-acl-http-status-dupe.conf

@@ -15,4 +15,4 @@ acl test14 http_status 304
 acl test21 http_status 100-300 350 200-400
 acl test22 http_status 150 200-400 100-300
 
-acl test30 http_status 300 400 300-400
+acl test30 http_status 300 399 300-399

test-suite/squidconf/bad-acl-http-status-dupe.conf.instructions

@@ -1,28 +1,35 @@
 expect-messages <<END
 WARNING: Ignoring 400 because it is already covered by 400
-    advice: Remove value 400 from the ACL named test11
+    advice: Remove value 400 from the ACL
+    acl name: test11
 
 WARNING: Ignoring 407 because it is already covered by 407
-    advice: Remove value 407 from the ACL named test12
+    advice: Remove value 407 from the ACL
+    acl name: test12
 
 WARNING: Ignoring earlier 200 because it is covered by 200-300
-    advice: Remove value 200 from the ACL named test13
+    advice: Remove value 200 from the ACL
+    acl name: test13
 
 WARNING: Ignoring 304 because it is already covered by 300-399
-    advice: Remove value 304 from the ACL named test14
+    advice: Remove value 304 from the ACL
+    acl name: test14
 
 WARNING: Ignoring earlier 350 because it is covered by 200-400
-    advice: Remove value 350 from the ACL named test21
+    advice: Remove value 350 from the ACL
 WARNING: Merging overlapping 200-400 and 100-300 into 100-400
-    advice: Replace values 200-400 and 100-300 with 100-400 in the ACL named test21
+    advice: Replace values 200-400 and 100-300 with 100-400 in the ACL
+    acl name: test21
 
 WARNING: Merging overlapping 100-300 and 200-400 into 100-400
-    advice: Replace values 100-300 and 200-400 with 100-400 in the ACL named test22
+    advice: Replace values 100-300 and 200-400 with 100-400 in the ACL
 WARNING: Ignoring earlier 150 because it is covered by 100-400
-    advice: Remove value 150 from the ACL named test22
+    advice: Remove value 150 from the ACL
+    acl name: test22
 
-WARNING: Ignoring earlier 400 because it is covered by 300-400
-    advice: Remove value 400 from the ACL named test30
-WARNING: Ignoring earlier 300 because it is covered by 300-400
-    advice: Remove value 300 from the ACL named test30
+WARNING: Ignoring earlier 399 because it is covered by 300-399
+    advice: Remove value 399 from the ACL
+WARNING: Ignoring earlier 300 because it is covered by 300-399
+    advice: Remove value 300 from the ACL
+    acl name: test30
 END

test-suite/squidconf/bad-acl-src-dupe.conf

@@ -8,15 +8,15 @@
 acl test11 src 127.0.0.1  127.0.0.0-127.0.0.255
 acl test12 src 192.168.1.0/24 192.168.0.0/16
 
-acl test13 src 127.0.0.0-127.0.0.255  127.0.0.1
+acl test13 src 127.0.0.0-127.0.0.255  127.0.0.2
 acl test14 src 127.0.0.0-127.0.0.128  127.0.0.128-127.0.0.255
 
 acl test15 src 10.0.0.1-10.0.0.128 10.0.0.0-10.0.0.1 10.0.0.128-10.0.0.255
 
 acl test25 dst 127.0.0.0-127.0.0.128/32  127.0.0.128-127.1.0.255
 
 acl test36 dst 127.0.0.1-127.0.0.128  127.0.0.0-127.1.0.0/16
-acl test37 dst 127.0.0.0-127.1.0.0/16 127.0.0.1-127.0.0.128
+acl test37 dst 127.1.0.0-127.2.0.0/16 127.1.0.1-127.1.0.128
 
 # TODO: make configurable depending on USE_IPV6
 # acl test41 src bad::1 bad::0-bad::f