From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Wed, 28 Aug 2024 21:01:15 +0000 (+0000)
Subject: Reject config with unknown directives before committing to it (#1897)
X-Git-Tag: SQUID_7_0_1~67
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=ed8b040461bc6916d2acc96c2e6dd86c89de0110;p=thirdparty%2Fsquid.git

Reject config with unknown directives before committing to it (#1897)

Ideally, we want to reject configurations with unknown directives before
applying any configuration changes that correspond to known directives,
but current apply-as-you-parse architecture makes that impractical.
Pending smooth reconfiguration refactoring will make that possible, but
we can make a step towards that ideal future now.

Rejecting bad configurations before calling configDoConfigure() reduces
the set of configuration errors that Squid can detect in one execution
(because configDoConfigure() error-checking code is not reached), but
that small reduction is a lesser evil compared to running
configDoConfigure() with a clearly broken config, especially when we are
going to kill Squid anyway. While many legacy parse_foo() functions do
apply significant changes before configDoConfigure(), we cannot easily
prevent that (for now). We can easily prevent configDoConfigure().
---

diff --git a/src/cache_cf.cc b/src/cache_cf.cc
index b66ba93cd9..58f69f43f5 100644
--- a/src/cache_cf.cc
+++ b/src/cache_cf.cc
@@ -625,6 +625,9 @@ Configuration::Parse()
 
     defaults_postscriptum();
 
+    if (unrecognizedDirectives)
+        throw TextException(ToSBuf("Found ", unrecognizedDirectives, " unrecognized directive(s)"), Here());
+
     /*
      * We must call configDoConfigure() before leave_suid() because
      * configDoConfigure() is where we turn username strings into
@@ -632,15 +635,6 @@ Configuration::Parse()
      */
     configDoConfigure();
 
-    // TODO: Throw before configDoConfigure(). Doing that would reduce the set
-    // of configuration errors Squid can detect in one execution, but we should
-    // not apply a clearly broken configuration, especially when we are going to
-    // quit anyway. While some legacy parse_foo() functions apply significant
-    // changes before configDoConfigure(), we cannot easily stop them. We can
-    // easily stop configDoConfigure().
-    if (unrecognizedDirectives)
-        throw TextException(ToSBuf("Found ", unrecognizedDirectives, " unrecognized directive(s)"), Here());
-
     if (opt_send_signal == -1) {
         Mgr::RegisterAction("config",
                             "Current Squid Configuration",