]> git.ipfire.org Git - thirdparty/squid.git/commitdiff
Remove ConfigParser::Undo() hack to improve ACL flags parsing (#960)
authorEduard Bagdasaryan <eduard.bagdasaryan@measurement-factory.com>
Thu, 13 Jan 2022 00:59:24 +0000 (00:59 +0000)
committerSquid Anubis <squid-anubis@squid-cache.org>
Thu, 13 Jan 2022 15:22:19 +0000 (15:22 +0000)
Since `acl ... -n` support was added in commit 33810b1, flag-agnostic
parseFlags() extracts ACL flags, applies supported ones, and rejects the
rest. However, that extraction code does not apply the supported `-i`
flag! Instead, the flag is put "back" via Undo() as if we did not see it
at all. Later, the ACL data parser re-parses and applies it.

That "undo" hack avoided ACL data parsing changes but caused several
problems, including:

* The global Undo_ storage could "leak" the stored flag to the wrong ACL
  on certain ACL parsing errors. The global nature of that storage also
  blocked serious preprocessing/reconfiguration support improvements.

* AclData::parse() did not distinguish (previously undone and now
  "redone") flags from (post-"--") ACL data, blindly assuming that the
  first token is a flag and treating the remaining tokens as ACL data.

* Increasingly inconsistent handling of `-i` and `+i` flags.

* Related `ident -i` parsing code had severe performance problems: Some
  tests timed out due to exponential growth of `-i` parsing delays (with
  the number of parsed ACLs) caused by excessive userDataNames copying.

This change removes the "undo" hack for good. We now parse all leading
ACL options once, using a single Acl::Option API for both "global" (i.e.
applicable to the entire named ACL object) and "line" scoped options.
The parsed line-scoped flags (e.g., `-i`) are reset before parsing each
ACL directive line. They are delivered to the (ACL data) parsing code
using the existing Acl::Option linkedWith() mechanism.

TODO: This change does not fix all ACL data flag handling problems. For
example, ACL data parsing methods should be refactored to reuse the
now-generalized Acl::Option API for handling flags located _between_ ACL
parameters. Those non-trivial fixes are unrelated to Undo() removal and
will fix/improve ACL data handling, so they deserve dedicated commits.

27 files changed:
src/ConfigParser.cc
src/ConfigParser.h
src/acl/Acl.cc
src/acl/Acl.h
src/acl/CharacterSetOption.h
src/acl/Data.h
src/acl/DestinationDomain.cc
src/acl/DestinationIp.cc
src/acl/ExtUser.cc
src/acl/ExtUser.h
src/acl/HttpHeaderData.cc
src/acl/HttpHeaderData.h
src/acl/Note.cc
src/acl/Options.cc
src/acl/Options.h
src/acl/RegexData.cc
src/acl/RegexData.h
src/acl/ServerName.cc
src/acl/Strategised.h
src/acl/UserData.cc
src/acl/UserData.h
src/auth/AclMaxUserIp.cc
src/auth/AclProxyAuth.cc
src/auth/AclProxyAuth.h
src/ident/AclIdent.cc
src/ident/AclIdent.h
src/tests/stub_libauth_acls.cc

index 55b3b421300d6a508d585e6b77df5d1000b7a626..53e68a0f996eea1cf0827a04c37ad44e204fb8ee 100644 (file)
@@ -23,7 +23,6 @@ ConfigParser::TokenType ConfigParser::LastTokenType = ConfigParser::SimpleToken;
 const char *ConfigParser::CfgLine = NULL;
 const char *ConfigParser::CfgPos = NULL;
 std::queue<char *> ConfigParser::CfgLineTokens_;
-std::queue<std::string> ConfigParser::Undo_;
 bool ConfigParser::AllowMacros_ = false;
 bool ConfigParser::ParseQuotedOrToEol_ = false;
 bool ConfigParser::ParseKvPair_ = false;
@@ -60,27 +59,6 @@ ConfigParser::destruct()
                cfg_filename, config_lineno, config_input_line);
 }
 
