]> git.ipfire.org Git - thirdparty/squid.git/commit
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)
commit8d76389c009124d15d92c85a6ad4c17f38fa2705
tree1c97692275a1b563d4ebbfdb113cd93506cdb886
parenta00584a13044548c8effc19b94c96e6211f13200
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.
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