Re: Issue with acl note (without -m) splitting helper tokens containing commas
Andrey K <[email protected]> Wed, 22 Apr 2026 12:46:46 +0300
| Newsgroups | gmane.comp.web.squid.devel |
|---|---|
| Message-ID | <CADJd0Y0Gz1CycykX8SbYuNtJqxvDtgGdg93xfo34tBouA=kpzg@mail.gmail.com> |
--===============2111123360172676870==
Content-Type: multipart/alternative; boundary="000000000000ac3a4606500969a3"
--000000000000ac3a4606500969a3
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Hello, Alex and Amos,
I have submitted the PR: https://github.com/squid-cache/squid/pull/2410
Alex, thank you very much for your recommendations regarding the code
refactoring.
> Ideally, explicitOrDefaultValue should be private, but achieving that
> ideal requires more refactoring work, and I do not recommend it at this
> time.
I didn't see any difficulties there, so I've already made
explicitOrDefaultValue private.
> The "TODO: Some callers..." comment will need to be
> adjusted as well.
I have removed the "TODO" comment entirely since access to the storage
variable is now properly protected via accessors.
Kind regards,
Ankor.
=D0=B2=D1=82, 21 =D0=B0=D0=BF=D1=80. 2026=E2=80=AF=D0=B3. =D0=B2 17:47, Ale=
x Rousskov <[email protected]
>:
> On 2026-04-21 04:01, Amos Jeffries wrote:
> > I do not understand why we need to store annotations in their
> > serialized format in the first place.
>
> Today, we store "serialized" annotations because different ACLs may
> specify different delimiters for the same imported annotation value.
> There are two reasonable ways to look at the current design:
>
> A: This design is broken: Deserialization should happen at annotation
> import time (e.g., in helper response parser). Something like "-m"
> belongs to helper configuration, not ACLs. 2015 commit 76ee67ac was too
> focused on its "_match_ substrings of _existing_ annotations" use cases
> to realize that this is an import feature, not a matching feature.
>
> B: This design is correct: Future ACLs may specify match regexes or even
> custom matching functions that would interpret imported values
> (effectively deserializign them) differently in different contexts. For
> example, a helper may send "metadata_" annotation for a denied user
> using a different syntax than the one used for "metadata_" annotation
> for an allowed user. 2015 commit 76ee67ac got it right: We should place
> "-m" inside ACLs, just like we place "-i" inside ACLs. If helpers want
> Squid to import a list of values, we should add such support, but that
> is a separate matter.
>
>
> >>> 2. Supporting custom delimiters in helper responses.
> >>> I also propose a PR to support a format where tag values can be
> >>> passed as a list with a custom delimiter:
> >>> <key>=3D<delimiter>"<value1><delimiter><value2>..."
> >>> For example:
> >>> group=3D,"group1,group2,group3"
> >>> clt_con_tag=3D;"tag1;tag2;tag3"
> >>> In this PR, the helper response would be tokenized based on the
> >>> specified custom delimiter, while still supporting delimiter escaping
> >>> with a backslash (\).
> >>
> >> I do not think this hack will work well as is, without syntax
> >> modifications because Squid already uses double quotes specially in
> >> this context. Overloading quotation meaning would be confusing/wrong.
> >>
> >> Overall, I am not excited about this hack, but let's start with these
> >> questions about its scope:
> >>
> >> * Can the same effect be achieved today by sending a helper response
> >> containing multiple same-name annotations? For example:
> >>
> >> group=3Dgroup1 group=3Dgroup2 group=3Dgroup3
> >>
> >
> > No. That will be added as three different kv-pair by the helper logic.
>
> If the above does add three different name=3Dvalue annotations with the
> same name, then the answer to my question is "Yes" rather than "No".
>
> I did _not_ test this, but, after looking at
> Helper::Reply::parseResponseKeys() code, I suspect that, yes, "the same
> effect be achieved today by sending a helper response containing
> multiple same-name annotations". If that untested theory is correct,
> then we do not need an additional hack to allow helpers to send Squid a
> list of same-name values.
>
> Alex.
>
> _______________________________________________
> squid-dev mailing list
> [email protected]
> https://lists.squid-cache.org/listinfo/squid-dev
>
--000000000000ac3a4606500969a3
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
<div dir=3D"ltr">Hello, Alex and Amos,<div><br></div><div>I have submitted =
the PR:=C2=A0<a href=3D"https://github.com/squid-cache/squid/pull/2410" tar=
get=3D"_blank">https://github.com/squid-cache/squid/pull/2410</a></div><div=
><br></div><div>Alex, thank you very much for your recommendations regardin=
g the code refactoring.</div><div><br></div><div>> Ideally, explicitOrDe=
faultValue should be private, but achieving that<br>> ideal requires mor=
e refactoring work, and I do not recommend it at this<br>> time.</div><d=
iv><br></div><div>I didn't see any difficulties there, so I've alre=
ady made=C2=A0<font face=3D"monospace">explicitOrDefaultValue</font>=C2=A0p=
rivate.<br><br><br>> The "TODO: Some callers..." comment will =
need to be<br>> adjusted as well.</div><div><br></div><div>I have remove=
d the=C2=A0<font face=3D"monospace">"TODO"</font>=C2=A0comment en=
tirely since access to the storage variable is now properly protected via a=
ccessors.</div><div><br></div><div>Kind regards,</div><div>=C2=A0 =C2=A0 An=
kor.</div></div><br><div class=3D"gmail_quote gmail_quote_container"><div d=
ir=3D"ltr" class=3D"gmail_attr">=D0=B2=D1=82, 21 =D0=B0=D0=BF=D1=80. 2026=
=E2=80=AF=D0=B3. =D0=B2 17:47, Alex Rousskov <<a href=3D"mailto:rousskov=
@measurement-factory.com">[email protected]</a>>:<br></di=
v><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;borde=
r-left:1px solid rgb(204,204,204);padding-left:1ex">On 2026-04-21 04:01, Am=
os Jeffries wrote:<br>
> I do not understand why we need to store annotations in their <br>
> serialized format in the first place.<br>
<br>
Today, we store "serialized" annotations because different ACLs m=
ay <br>
specify different delimiters for the same imported annotation value. <br>
There are two reasonable ways to look at the current design:<br>
<br>
A: This design is broken: Deserialization should happen at annotation <br>
import time (e.g., in helper response parser). Something like "-m"=
; <br>
belongs to helper configuration, not ACLs. 2015 commit 76ee67ac was too <br=
>
focused on its "_match_ substrings of _existing_ annotations" use=
cases <br>
to realize that this is an import feature, not a matching feature.<br>
<br>
B: This design is correct: Future ACLs may specify match regexes or even <b=
r>
custom matching functions that would interpret imported values <br>
(effectively deserializign them) differently in different contexts. For <br=
>
example, a helper may send "metadata_" annotation for a denied us=
er <br>
using a different syntax than the one used for "metadata_" annota=
tion <br>
for an allowed user. 2015 commit 76ee67ac got it right: We should place <br=
>
"-m" inside ACLs, just like we place "-i" inside ACLs. =
If helpers want <br>
Squid to import a list of values, we should add such support, but that <br>
is a separate matter.<br>
<br>
<br>
>>> 2. Supporting custom delimiters in helper responses.<br>
>>> I also propose a PR to support a format where tag values can b=
e <br>
>>> passed as a list with a custom delimiter:<br>
>>> =C2=A0=C2=A0 =C2=A0 <key>=3D<delimiter>"<v=
alue1><delimiter><value2>..."<br>
>>> For example:<br>
>>> =C2=A0=C2=A0 =C2=A0 group=3D,"group1,group2,group3"<=
br>
>>> =C2=A0=C2=A0 =C2=A0 clt_con_tag=3D;"tag1;tag2;tag3"<=
br>
>>> In this PR, the helper response would be tokenized based on th=
e <br>
>>> specified custom delimiter, while still supporting delimiter e=
scaping <br>
>>> with a backslash (\).<br>
>><br>
>> I do not think this hack will work well as is, without syntax <br>
>> modifications because Squid already uses double quotes specially i=
n <br>
>> this context. Overloading quotation meaning would be confusing/wro=
ng.<br>
>><br>
>> Overall, I am not excited about this hack, but let's start wit=
h these <br>
>> questions about its scope:<br>
>><br>
>> * Can the same effect be achieved today by sending a helper respon=
se <br>
>> containing multiple same-name annotations? For example:<br>
>><br>
>> =C2=A0=C2=A0=C2=A0=C2=A0 group=3Dgroup1 group=3Dgroup2 group=3Dgro=
up3<br>
>><br>
> <br>
> No. That will be added as three different kv-pair by the helper logic.=
<br>
<br>
If the above does add three different name=3Dvalue annotations with the <br=
>
same name, then the answer to my question is "Yes" rather than &q=
uot;No".<br>
<br>
I did _not_ test this, but, after looking at <br>
Helper::Reply::parseResponseKeys() code, I suspect that, yes, "the sam=
e <br>
effect be achieved today by sending a helper response containing <br>
multiple same-name annotations". If that untested theory is correct, <=
br>
then we do not need an additional hack to allow helpers to send Squid a <br=
>
list of same-name values.<br>
<br>
Alex.<br>
<br>
_______________________________________________<br>
squid-dev mailing list<br>
<a href=3D"mailto:[email protected]" target=3D"_blank">squid-=
[email protected]</a><br>
<a href=3D"https://lists.squid-cache.org/listinfo/squid-dev" rel=3D"norefer=
rer" target=3D"_blank">https://lists.squid-cache.org/listinfo/squid-dev</a>=
<br>
</blockquote></div>
--000000000000ac3a4606500969a3--
--===============2111123360172676870==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
_______________________________________________
squid-dev mailing list
[email protected]
https://lists.squid-cache.org/listinfo/squid-dev
--===============2111123360172676870==--