|
| 1 | +<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"> |
| 2 | +<HTML> |
| 3 | + <HEAD> |
| 4 | + <TITLE> [squid-dev] Issue with acl note (without -m) splitting helper tokens containing commas |
| 5 | + </TITLE> |
| 6 | + <LINK REL="Index" HREF="index.html" > |
| 7 | + <LINK REL="made" HREF="mailto:squid-dev%40lists.squid-cache.org?Subject=Re%3A%20%5Bsquid-dev%5D%20Issue%20with%20acl%20note%20%28without%20-m%29%20splitting%20helper%0A%20tokens%20containing%20commas&In-Reply-To=%3CCADJd0Y2eRvv9dtZKRFgzQk3bRZJCXvzxkYCAzSgqw-fweGd7MA%40mail.gmail.com%3E"> |
| 8 | + <META NAME="robots" CONTENT="index,nofollow"> |
| 9 | + <style type="text/css"> |
| 10 | + pre { |
| 11 | + white-space: pre-wrap; /* css-2.1, curent FF, Opera, Safari */ |
| 12 | + } |
| 13 | + </style> |
| 14 | + <META http-equiv="Content-Type" content="text/html; charset=us-ascii"> |
| 15 | + <LINK REL="Previous" HREF="010017.html"> |
| 16 | + <LINK REL="Next" HREF="010018.html"> |
| 17 | + </HEAD> |
| 18 | + <BODY BGCOLOR="#ffffff"> |
| 19 | + <H1>[squid-dev] Issue with acl note (without -m) splitting helper tokens containing commas</H1> |
| 20 | + <B>Andrey K</B> |
| 21 | + <A HREF="mailto:squid-dev%40lists.squid-cache.org?Subject=Re%3A%20%5Bsquid-dev%5D%20Issue%20with%20acl%20note%20%28without%20-m%29%20splitting%20helper%0A%20tokens%20containing%20commas&In-Reply-To=%3CCADJd0Y2eRvv9dtZKRFgzQk3bRZJCXvzxkYCAzSgqw-fweGd7MA%40mail.gmail.com%3E" |
| 22 | + TITLE="[squid-dev] Issue with acl note (without -m) splitting helper tokens containing commas">ankor2023 at gmail.com |
| 23 | + </A><BR> |
| 24 | + <I>Mon Apr 20 09:50:21 UTC 2026</I> |
| 25 | + <P><UL> |
| 26 | + <LI>Previous message (by thread): <A HREF="010017.html">[squid-dev] Issue with acl note (without -m) splitting helper tokens containing commas |
| 27 | +</A></li> |
| 28 | + <LI>Next message (by thread): <A HREF="010018.html">[squid-dev] NO_SPECIAL_HANDLING define for HTTP methods |
| 29 | +</A></li> |
| 30 | + <LI> <B>Messages sorted by:</B> |
| 31 | + <a href="date.html#10019">[ date ]</a> |
| 32 | + <a href="thread.html#10019">[ thread ]</a> |
| 33 | + <a href="subject.html#10019">[ subject ]</a> |
| 34 | + <a href="author.html#10019">[ author ]</a> |
| 35 | + </LI> |
| 36 | + </UL> |
| 37 | + <HR> |
| 38 | +<!--beginarticle--> |
| 39 | +<PRE>Hello, |
| 40 | + |
| 41 | +I think I’ve come up with a way to minimize the impact of removing the |
| 42 | +default comma delimiter on existing Squid setups. |
| 43 | +We can introduce a new build-time configuration option, |
| 44 | +--with-annotation-values-default-delimiter, so that users who rely on the |
| 45 | +comma separator can rebuild Squid with |
| 46 | +--with-annotation-values-default-delimiter=,. |
| 47 | +While this approach is more conceptually sound and aligns better with the |
| 48 | +documentation, I am concerned that many users rely on vendor-provided |
| 49 | +binary packages and won't have the opportunity to recompile Squid from |
| 50 | +source. |
| 51 | +Alternatively, to avoid breaking existing installations, we could keep the |
| 52 | +comma as the default but allow users to specify an empty delimiter to |
| 53 | +correctly handle tokens that may contain commas and to ensure Squid's |
| 54 | +behavior aligns with the documentation. |
| 55 | + |
| 56 | +Which of these approaches would you prefer me to implement in the PR? |
| 57 | + |
| 58 | +Kind regards, |
| 59 | + Ankor. |
| 60 | + |
| 61 | +пт, 17 апр. 2026 г. в 16:08, Andrey K <<A HREF="https://lists.squid-cache.org/listinfo/squid-dev">ankor2023 at gmail.com</A>>: |
| 62 | + |
| 63 | +><i> Hello, |
| 64 | +</I>><i> |
| 65 | +</I>><i> While working with annotations, I’ve noticed an inconsistency in how acl |
| 66 | +</I>><i> note (without the -m option) handles tokens received from helpers when |
| 67 | +</I>><i> they contain a comma. |
| 68 | +</I>><i> |
| 69 | +</I>><i> According to the documentation, an ACL like this: |
| 70 | +</I>><i> acl staff note group Staff:accountants,lawyers,security |
| 71 | +</I>><i> should match a helper response such as: |
| 72 | +</I>><i> group="Staff:accountants,lawyers,security" |
| 73 | +</I>><i> |
| 74 | +</I>><i> However, this is not the case. The helper's response is split into tokens |
| 75 | +</I>><i> using a comma as the default delimiter. As a result, only ACLs like the |
| 76 | +</I>><i> following will match: |
| 77 | +</I>><i> acl staff note group lawyers |
| 78 | +</I>><i> |
| 79 | +</I>><i> This behavior occurs because in Acl::NoteCheck::matchNotes(), a comma is |
| 80 | +</I>><i> passed as the default delimiter to the expandListEntries() function: |
| 81 | +</I>><i> bool |
| 82 | +</I>><i> ACLNoteStrategy::matchNotes(ACLData<MatchType> *noteData, const |
| 83 | +</I>><i> NotePairs *note) const |
| 84 | +</I>><i> { |
| 85 | +</I>><i> const NotePairs::Entries &entries = |
| 86 | +</I>><i> note->expandListEntries(&delimiters.value); |
| 87 | +</I>><i> for (auto e: entries) |
| 88 | +</I>><i> if (noteData->match(e.getRaw())) |
| 89 | +</I>><i> return true; |
| 90 | +</I>><i> return false; |
| 91 | +</I>><i> } |
| 92 | +</I>><i> |
| 93 | +</I>><i> After reviewing the source code, I found that the comma is added in |
| 94 | +</I>><i> src/acl/Note.h within the AnnotationCheck class constructor: |
| 95 | +</I>><i> AnnotationCheck(): delimiters(CharacterSet("__FILE__", ",")) {} |
| 96 | +</I>><i> |
| 97 | +</I>><i> Using a comma as the default delimiter makes it difficult to validate full |
| 98 | +</I>><i> tokens that may naturally contain commas. |
| 99 | +</I>><i> |
| 100 | +</I>><i> I would like to propose two changes: |
| 101 | +</I>><i> 1. Removing the default comma delimiter. |
| 102 | +</I>><i> I am prepared to submit a simple PR to exclude this comma to fix the |
| 103 | +</I>><i> incorrect matching of strings containing commas. |
| 104 | +</I>><i> However, I realize this might be a breaking change for users who currently |
| 105 | +</I>><i> rely on this implicit splitting behavior. |
| 106 | +</I>><i> |
| 107 | +</I>><i> 2. Supporting custom delimiters in helper responses. |
| 108 | +</I>><i> I also propose a PR to support a format where tag values can be passed as |
| 109 | +</I>><i> a list with a custom delimiter: |
| 110 | +</I>><i> <key>=<delimiter>"<value1><delimiter><value2>..." |
| 111 | +</I>><i> For example: |
| 112 | +</I>><i> group=,"group1,group2,group3" |
| 113 | +</I>><i> clt_con_tag=;"tag1;tag2;tag3" |
| 114 | +</I>><i> In this PR, the helper response would be tokenized based on the specified |
| 115 | +</I>><i> custom delimiter, while still supporting delimiter escaping with a |
| 116 | +</I>><i> backslash (\). |
| 117 | +</I>><i> In this scenario, having a hardcoded comma as the default delimiter in the AnnotationCheck |
| 118 | +</I>><i> class would also create complications. |
| 119 | +</I>><i> |
| 120 | +</I>><i> I would appreciate your thoughts on these proposals and whether they align |
| 121 | +</I>><i> with the project's roadmap. |
| 122 | +</I>><i> |
| 123 | +</I>><i> Kind regards, |
| 124 | +</I>><i> Ankor. |
| 125 | +</I>><i> |
| 126 | +</I>><i> |
| 127 | +</I>-------------- next part -------------- |
| 128 | +An HTML attachment was scrubbed... |
| 129 | +URL: <<A HREF="http://lists.squid-cache.org/pipermail/squid-dev/attachments/20260420/b4759344/attachment.htm">http://lists.squid-cache.org/pipermail/squid-dev/attachments/20260420/b4759344/attachment.htm</A>> |
| 130 | +</PRE> |
| 131 | + |
| 132 | +<!--endarticle--> |
| 133 | + <HR> |
| 134 | + <P><UL> |
| 135 | + <!--threads--> |
| 136 | + <LI>Previous message (by thread): <A HREF="010017.html">[squid-dev] Issue with acl note (without -m) splitting helper tokens containing commas |
| 137 | +</A></li> |
| 138 | + <LI>Next message (by thread): <A HREF="010018.html">[squid-dev] NO_SPECIAL_HANDLING define for HTTP methods |
| 139 | +</A></li> |
| 140 | + <LI> <B>Messages sorted by:</B> |
| 141 | + <a href="date.html#10019">[ date ]</a> |
| 142 | + <a href="thread.html#10019">[ thread ]</a> |
| 143 | + <a href="subject.html#10019">[ subject ]</a> |
| 144 | + <a href="author.html#10019">[ author ]</a> |
| 145 | + </LI> |
| 146 | + </UL> |
| 147 | + |
| 148 | +<hr> |
| 149 | +<a href="https://lists.squid-cache.org/listinfo/squid-dev">More information about the squid-dev |
| 150 | +mailing list</a><br> |
| 151 | +</body></html> |
0 commit comments