<div dir="ltr">Hello,<div><br></div><div>I think I’ve come up with a way to minimize the impact of removing the default comma delimiter on existing Squid setups.</div><div>We can introduce a new build-time configuration option, <font face="monospace">--with-annotation-values-default-delimiter</font>, so that users who rely on the comma separator can rebuild Squid with <font face="monospace">--with-annotation-values-default-delimiter=,</font>. </div><div>While this approach is more conceptually sound and aligns better with the documentation, I am concerned that many users rely on vendor-provided binary packages and won't have the opportunity to recompile Squid from source. </div><div>Alternatively, to avoid breaking existing installations, we could keep the <font face="monospace">comma</font> as the default but allow users to specify an empty delimiter to correctly handle tokens that may contain commas and to ensure Squid's behavior aligns with the documentation. </div><div><br></div><div>Which of these approaches would you prefer me to implement in the PR?</div><div><br></div><div>Kind regards,</div><div> Ankor.</div></div><br><div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">пт, 17 апр. 2026 г. в 16:08, Andrey K <<a href="mailto:ankor2023@gmail.com">ankor2023@gmail.com</a>>:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Hello,<div><br>While working with annotations, I’ve noticed an inconsistency in how <font face="monospace">acl note</font> (without the <font face="monospace">-m</font> option) handles tokens received from helpers when they contain a comma.<br><br></div><div>According to the documentation, an ACL like this:<br><font face="monospace"> acl staff note group Staff:accountants,lawyers,security</font><br>should match a helper response such as:<br><font face="monospace"> group="Staff:accountants,lawyers,security"</font><br><br><div>However, this is not the case. The helper's response is split into tokens using a comma as the default delimiter. As a result, only ACLs like the following will match:</div><div><font face="monospace"> acl staff note group lawyers</font><br><br></div><div>This behavior occurs because in Acl::NoteCheck::matchNotes(), a comma is passed as the default delimiter to the expandListEntries() function:</div><div><font face="monospace"> bool<br> ACLNoteStrategy::matchNotes(ACLData<MatchType> *noteData, const NotePairs *note) const<br> {<br> const NotePairs::Entries &entries = note->expandListEntries(&delimiters.value);<br> for (auto e: entries)<br> if (noteData->match(e.getRaw()))<br> return true;<br> return false;<br> }<br></font></div><div><font face="monospace"><br></font></div><div>After reviewing the source code, I found that the comma is added in src/acl/Note.h within the AnnotationCheck class constructor:<br><font face="monospace"> AnnotationCheck(): delimiters(CharacterSet("__FILE__", ",")) {}</font><br><br></div><div>Using a comma as the default delimiter makes it difficult to validate full tokens that may naturally contain commas.<br><br></div><div>I would like to propose two changes:<br>1. Removing the default comma delimiter.</div><div>I am prepared to submit a simple PR to exclude this comma to fix the incorrect matching of strings containing commas. </div><div>However, I realize this might be a breaking change for users who currently rely on this implicit splitting behavior.</div><div><br>2. Supporting custom delimiters in helper responses.</div><div>I also propose a PR to support a format where tag values can be passed as a list with a custom delimiter:<br><font face="monospace"> <key>=<delimiter>"<value1><delimiter><value2>..."</font><br>For example:<br><font face="monospace"> group=,"group1,group2,group3"<br> clt_con_tag=;"tag1;tag2;tag3"</font><br>In this PR, the helper response would be tokenized based on the specified custom delimiter, while still supporting delimiter escaping with a backslash (<font face="monospace">\</font>).<br>In this scenario, having a hardcoded comma as the default delimiter in the <font face="monospace">AnnotationCheck </font>class would also create complications.</div><div><br>I would appreciate your thoughts on these proposals and whether they align with the project's roadmap.</div><div><br></div><div>Kind regards,</div><div> Ankor.</div><div><br></div></div></div>
</blockquote></div>