squid-cache · FireBurn · Jul 9, 2026 · Jul 12, 2026 · Jul 13, 2026 · rousskov
diff --git a/CONTRIBUTORS b/CONTRIBUTORS
@@ -367,6 +367,7 @@ Thank you!
     Michele Bergonzoni <bergonz@labs.it>
     Miguel A.L. Paraz <map@iphil.net>
     Mike Groeneweg <mikeg@scorpion.murdoch.edu.au>
+    Mike Lothian <mike@fireburn.co.uk>
     Mike Mitchell <mike.mitchell@sas.com>
     Mikio Kishi <mkishi@104.net>
     Milen Pankov <mail@milen.pankov.eu>

diff --git 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
@@ -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
@@ -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
@@ -0,0 +1 @@
+.example.com
Original file line number	Diff line number	Diff line change
Expand Up		@@ -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
rousskov Jul 9, 2026 Copy link Copy Markdown Contributor Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. I hope we are done: If automated PR ... tests succeed They failed. I do not know what exactly causes those failures. The same change appears to work in my local quick-and-dirty tests, but I did not test the latest master (yet). The missing `est` letters after `configuration context: t` in the logged message quoted below are particularly alarming (to me): `2026/07/09 21:44:38\| ERROR: Can not open file bad-acl-dstdomain-dupe.domains for reading configuration context: t-suite/squidconf/bad-acl-dstdomain-dupe.conf(21) acl` There are other similarly malformed messages in that log as well. I will find the time to investigate this further if you do not. rousskov Jul 9, 2026 Copy link Copy Markdown Contributor Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. I have (the bigger part of) the answer: There is another Squid bug here. I fixed that bug more than a year ago while working on other Squid configuration improvements, but that fix is still in the backlog. It explains why your (correct) changes work in my tests but not in the official Squid Project CI. I will post a dedicated PR to fix that other bug, so that your PR can be merged afterwords, hopefully without changes. Please stand by. yadij Jul 10, 2026 Copy link Copy Markdown Contributor Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. FYI: I believe the missing letters are probably `tes`. Likely Squid trying badly to remove a relative path prefix (eg "../"). rousskov Jul 10, 2026 Copy link Copy Markdown Contributor Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. I will post a dedicated PR to fix that other bug Posted at #2458 . I have tested this PR changes on top of #2458 and saw the expected behavior, including a test failure if this PR primary fix change is reverted. I plan to clear this PR for merging after #2458 lands. rousskov Jul 10, 2026 Copy link Copy Markdown Contributor Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. FYI: I believe the missing letters are probably `tes`. As I noted earlier, in the posted sample, the missing letters are `est`. The same CI log contains other missing letters as well: `configuration context: iguration(8) acl` Likely Squid trying badly to remove a relative path prefix (eg "../"). This is a different bug. I fixed this one in December 2024. That fix is also in the backlog. Hopefully, we will eventually find a way to resolve the long-standing issues that have created (and keep growing!) that backlog, badly hurting the Squid Project, but doing so is outside this PR scope. yadij Jul 11, 2026 • edited Loading Copy link Copy Markdown Contributor Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. I read your OP. It seems very strange that you think that. `Debug::Extra << "configuration context: " << ConfigParser::CurrentLocation();` `ConfigParser::CurrentLocation() { return ToSBuf(SourceLocation(cfg_directive, cfg_filename, config_lineno)); }` `SourceLocation::print(std::ostream &os) { if (fileName) { os << SkipBuildPrefix(fileName); ...` `SkipBuildPrefix(const char* path) { ... return path + ToSkip; }` Much more likely that `ToSkip` is somehow `N+3` when it should be `N` instead of that logic removing `est` from the middle of "test-suite...". The fix for above bug still being in the backlog would explain why the current code shows it still present.