-void
-ConfigParser::TokenPutBack(const char *tok)
-{
-    assert(tok);
-    Undo_.push(tok);
-}
-
-char *
-ConfigParser::Undo()
-{
-    static char undoToken[CONFIG_LINE_LIMIT];
-    if (!Undo_.empty()) {
-        xstrncpy(undoToken, Undo_.front().c_str(), sizeof(undoToken));
-        undoToken[sizeof(undoToken) - 1] = '\0';
-        if (!PreviewMode_)
-            Undo_.pop();
-        return undoToken;
-    }
-    return NULL;
-}
-
 char *
 ConfigParser::strtokFile()
 {
@@ -93,9 +71,6 @@ ConfigParser::strtokFile()
     char *t;
     static char buf[CONFIG_LINE_LIMIT];
 
-    if ((t = ConfigParser::Undo()))
-        return t;
-
     do {
 
         if (!fromFile) {
@@ -356,10 +331,6 @@ char *
 ConfigParser::NextToken()
 {
     char *token = NULL;
-    if ((token = ConfigParser::Undo())) {
-        debugs(3, 6, "TOKEN (undone): " << token);
-        return token;
-    }
 
     do {
         while (token == NULL && !CfgFiles.empty()) {
index d6a80a4be9d25f6b2e4a556fa6e873fe330be099..4dfb20ce1b3b9ffb1b863c89e221cf0850c68612 100644 (file)
@@ -134,12 +134,6 @@ public:
      */
     static char *PeekAtToken();
 
-    /**
-     * The next NextToken call will return the token as next element
-     * It can be used repeatedly to add more than one tokens in a FIFO list.
-     */
-    static void TokenPutBack(const char *token);
-
     /// Set the configuration file line to parse.
     static void SetCfgLine(char *line);
 
@@ -205,9 +199,6 @@ protected:
         int lineNo; ///< Current line number
     };
 
-    /// Return the last TokenPutBack() queued element or NULL if none exist
-    static char *Undo();
-
     /**
      * Unquotes the token, which must be quoted.
      * \param next if it is not NULL, it is set after the end of token.
@@ -230,7 +221,6 @@ protected:
     static const char *CfgLine; ///< The current line to parse
     static const char *CfgPos; ///< Pointer to the next element in cfgLine string
     static std::queue<char *> CfgLineTokens_; ///< Store the list of tokens for current configuration line
-    static std::queue<std::string> Undo_; ///< The list with TokenPutBack() queued elements
     static bool AllowMacros_;
     static bool ParseQuotedOrToEol_; ///< The next tokens will be handled as quoted or to_eol token
     static bool RecognizeQuotedPair_; ///< The next tokens may contain quoted-pair (\-escaped) characters
index 9566397ffb03ff9c6ce0facb3cd6991de2c755aa..eca6e9c9b9578d53b8681f0cf3b24b924acb6465 100644 (file)
@@ -288,15 +288,23 @@ ACL::isProxyAuth() const
 void
 ACL::parseFlags()
 {
-    // ACL kids that carry ACLData which supports parameter flags override this
-    Acl::ParseFlags(options(), Acl::NoFlags());
+    Acl::Options allOptions = options();
+    for (const auto lineOption: lineOptions()) {
+        lineOption->unconfigure(); // forget any previous "acl ..." line effects
+        allOptions.push_back(lineOption);
+    }
+    Acl::ParseFlags(allOptions);
 }
 
 SBufList
 ACL::dumpOptions()
 {
     SBufList result;
+
     const auto &myOptions = options();
+    // XXX: No lineOptions() call here because we do not remember ACL "line"
+    // boundaries and associated "line" options; we cannot report them.
+
     // optimization: most ACLs do not have myOptions
     // this check also works around dump_SBufList() adding ' ' after empty items
     if (!myOptions.empty()) {
index e7934ab449e1a71866daff6c9620310baa82cc30..a8b163ee105191dd42235a23304dc8db8371c9d5 100644 (file)
@@ -60,11 +60,8 @@ public:
     /// Updates the checklist state on match, async, and failure.
     bool matches(ACLChecklist *checklist) const;
 
-    /// \returns (linked) Options supported by this ACL
-    virtual const Acl::Options &options() { return Acl::NoOptions(); }
-
     /// configures ACL options, throwing on configuration errors
-    virtual void parseFlags();
+    void parseFlags();
 
     /// parses node representation in squid.conf; dies on failures
     virtual void parse() = 0;
@@ -96,6 +93,14 @@ private:
     virtual bool requiresRequest() const;
     /// whether our (i.e. shallow) match() requires checklist to have a reply
     virtual bool requiresReply() const;
+
+    // TODO: Rename to globalOptions(); these are not the only supported options
+    /// \returns (linked) 'global' Options supported by this ACL
+    virtual const Acl::Options &options() { return Acl::NoOptions(); }
+
+    /// \returns (linked) "line" Options supported by this ACL
+    /// \see ACL::options()
+    virtual const Acl::Options &lineOptions() { return Acl::NoOptions(); }
 };
 
 /// \ingroup ACLAPI
index e4b839604f58966b3f9f04a7c11c7c8ea82b5a85..600a32f6e96d4903f8d59327dfcd29e5b0b520bb 100644 (file)
@@ -41,7 +41,7 @@ class CharacterSetOption: public TypedOption<CharacterSetOptionValue>
 {
 public:
     typedef TypedOption<CharacterSetOptionValue> Parent;
-    CharacterSetOption(): Parent(valueOptional) {}
+    explicit CharacterSetOption(const char *name): Parent(name, nullptr, valueOptional) {}
 };
 
 } // namespace Acl
index 5037bb9618fec3f4b805f6cd083157833f676ffe..4a3d5fac173849cc173197b9dbabd935a4a7de04 100644 (file)
@@ -22,8 +22,8 @@ public:
     ACLData(ACLData<M> &&) = delete; // no copying of any kind
     virtual ~ACLData() {}
 
-    /// \returns the flags supported by these ACL parameters (e.g., "-i")
-    virtual const Acl::ParameterFlags &supportedFlags() const { return Acl::NoFlags(); }
+    /// supported ACL "line" options (e.g., "-i")
+    virtual const Acl::Options &lineOptions() { return Acl::NoOptions(); }
 
     virtual bool match(M) =0;
     virtual SBufList dump() const =0;
index 1cec068598cac2bb331044a4c9790c4adb6b8786..60e6d68dbe19177a4eb1d0c0ed3f36bbfdb222a2 100644 (file)
@@ -45,8 +45,8 @@ DestinationDomainLookup::LookupDone(const char *, const Dns::LookupDetails &deta
 const Acl::Options &
 ACLDestinationDomainStrategy::options()
 {
-    static const Acl::BooleanOption LookupBanFlag;
-    static const Acl::Options MyOptions = { { "-n", &LookupBanFlag } };
+    static const Acl::BooleanOption LookupBanFlag("-n");
+    static const Acl::Options MyOptions = { &LookupBanFlag };
     LookupBanFlag.linkWith(&lookupBanned);
     return MyOptions;
 }
index f0ef36e59e9ce8583ddc0bcea0761aa2d3346242..a7fed9d9c44ea6a3c3f434ca2eaa2c094de42496 100644 (file)
@@ -26,8 +26,8 @@ ACLDestinationIP::typeString() const
 const Acl::Options &
 ACLDestinationIP::options()
 {
-    static const Acl::BooleanOption LookupBan;
-    static const Acl::Options MyOptions = { { "-n", &LookupBan } };
+    static const Acl::BooleanOption LookupBan("-n");
+    static const Acl::Options MyOptions = { &LookupBan };
     LookupBan.linkWith(&lookupBanned);
     return MyOptions;
 }
index 2e6ec30163d936e7517a693e075a70a9c3331504..f7abacd17a8f9ae45e31e4e20f0660dc2a3a0898 100644 (file)
@@ -32,10 +32,10 @@ ACLExtUser::typeString() const
     return type_;
 }
 
-void
-ACLExtUser::parseFlags()
+const Acl::Options &
+ACLExtUser::lineOptions()
 {
-    ParseFlags(Acl::NoOptions(), data->supportedFlags());
+    return data->lineOptions();
 }
 
 void
index 07e4dbbc1306c892f106c703a45636dad3440634..aec080e5228418e8997c213590d18160f65578df 100644 (file)
@@ -26,12 +26,14 @@ public:
     /* ACL API */
     virtual char const *typeString() const;
     virtual void parse();
-    virtual void parseFlags();
     virtual int match(ACLChecklist *checklist);
     virtual SBufList dump() const;
     virtual bool empty () const;
 
 private:
+    /* ACL API */
+    virtual const Acl::Options &lineOptions();
+
     ACLData<char const *> *data;
     char const *type_;
 };
index c78366b7f0b26b4b88c47ca99984d34db80c396f..a2736d4bcb0bed22117483317720d4b988248715 100644 (file)
@@ -66,6 +66,12 @@ ACLHTTPHeaderData::dump() const
     return sl;
 }
 
+const Acl::Options &
+ACLHTTPHeaderData::lineOptions()
+{
+    return regex_rule->lineOptions();
+}
+
 void
 ACLHTTPHeaderData::parse()
 {
index e94cf31cc459bf4b1e4ef9f33a5e32fb77564bb1..2599eb7ac5ff84d6bb7a97af5d5006ae9d092b5a 100644 (file)
@@ -27,6 +27,9 @@ public:
     virtual bool empty() const;
 
 private:
+    /* ACLData API */
+    virtual const Acl::Options &lineOptions();
+
     Http::HdrType hdrId;            /**< set if header is known */
     SBuf hdrName;                   /**< always set */
     ACLData<char const *> * regex_rule;
index 4cb3e6605a92141f6ed4cb93b9be43c8b5af8003..c0e210d5d5e42149f027a63f6a9038b8b81da257 100644 (file)
 const Acl::Options &
 Acl::AnnotationStrategy::options()
 {
-    static const Acl::CharacterSetOption Delimiters;
-    static const Acl::Options MyOptions = {
-        { "-m", &Delimiters }
-    };
+    static const Acl::CharacterSetOption Delimiters("-m");
+    static const Acl::Options MyOptions = { &Delimiters };
     Delimiters.linkWith(&delimiters);
     return MyOptions;
 }
index ca46dd6c2f35a418e48a9a183d0c3d58044019fe..4a7667d5fcbcacfe4a9473e5038ebf653a06b661 100644 (file)
@@ -13,6 +13,7 @@
 #include "sbuf/Stream.h"
 
 #include <iostream>
+#include <utility>
 #include <vector>
 
 namespace Acl {
@@ -46,31 +47,28 @@ private:
 class OptionsParser
 {
 public:
-    OptionsParser(const Options &options, const ParameterFlags &flags);
+    explicit OptionsParser(const Options &options);
 
     // fill previously supplied options container, throwing on errors
     void parse();
 
 private:
-    const Option *findOption(/* const */ SBuf &rawName);
-
-    /// ACL parameter flags in parsing order
-    typedef std::vector<OptionName> Names;
-    /// parsed ACL parameter flags that must be preserved for ACLData::parse()
-    static Names flagsToSkip;
+    using SupportedOption = std::pair<const Option *, bool /* enable */ >;
+    SupportedOption supportedOption(const SBuf &name) const;
 
     const Options &options_; ///< caller-supported, linked options
-    const ParameterFlags &parameterFlags_; ///< caller-supported parameter flags
 };
 
 } // namespace Acl
 
-/* Acl::OptionNameCmp */
+/* Acl::Option */
 
-bool
-Acl::OptionNameCmp::operator()(const OptionName a, const OptionName b) const
+Acl::Option::Option(const char * const nameThatEnables, const char * const nameThatDisables, const ValueExpectation vex):
+    onName(nameThatEnables),
+    offName(nameThatDisables),
+    valueExpectation(vex)
 {
-    return strcmp(a, b) < 0;
+    assert(onName);
 }
 
 /* Acl::OptionExtractor */
@@ -168,78 +166,67 @@ Acl::OptionExtractor::extractShort()
 
 /* Acl::OptionsParser */
 
-// being "static" is an optimization to avoid paying for vector creation/growth
-Acl::OptionsParser::Names Acl::OptionsParser::flagsToSkip;
-
-Acl::OptionsParser::OptionsParser(const Options &options, const ParameterFlags &flags):
-    options_(options),
-    parameterFlags_(flags)
+Acl::OptionsParser::OptionsParser(const Options &options):
+    options_(options)
 {
 }
 
-const Acl::Option *
-Acl::OptionsParser::findOption(/* const */ SBuf &rawNameBuf)
+/// \returns named supported option paired with a name-based enable/disable flag
+Acl::OptionsParser::SupportedOption
+Acl::OptionsParser::supportedOption(const SBuf &name) const
 {
-    // TODO: new std::map::find() in C++14 does not require this conversion
-    const auto rawName = rawNameBuf.c_str();
-
-    const auto optionPos = options_.find(rawName);
-    if (optionPos != options_.end())
-        return optionPos->second;
-
-    const auto flagPos = parameterFlags_.find(rawName);
-    if (flagPos != parameterFlags_.end()) {
-        flagsToSkip.push_back(*flagPos); // *flagPos is permanent unlike rawName
-        return nullptr;
+    for (const auto option: options_) {
+        if (name.cmp(option->onName) == 0)
+            return SupportedOption(option, true);
+        if (option->offName && name.cmp(option->offName) == 0)
+            return SupportedOption(option, false);
     }
 
-    throw TexcHere(ToSBuf("unsupported ACL option: ", rawNameBuf));
+    throw TexcHere(ToSBuf("unsupported ACL option: ", name));
 }
 
 void
 Acl::OptionsParser::parse()
 {
-    flagsToSkip.clear();
-
     OptionExtractor oex;
     while (oex.extractOne()) {
-        /* const */ auto rawName = oex.name;
-        if (const Option *optionPtr = findOption(rawName)) {
-            const Option &option = *optionPtr;
+        const auto explicitOption = supportedOption(oex.name);
+        const auto &option = *explicitOption.first;
+        if (explicitOption.second) {
+            /* configuration enables this option */
             if (option.configured())
-                debugs(28, 7, "acl uses multiple " << rawName << " options");
+                debugs(28, 7, "acl uses multiple " << oex.name << " options");
             switch (option.valueExpectation)
             {
             case Option::valueNone:
                 if (oex.hasValue)
-                    throw TexcHere(ToSBuf("unexpected value for an ACL option: ", rawName, '=', oex.value()));
-                option.configureDefault();
+                    throw TexcHere(ToSBuf("unexpected value for an ACL option: ", oex.name, '=', oex.value()));
+                option.enable();
                 break;
             case Option::valueRequired:
                 if (!oex.hasValue)
-                    throw TexcHere(ToSBuf("missing required value for ACL option ", rawName));
+                    throw TexcHere(ToSBuf("missing required value for ACL option ", oex.name));
                 option.configureWith(oex.value());
                 break;
             case Option::valueOptional:
                 if (oex.hasValue)
                     option.configureWith(oex.value());
                 else
-                    option.configureDefault();
+                    option.enable();
                 break;
             }
+        } else {
+            if (oex.hasValue)
+                throw TexcHere(ToSBuf("unexpected value when disabling an ACL option: ", oex.name, '=', oex.value()));
+            option.disable();
         }
-        // else skip supported parameter flag
     }
-
-    /* hack: regex code wants to parse all -i and +i flags itself */
-    for (const auto name: flagsToSkip)
-        ConfigParser::TokenPutBack(name);
 }
 
 void
-Acl::ParseFlags(const Options &options, const ParameterFlags &flags)
+Acl::ParseFlags(const Options &options)
 {
-    OptionsParser parser(options, flags);
+    OptionsParser parser(options);
     parser.parse();
 }
 
@@ -250,32 +237,26 @@ Acl::NoOptions()
     return none;
 }
 
-const Acl::ParameterFlags &
-Acl::NoFlags()
+const Acl::BooleanOption &
+Acl::CaseSensitivityOption()
 {
-    static const ParameterFlags none;
-    return none;
+    static const BooleanOption MyOption("-i", "+i");
+    return MyOption;
 }
 
 std::ostream &
 operator <<(std::ostream &os, const Acl::Option &option)
 {
-    if (option.valued()) {
-        os << '=';
-        option.print(os);
-    }
+    option.print(os);
     return os;
 }
 
 std::ostream &
 operator <<(std::ostream &os, const Acl::Options &options)
 {
-    for (const auto pos: options) {
-        assert(pos.second);
-        const auto &option = *pos.second;
-        if (option.configured())
-            os << pos.first << option;
-    }
+    for (const auto option: options)
+        os << *option;
+
     // TODO: Remember "--" presence and print that delimiter when present.
     // Detecting its need is difficult because parameter flags start with "-".
     return os;
index ddbc098c14c38a259def07720875608123c67e00..e45e0af471d19bcd72378262b8bf53e711099329 100644 (file)
 #include "sbuf/forward.h"
 
 #include <iosfwd>
-#include <map>
-#include <set>
+#include <vector>
 
-// After all same-name acl configuration lines are merged into one ACL:
-//   configuration = acl name type [option...] [[flag...] parameter...]
-//   option = -x[=value] | --name[=value]
-//   flag = option
+// After line continuation is handled by the preprocessor, an ACL object
+// configuration can be visualized as a sequence of same-name "acl ..." lines:
 //
-// Options and flags use the same syntax, but differ in scope and handling code:
-// * ACL options appear before all parameters and apply to all parameters.
-//   They are handled by ACL kids (or equivalent).
-// * Parameter flags may appear after some other parameters and apply only to
-//   the subsequent parameters (until they are overwritten by later flags).
-//   They are handled by ACLData kids.
-// ACL options parsing code skips and leaves leading parameter flags (if any)
-// for ACLData code to process.
+// L1: acl exampleA typeT parameter1 -i parameter2 parameter3
+// L2: acl exampleA typeT parameter4
+// L3: acl exampleA typeT -i -n parameter5 +i parameter6
+// L4: acl exampleA typeT -n parameter7
+//
+// There are two kinds of ACL options (a.k.a. flags):
+//
+// * Global (e.g., "-n"): Applies to all parameters regardless of where the
+//   option was discovered/parsed (e.g., "-n" on L3 affects parameter2 on L1).
+//   Declared by ACL class kids (or equivalent) via ACL::options().
+//
+// * Line (e.g., "-i"): Applies to the yet unparsed ACL parameters of the
+//   current "acl ..." line (e.g., "-i" on L1 has no effect on parameter4 on L2)
+//   Declared by ACLData class kids (or equivalent) via lineOptions().
+//
+// Here is the option:explicitly-affected-parameters map for the above exampleA:
+//   "-n": parameter1-7 (i.e. all parameters)
+//   "-i": parameter2, parameter3; parameter5
+//   "+i": parameter6
+//
+// The option name spelling determines the option kind and effect.
+// Both option kinds use the same general option configuration syntax:
+//   option = name [ '=' value ]
+// where "name" is option-specific spelling that looks like -x, +x, or --long
+//
+// On each "acl ..." line, global options can only appear before the first
+// parameter, while line options can go before any parameter.
+//
+// XXX: The fact that global options affect previous (and subsequent) same-name
+// "acl name ..." lines surprises and confuses those who comprehend ACLs in
+// terms of configuration lines (which Squid effectively merges together).
 
 namespace Acl {
 
-typedef const char *OptionName;
-
 /// A single option supported by an ACL: -x[=value] or --name[=value]
-/// Unlike a parameter flag, this option applies to all ACL parameters.
 class Option
 {
 public:
     typedef enum { valueNone, valueOptional, valueRequired } ValueExpectation;
-    explicit Option(ValueExpectation vex = valueNone): valueExpectation(vex) {}
+    explicit Option(const char *nameThatEnables, const char *nameThatDisables = nullptr, ValueExpectation vex = valueNone);
     virtual ~Option() {}
 
-    /// whether the admin explicitly specified this option
-    /// (i.e., whether configureWith() or configureDefault() has been called)
+    /// whether the admin explicitly specified this option (i.e., whether
+    /// enable(), configureWith(), or disable() has been called)
     virtual bool configured() const = 0;
 
-    /// called after parsing -x or --name
-    virtual void configureDefault() const = 0;
+    /// called after parsing onName without a value (e.g., -x or --enable-x)
+    virtual void enable() const = 0;
 
-    /// called after parsing -x=value or --name=value
+    /// called after parsing onName and a value (e.g., -x=v or --enable-x=v)
     virtual void configureWith(const SBuf &rawValue) const = 0;
 
+    /// called after parsing offName (e.g., +i or --disable-x)
+    virtual void disable() const = 0;
+
+    /// clear enable(), configureWith(), or disable() effects
+    virtual void unconfigure() const = 0;
+
+    /// whether disable() has been called
+    virtual bool disabled() const = 0;
+
     virtual bool valued() const = 0;
 
     /// prints a configuration snippet (as an admin could have typed)
     virtual void print(std::ostream &os) const = 0;
 
+    /// A name that must be used to explicitly enable this Option (required).
+    const char * const onName = nullptr;
+
+    /// A name that must be used to explicitly disable this Option (optional).
+    /// Nil for (and only for) options that cannot be disabled().
+    const char * const offName = nullptr;
+
     ValueExpectation valueExpectation = valueNone; ///< expect "=value" part?
 };
 
@@ -68,13 +101,26 @@ class OptionValue
 public:
     typedef Value value_type;
 
+    // TODO: Some callers use .value without checking whether the option is
+    // enabled(), accessing the (default-initialized or customized) default
+    // value that way. This trick will stop working if we add valued options
+    // that can be disabled (e.g., --with-foo=x --without-foo). To support such
+    // options, store the default value separately and provide value accessor.
+
     OptionValue(): value {} {}
     explicit OptionValue(const Value &aValue): value(aValue) {}
 
-    explicit operator bool() const { return configured; }
+    /// whether the option is explicitly turned "on" (with or without a value)
+    bool enabled() const { return configured && !disabled; }
+    explicit operator bool() const { return enabled(); }
+
+    /// go back to the default-initialized state
+    void reset() { *this = OptionValue<Value>(); }
 
     Value value; ///< final value storage, possibly after conversions
     bool configured = false; ///< whether the option was present in squid.conf
+    /* flags for configured options */
+    bool disabled = false; ///< whether the option was turned off
     bool valued = false; ///< whether a configured option had a value
 };
 
@@ -84,7 +130,8 @@ class TypedOption: public Option
 {
 public:
     //typedef typename Recipient::value_type value_type;
-    explicit TypedOption(ValueExpectation vex = valueNone): Option(vex) {}
+    explicit TypedOption(const char *nameThatEnables, const char *nameThatDisables = nullptr, ValueExpectation vex = valueNone):
+        Option(nameThatEnables, nameThatDisables, vex) {}
 
     /// who to tell when this option is enabled
     void linkWith(Recipient *recipient) const
@@ -96,32 +143,53 @@ public:
     /* Option API */
 
     virtual bool configured() const override { return recipient_ && recipient_->configured; }
+    virtual bool disabled() const override { return recipient_ && recipient_->disabled && /* paranoid: */ offName; }
     virtual bool valued() const override { return recipient_ && recipient_->valued; }
 
-    /// sets the default value when option is used without a value
-    virtual void configureDefault() const override
+    virtual void unconfigure() const override {
+        assert(recipient_);
+        recipient_->reset();
+    }
+
+    virtual void enable() const override
     {
         assert(recipient_);
         recipient_->configured = true;
+        recipient_->disabled = false;
         recipient_->valued = false;
-        // sets recipient_->value to default
-        setDefault();
+        // leave recipient_->value unchanged
     }
 
-    /// sets the option value from rawValue
     virtual void configureWith(const SBuf &rawValue) const override
     {
         assert(recipient_);
         recipient_->configured = true;
+        recipient_->disabled = false;
         recipient_->valued = true;
         import(rawValue);
     }
 
-    virtual void print(std::ostream &os) const override { if (valued()) os << recipient_->value; }
+    virtual void disable() const override
+    {
+        assert(recipient_);
+        recipient_->configured = true;
+        recipient_->disabled = true;
+        recipient_->valued = false;
+        // leave recipient_->value unchanged
+    }
+
+    virtual void print(std::ostream &os) const override
+    {
+        if (configured()) {
+            os << ' ' << (disabled() ? offName : onName);
+            if (valued())
+                os << '=' << recipient_->value;
+        }
+        // else do not report the implicit default
+    }
 
 private:
     void import(const SBuf &rawValue) const { recipient_->value = rawValue; }
-    void setDefault() const { /*leave recipient_->value as is*/}
 
     // The "mutable" specifier demarcates set-once Option kind/behavior from the
     // ever-changing recipient of the actual admin-configured option value.
@@ -143,32 +211,19 @@ BooleanOption::import(const SBuf &) const
     assert(!"boolean options do not have ...=values (for now)");
 }
 
-template <>
-inline void
-BooleanOption::setDefault() const
-{
-    recipient_->value = true;
-}
-
-/// option name comparison functor
-class OptionNameCmp {
-public:
-    bool operator()(const OptionName a, const OptionName b) const;
-};
-/// name:option map
-typedef std::map<OptionName, const Option*, OptionNameCmp> Options;
-
-/// a set of parameter flag names
-typedef std::set<OptionName, OptionNameCmp> ParameterFlags;
+using Options = std::vector<const Option *>;
 
 /// parses the flags part of the being-parsed ACL, filling Option values
 /// \param options options supported by the ACL as a whole (e.g., -n)
-/// \param flags options supported by ACL parameter(s) (e.g., -i)
-void ParseFlags(const Options &options, const ParameterFlags &flags);
+void ParseFlags(const Options &options);
 
-/* handy for Class::options() and Class::supportedFlags() defaults */
+/* handy for Class::options() and lineOptions() defaults */
 const Options &NoOptions(); ///< \returns an empty Options container
-const ParameterFlags &NoFlags(); ///< \returns an empty ParameterFlags container
+
+/// A boolean option that controls case-sensitivity (-i/+i).
+/// An enabled (-i) state is "case insensitive".
+/// A disabled (+i) and default states are "case sensitive".
+const BooleanOption &CaseSensitivityOption();
 
 } // namespace Acl
 
index 548575759ad8d73e94da8f3dd2afe84c6f570251..ecd9c33ecd749dd988871bb73bf117b302c07b03 100644 (file)
 #include "sbuf/Algorithms.h"
 #include "sbuf/List.h"
 
+Acl::BooleanOptionValue ACLRegexData::CaseInsensitive_;
+
 ACLRegexData::~ACLRegexData()
 {
 }
 
-const Acl::ParameterFlags &
-ACLRegexData::supportedFlags() const
+const Acl::Options &
+ACLRegexData::lineOptions()
 {
-    static const Acl::ParameterFlags flags = { "-i", "+i" };
-    return flags;
+    static auto MyCaseSensitivityOption = Acl::CaseSensitivityOption();
+    static const Acl::Options MyOptions = { &MyCaseSensitivityOption };
+    MyCaseSensitivityOption.linkWith(&CaseInsensitive_);
+    return MyOptions;
 }
 
 bool
@@ -148,12 +152,12 @@ compileRE(std::list<RegexPattern> &curlist, const SBufList &RE, int flags)
  * called only once per ACL.
  */
 static int
-compileOptimisedREs(std::list<RegexPattern> &curlist, const SBufList &sl)
+compileOptimisedREs(std::list<RegexPattern> &curlist, const SBufList &sl, const int flagsAtLineStart)
 {
     std::list<RegexPattern> newlist;
     SBufList accumulatedRE;
     int numREs = 0, reSize = 0;
-    int flags = REG_EXTENDED | REG_NOSUB;
+    auto flags = flagsAtLineStart;
 
     for (const SBuf & configurationLineWord : sl) {
         static const SBuf minus_i("-i");
@@ -221,9 +225,9 @@ compileOptimisedREs(std::list<RegexPattern> &curlist, const SBufList &sl)
 }
 
 static void
-compileUnoptimisedREs(std::list<RegexPattern> &curlist, const SBufList &sl)
+compileUnoptimisedREs(std::list<RegexPattern> &curlist, const SBufList &sl, const int flagsAtLineStart)
 {
-    int flags = REG_EXTENDED | REG_NOSUB;
+    auto flags = flagsAtLineStart;
 
     static const SBuf minus_i("-i"), plus_i("+i");
     for (auto configurationLineWord : sl) {
@@ -244,6 +248,10 @@ ACLRegexData::parse()
 {
     debugs(28, 2, "new Regex line or file");
 
+    int flagsAtLineStart = REG_EXTENDED | REG_NOSUB;
+    if (CaseInsensitive_)
+        flagsAtLineStart |= REG_ICASE;
+
     SBufList sl;
     while (char *t = ConfigParser::RegexStrtokFile()) {
         const char *clean = removeUnnecessaryWildcards(t);
@@ -256,9 +264,9 @@ ACLRegexData::parse()
         }
     }
 
-    if (!compileOptimisedREs(data, sl)) {
+    if (!compileOptimisedREs(data, sl, flagsAtLineStart)) {
         debugs(28, DBG_IMPORTANT, "WARNING: optimisation of regular expressions failed; using fallback method without optimisation");
-        compileUnoptimisedREs(data, sl);
+        compileUnoptimisedREs(data, sl, flagsAtLineStart);
     }
 }
 
index f67c85f2987625cb2d54723621f01d3ab59f0d9d..0512c0ed496d88c27e026ffa8e8ce13ce953813c 100644 (file)
@@ -24,10 +24,15 @@ public:
     virtual bool match(char const *user);
     virtual SBufList dump() const;
     virtual void parse();
-    virtual const Acl::ParameterFlags &supportedFlags() const;
     virtual bool empty() const;
 
 private:
+    /// whether parse() is called in a case insensitive context
+    static Acl::BooleanOptionValue CaseInsensitive_;
+
+    /* ACLData API */
+    virtual const Acl::Options &lineOptions();
+
     std::list<RegexPattern> data;
 };
 
index da74395ecdbec84c194801ef8b743e2868cbef24..ad43affca0df918bf1c6d5d133b912bc82d59003 100644 (file)
@@ -119,15 +119,10 @@ ACLServerNameStrategy::match (ACLData<MatchType> * &data, ACLFilledChecklist *ch
 const Acl::Options &
 ACLServerNameStrategy::options()
 {
-    static const Acl::BooleanOption ClientRequested;
-    static const Acl::BooleanOption ServerProvided;
-    static const Acl::BooleanOption Consensus;
-    static const Acl::Options MyOptions = {
-        {"--client-requested", &ClientRequested},
-        {"--server-provided", &ServerProvided},
-        {"--consensus", &Consensus}
-    };
-
+    static const Acl::BooleanOption ClientRequested("--client-requested");
+    static const Acl::BooleanOption ServerProvided("--server-provided");
+    static const Acl::BooleanOption Consensus("--consensus");
+    static const Acl::Options MyOptions = { &ClientRequested, &ServerProvided, &Consensus };
     ClientRequested.linkWith(&useClientRequested);
     ServerProvided.linkWith(&useServerProvided);
     Consensus.linkWith(&useConsensus);
index 4c9d18b91559176a9e9b99b47e6ec33adf434fb1..f3814667bb58af2b9db25e71dd362ca6f1d33ed0 100644 (file)
@@ -36,14 +36,12 @@ public:
     ACLStrategised(ACLData<MatchType> *, ACLStrategy<MatchType> *, char const *);
 
     virtual char const *typeString() const;
-    virtual void parseFlags();
 
     virtual bool requiresRequest() const {return matcher->requiresRequest();}
 
     virtual bool requiresReply() const {return matcher->requiresReply();}
 
     virtual void prepareForUse() { data->prepareForUse();}
-    virtual const Acl::Options &options() { return matcher->options(); }
     virtual void parse();
     virtual int match(ACLChecklist *checklist);
     virtual int match (M const &);
@@ -52,6 +50,10 @@ public:
     virtual bool valid () const;
 
 private:
+    /* ACL API */
+    virtual const Acl::Options &options() { return matcher->options(); }
+    virtual const Acl::Options &lineOptions() { return data->lineOptions(); }
+
     ACLData<MatchType> *data;
     char const *type_;
     ACLStrategy<MatchType> *matcher;
@@ -77,13 +79,6 @@ ACLStrategised<MatchType>::typeString() const
     return type_;
 }
 
-template <class MatchType>
-void
-ACLStrategised<MatchType>::parseFlags()
-{
-    ParseFlags(options(), data->supportedFlags());
-}
-
 template <class MatchType>
 void
 ACLStrategised<MatchType>::parse()
index 7f7de9f8717d857f7418badb635f6306099c1e65..d96d312121e8fd75b1f84b4e5079d09c49217721 100644 (file)
@@ -10,6 +10,7 @@
 
 #include "squid.h"
 #include "acl/Checklist.h"
+#include "acl/Options.h"
 #include "acl/UserData.h"
 #include "ConfigParser.h"
 #include "Debug.h"
 #include "sbuf/Algorithms.h"
 #include "util.h"
 
-const Acl::ParameterFlags &
-ACLUserData::supportedFlags() const
-{
-    static const Acl::ParameterFlags flagNames = { "-i", "+i" };
-    return flagNames;
-}
+Acl::BooleanOptionValue ACLUserData::CaseInsensitive_;
 
 bool
 ACLUserData::match(char const *user)
@@ -82,10 +78,20 @@ ACLUserData::ACLUserData() :
     flags.required = false;
 }
 
+const Acl::Options &
+ACLUserData::lineOptions()
+{
+    static auto MyCaseSensitivityOption = Acl::CaseSensitivityOption();
+    static const Acl::Options MyOptions = { &MyCaseSensitivityOption };
+    MyCaseSensitivityOption.linkWith(&CaseInsensitive_);
+    return MyOptions;
+}
+
 void
 ACLUserData::parse()
 {
     debugs(28, 2, "parsing user list");
+    flags.case_insensitive = bool(CaseInsensitive_);
 
     char *t = NULL;
     if ((t = ConfigParser::strtokFile())) {
index 3e3d0315bd73716bca4791496ca5d6663cd4f6d9..0a8f4862c55a48bfdce4164e0bec862d0082492c 100644 (file)
@@ -24,11 +24,15 @@ public:
     ACLUserData();
     bool match(char const *user);
     virtual SBufList dump() const;
-    void parse();
-    virtual const Acl::ParameterFlags &supportedFlags() const;
+    virtual void parse();
     bool empty() const;
 
 private:
+    /// whether parse() is called in a case insensitive context
+    static Acl::BooleanOptionValue CaseInsensitive_;
+
+    /* ACLData API */
+    virtual const Acl::Options &lineOptions();
 
     typedef std::set<SBuf,bool(*)(const SBuf&, const SBuf&)> UserDataNames_t;
     UserDataNames_t userDataNames;
index 1788f10ce75e0ea5393a69a0613d3f648609e8bf..0f73ebe19d1469de2e555284c8f8ce6fd3da8a94 100644 (file)
@@ -44,8 +44,8 @@ ACLMaxUserIP::valid() const
 const Acl::Options &
 ACLMaxUserIP::options()
 {
-    static const Acl::BooleanOption BeStrict;
-    static const Acl::Options MyOptions = { { "-s", &BeStrict } };
+    static const Acl::BooleanOption BeStrict("-s");
+    static const Acl::Options MyOptions = { &BeStrict };
     BeStrict.linkWith(&beStrict);
     return MyOptions;
 }
index b3bed5c3ec573db3b78a8af2dc31ac361e195214..8cec532e0553863142fe5b665b08d845dd6861e3 100644 (file)
@@ -37,10 +37,10 @@ ACLProxyAuth::typeString() const
     return type_;
 }
 
-void
-ACLProxyAuth::parseFlags()
+const Acl::Options &
+ACLProxyAuth::lineOptions()
 {
-    ParseFlags(Acl::NoOptions(), data->supportedFlags());
+    return data->lineOptions();
 }
 
 void
index 60d290e416c6ae2d1d44501b2ea18ad31bde27c4..ea68538087ade68a1c9470b300c49b47b626870a 100644 (file)
@@ -39,7 +39,6 @@ public:
     virtual char const *typeString() const;
     virtual void parse();
     virtual bool isProxyAuth() const {return true;}
-    virtual void parseFlags();
     virtual int match(ACLChecklist *checklist);
     virtual SBufList dump() const;
     virtual bool valid() const;
@@ -48,6 +47,9 @@ public:
     virtual int matchForCache(ACLChecklist *checklist);
 
 private:
+    /* ACL API */
+    virtual const Acl::Options &lineOptions();
+
     int matchProxyAuth(ACLChecklist *);
     ACLData<char const *> *data;
     char const *type_;
index d944e26ee80747486f1eab2eec5edce0f534b31f..ed3ba43f3709cce340bbbbb33914dd65db4b6cdb 100644 (file)
@@ -35,10 +35,10 @@ ACLIdent::typeString() const
     return type_;
 }
 
-void
-ACLIdent::parseFlags()
+const Acl::Options &
+ACLIdent::lineOptions()
 {
-    ParseFlags(Acl::NoOptions(), data->supportedFlags());
+    return data->lineOptions();
 }
 
 void
index c788871e87a436c6732a0d98184c07e7e566b742..e45e74c9a962a4fd9e1a0cdd10b3e6abf8e5ab4f 100644 (file)
@@ -42,12 +42,14 @@ public:
     virtual char const *typeString() const;
     virtual void parse();
     virtual bool isProxyAuth() const {return true;}
-    virtual void parseFlags();
     virtual int match(ACLChecklist *checklist);
     virtual SBufList dump() const;
     virtual bool empty () const;
 
 private:
+    /* ACL API */
+    virtual const Acl::Options &lineOptions();
+
     ACLData<char const *> *data;
     char const *type_;
 };
index 9461607412bd2f58e19d066a218fcaa02b66801f..d9d3b061d831041bb47c31595331691b3d8b3126 100644 (file)
@@ -42,7 +42,7 @@ void ProxyAuthLookup::checkForAsync(ACLChecklist *) const STUB
 void ProxyAuthLookup::LookupDone(void *) STUB
 int ACLProxyAuth::matchForCache(ACLChecklist *) STUB_RETVAL(0)
 int ACLProxyAuth::matchProxyAuth(ACLChecklist *) STUB_RETVAL(0)
-void ACLProxyAuth::parseFlags() STUB
+const Acl::Options &ACLProxyAuth::lineOptions() STUB_RETVAL(Acl::NoOptions())
 
 #endif /* USE_AUTH */