From d6a0ee12c9aceebedc38ed064b38f30c35ab3bd8 Mon Sep 17 00:00:00 2001 From: Mike Lothian Date: Thu, 9 Jul 2026 14:41:24 +0100 Subject: [PATCH] Honor directive parameters after a quoted filename reference For example, the following ACL did not match `example.com`: acl foo dstdomain "file-of-domains.txt" example.com strtokFile() implements the "read directive parameters from a quoted file" syntax used when configuration_includes_quoted_values is off (the documented default). Once the named file's lines were exhausted, strtokFile() returned nullptr, signalling end-of-directive to callers such as ACLDomainData::parse(). Any tokens following the quoted filename on the same configuration line (e.g., "example.com" above) were silently discarded. A parameter file reference and trailing literal parameters are valid syntax individually; combining them on one line has always been allowed by the grammar, so the omission was "invisible" to admins. This bug dates back to 2008 commit f32cd13e ("Replace cnfig parser gotos with do-while loop."), which replaced a goto that resumed parsing after the file's last line with an unconditional "return nullptr". Every Squid release starting with v3.1.0.1 is affected. Signed-off-by: Mike Lothian --- CONTRIBUTORS | 1 + src/ConfigParser.cc | 4 +++- test-suite/squidconf/bad-acl-dstdomain-dupe.conf | 2 ++ test-suite/squidconf/bad-acl-dstdomain-dupe.conf.instructions | 4 ++++ test-suite/squidconf/bad-acl-dstdomain-dupe.domains | 1 + 5 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 test-suite/squidconf/bad-acl-dstdomain-dupe.domains diff --git a/CONTRIBUTORS b/CONTRIBUTORS index 23334bd09c3..33d38274e48 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -367,6 +367,7 @@ Thank you! Michele Bergonzoni Miguel A.L. Paraz Mike Groeneweg + Mike Lothian Mike Mitchell Mikio Kishi Milen Pankov diff --git a/src/ConfigParser.cc b/src/ConfigParser.cc index 6dfa92f3cfd..90a5dc676c2 100644 --- a/src/ConfigParser.cc +++ b/src/ConfigParser.cc @@ -111,7 +111,9 @@ ConfigParser::strtokFile() fclose(wordFile); wordFile = nullptr; fromFile = 0; - return nullptr; + t = buf; + *t = '\0'; + // and resume parsing post-"file" input, if any } else { char *t2, *t3; t = buf; diff --git a/test-suite/squidconf/bad-acl-dstdomain-dupe.conf b/test-suite/squidconf/bad-acl-dstdomain-dupe.conf index 52f0a39c925..9807cf104b3 100644 --- a/test-suite/squidconf/bad-acl-dstdomain-dupe.conf +++ b/test-suite/squidconf/bad-acl-dstdomain-dupe.conf @@ -17,3 +17,5 @@ acl test41 dstdomain .d.example.com .example.com acl test42 dstdomain .example.com .e.example.com acl test51 dstdomain example.com example.net . + +acl test61 dstdomain "bad-acl-dstdomain-dupe.domains" after-file.example.com diff --git a/test-suite/squidconf/bad-acl-dstdomain-dupe.conf.instructions b/test-suite/squidconf/bad-acl-dstdomain-dupe.conf.instructions index ff18d9c66a7..e94a11534d2 100644 --- a/test-suite/squidconf/bad-acl-dstdomain-dupe.conf.instructions +++ b/test-suite/squidconf/bad-acl-dstdomain-dupe.conf.instructions @@ -28,4 +28,8 @@ WARNING: Ignoring earlier .d.example.com because it is covered by .example.com WARNING: Ignoring .e.example.com because it is already covered by .example.com advice: Remove value .e.example.com from the ACL acl name: test42 + +WARNING: Ignoring after-file.example.com because it is already covered by .example.com + advice: Remove value after-file.example.com from the ACL + acl name: test61 END diff --git a/test-suite/squidconf/bad-acl-dstdomain-dupe.domains b/test-suite/squidconf/bad-acl-dstdomain-dupe.domains new file mode 100644 index 00000000000..c30ac315107 --- /dev/null +++ b/test-suite/squidconf/bad-acl-dstdomain-dupe.domains @@ -0,0 +1 @@ +.example.com