From: Eduard Bagdasaryan Date: Thu, 13 Jan 2022 00:59:24 +0000 (+0000) Subject: Remove ConfigParser::Undo() hack to improve ACL flags parsing (#960) X-Git-Tag: SQUID_6_0_1~252 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=8d76389c009124d15d92c85a6ad4c17f38fa2705;p=thirdparty%2Fsquid.git Remove ConfigParser::Undo() hack to improve ACL flags parsing (#960) 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. --- diff --git a/src/ConfigParser.cc b/src/ConfigParser.cc index 55b3b42130..53e68a0f99 100644 --- a/src/ConfigParser.cc +++ b/src/ConfigParser.cc @@ -23,7 +23,6 @@ ConfigParser::TokenType ConfigParser::LastTokenType = ConfigParser::SimpleToken; const char *ConfigParser::CfgLine = NULL; const char *ConfigParser::CfgPos = NULL; std::queue ConfigParser::CfgLineTokens_; -std::queue 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()) { diff --git a/src/ConfigParser.h b/src/ConfigParser.h index d6a80a4be9..4dfb20ce1b 100644 --- a/src/ConfigParser.h +++ b/src/ConfigParser.h @@ -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 CfgLineTokens_; ///< Store the list of tokens for current configuration line - static std::queue 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 diff --git a/src/acl/Acl.cc b/src/acl/Acl.cc index 9566397ffb..eca6e9c9b9 100644 --- a/src/acl/Acl.cc +++ b/src/acl/Acl.cc @@ -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()) { diff --git a/src/acl/Acl.h b/src/acl/Acl.h index e7934ab449..a8b163ee10 100644 --- a/src/acl/Acl.h +++ b/src/acl/Acl.h @@ -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 diff --git a/src/acl/CharacterSetOption.h b/src/acl/CharacterSetOption.h index e4b839604f..600a32f6e9 100644 --- a/src/acl/CharacterSetOption.h +++ b/src/acl/CharacterSetOption.h @@ -41,7 +41,7 @@ class CharacterSetOption: public TypedOption { public: typedef TypedOption Parent; - CharacterSetOption(): Parent(valueOptional) {} + explicit CharacterSetOption(const char *name): Parent(name, nullptr, valueOptional) {} }; } // namespace Acl diff --git a/src/acl/Data.h b/src/acl/Data.h index 5037bb9618..4a3d5fac17 100644 --- a/src/acl/Data.h +++ b/src/acl/Data.h @@ -22,8 +22,8 @@ public: ACLData(ACLData &&) = 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; diff --git a/src/acl/DestinationDomain.cc b/src/acl/DestinationDomain.cc index 1cec068598..60e6d68dbe 100644 --- a/src/acl/DestinationDomain.cc +++ b/src/acl/DestinationDomain.cc @@ -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; } diff --git a/src/acl/DestinationIp.cc b/src/acl/DestinationIp.cc index f0ef36e59e..a7fed9d9c4 100644 --- a/src/acl/DestinationIp.cc +++ b/src/acl/DestinationIp.cc @@ -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; } diff --git a/src/acl/ExtUser.cc b/src/acl/ExtUser.cc index 2e6ec30163..f7abacd17a 100644 --- a/src/acl/ExtUser.cc +++ b/src/acl/ExtUser.cc @@ -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 diff --git a/src/acl/ExtUser.h b/src/acl/ExtUser.h index 07e4dbbc13..aec080e522 100644 --- a/src/acl/ExtUser.h +++ b/src/acl/ExtUser.h @@ -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 *data; char const *type_; }; diff --git a/src/acl/HttpHeaderData.cc b/src/acl/HttpHeaderData.cc index c78366b7f0..a2736d4bcb 100644 --- a/src/acl/HttpHeaderData.cc +++ b/src/acl/HttpHeaderData.cc @@ -66,6 +66,12 @@ ACLHTTPHeaderData::dump() const return sl; } +const Acl::Options & +ACLHTTPHeaderData::lineOptions() +{ + return regex_rule->lineOptions(); +} + void ACLHTTPHeaderData::parse() { diff --git a/src/acl/HttpHeaderData.h b/src/acl/HttpHeaderData.h index e94cf31cc4..2599eb7ac5 100644 --- a/src/acl/HttpHeaderData.h +++ b/src/acl/HttpHeaderData.h @@ -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 * regex_rule; diff --git a/src/acl/Note.cc b/src/acl/Note.cc index 4cb3e6605a..c0e210d5d5 100644 --- a/src/acl/Note.cc +++ b/src/acl/Note.cc @@ -18,10 +18,8 @@ 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; } diff --git a/src/acl/Options.cc b/src/acl/Options.cc index ca46dd6c2f..4a7667d5fc 100644 --- a/src/acl/Options.cc +++ b/src/acl/Options.cc @@ -13,6 +13,7 @@ #include "sbuf/Stream.h" #include +#include #include 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 Names; - /// parsed ACL parameter flags that must be preserved for ACLData::parse() - static Names flagsToSkip; + using SupportedOption = std::pair; + SupportedOption supportedOption(const SBuf &name) const; const Options &options_; ///< caller-supported, linked options - const ParameterFlags ¶meterFlags_; ///< 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; diff --git a/src/acl/Options.h b/src/acl/Options.h index ddbc098c14..e45e0af471 100644 --- a/src/acl/Options.h +++ b/src/acl/Options.h @@ -13,51 +13,84 @@ #include "sbuf/forward.h" #include -#include -#include +#include -// 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; ///< 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 Options; - -/// a set of parameter flag names -typedef std::set ParameterFlags; +using Options = std::vector; /// 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 diff --git a/src/acl/RegexData.cc b/src/acl/RegexData.cc index 548575759a..ecd9c33ecd 100644 --- a/src/acl/RegexData.cc +++ b/src/acl/RegexData.cc @@ -25,15 +25,19 @@ #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 &curlist, const SBufList &RE, int flags) * called only once per ACL. */ static int -compileOptimisedREs(std::list &curlist, const SBufList &sl) +compileOptimisedREs(std::list &curlist, const SBufList &sl, const int flagsAtLineStart) { std::list 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 &curlist, const SBufList &sl) } static void -compileUnoptimisedREs(std::list &curlist, const SBufList &sl) +compileUnoptimisedREs(std::list &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); } } diff --git a/src/acl/RegexData.h b/src/acl/RegexData.h index f67c85f298..0512c0ed49 100644 --- a/src/acl/RegexData.h +++ b/src/acl/RegexData.h @@ -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 data; }; diff --git a/src/acl/ServerName.cc b/src/acl/ServerName.cc index da74395ecd..ad43affca0 100644 --- a/src/acl/ServerName.cc +++ b/src/acl/ServerName.cc @@ -119,15 +119,10 @@ ACLServerNameStrategy::match (ACLData * &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); diff --git a/src/acl/Strategised.h b/src/acl/Strategised.h index 4c9d18b915..f3814667bb 100644 --- a/src/acl/Strategised.h +++ b/src/acl/Strategised.h @@ -36,14 +36,12 @@ public: ACLStrategised(ACLData *, ACLStrategy *, 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 *data; char const *type_; ACLStrategy *matcher; @@ -77,13 +79,6 @@ ACLStrategised::typeString() const return type_; } -template -void -ACLStrategised::parseFlags() -{ - ParseFlags(options(), data->supportedFlags()); -} - template void ACLStrategised::parse() diff --git a/src/acl/UserData.cc b/src/acl/UserData.cc index 7f7de9f871..d96d312121 100644 --- a/src/acl/UserData.cc +++ b/src/acl/UserData.cc @@ -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" @@ -17,12 +18,7 @@ #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())) { diff --git a/src/acl/UserData.h b/src/acl/UserData.h index 3e3d0315bd..0a8f4862c5 100644 --- a/src/acl/UserData.h +++ b/src/acl/UserData.h @@ -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 UserDataNames_t; UserDataNames_t userDataNames; diff --git a/src/auth/AclMaxUserIp.cc b/src/auth/AclMaxUserIp.cc index 1788f10ce7..0f73ebe19d 100644 --- a/src/auth/AclMaxUserIp.cc +++ b/src/auth/AclMaxUserIp.cc @@ -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; } diff --git a/src/auth/AclProxyAuth.cc b/src/auth/AclProxyAuth.cc index b3bed5c3ec..8cec532e05 100644 --- a/src/auth/AclProxyAuth.cc +++ b/src/auth/AclProxyAuth.cc @@ -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 diff --git a/src/auth/AclProxyAuth.h b/src/auth/AclProxyAuth.h index 60d290e416..ea68538087 100644 --- a/src/auth/AclProxyAuth.h +++ b/src/auth/AclProxyAuth.h @@ -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 *data; char const *type_; diff --git a/src/ident/AclIdent.cc b/src/ident/AclIdent.cc index d944e26ee8..ed3ba43f37 100644 --- a/src/ident/AclIdent.cc +++ b/src/ident/AclIdent.cc @@ -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 diff --git a/src/ident/AclIdent.h b/src/ident/AclIdent.h index c788871e87..e45e74c9a9 100644 --- a/src/ident/AclIdent.h +++ b/src/ident/AclIdent.h @@ -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 *data; char const *type_; }; diff --git a/src/tests/stub_libauth_acls.cc b/src/tests/stub_libauth_acls.cc index 9461607412..d9d3b061d8 100644 --- a/src/tests/stub_libauth_acls.cc +++ b/src/tests/stub_libauth_acls.cc @@ -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 */