[squid-dev] [PATCH] annotate_transaction ACL

[Christos Tsantilas]

The adjusted patch which implements the new acls applied to squid-5  as 
r15024 and r15026.
The patch which fixed  Auth::UserRequest::denyMessage() method applied 
to squid-5 as r15025



On 27/01/2017 08:05 μμ, Alex Rousskov wrote:
> On 01/27/2017 10:39 AM, Christos Tsantilas wrote:
>
>> Which is the status of this patch?
>
> Based on squid-dev archives, this patch has been posted 27 days ago and
> received no reviews, comments, or votes (but you already know that).
>
>
>> Can be applied to squid-5?
>
> Yes it can be. MergeProcedure says that "Submissions older than 10 days
> without negative votes are accepted". When there has been no discussion
> (or no clear consensus during a discussion), it is customary to warn
> folks that a posted patch is going to be committed ASAP, to give others
> the last chance to object. Let's consider this email to be that final
> warning.
>
>
> When you commit this, I suggest reformatting
>
>> acl aclname annotate_... # [fast]
>>   #
>>   # Always matches. Used for its side effect: This ACL immediately
>
> as
>
>> acl aclname annotate_...
>>   # Always matches. [fast]
>>   # Used for its side effect: This ACL immediately ...
>
> to simplify and for consistency sake.
>
>
> Thank you,
>
> Alex.
>
>
>
>> On 02/01/2017 12:19 πμ, Eduard Bagdasaryan wrote:
>>> Hello,
>>>
>>>
>>> The "annotate transaction" patch implements  two new ACLs:
>>> annotate_transaction and annotate_client. Please apply this patch first.
>>>
>>> Both ACLs always match and are useful for their side effect, immediately
>>> adding a key-value pair to the current transaction annotation
>>> (annotate_transaction) or to the current client-to-Squid connection
>>> annotation (annotate_client).  Connection annotations are propagated to
>>> the current and all future master transactions on the annotated
>>> connection. Before this patch only 'clt_conn_tag' annotation tag could
>>> be used for a connection annotation.
>>>
>>> To reuse the existing notes parsing code, I had to refactor Note, Notes
>>> and NotePairs classes:
>>>
>>> * Made data members private and adjusted to follow 'rule of three'.
>>>   Having public assess to containers with pointers may cause memory
>>>   problems: for example ExternalACLEntry::update() called directly
>>>   notes.entries.clear() without deleting the pointers.
>>> * None-fatal check for 'special' characters inside note name.
>>> * Used SBufs instead of Strings and const char* where possible.
>>> * Adjusted ACLNoteStrategy::matchNotes() to avoid 'expanding quoted
>>> values'
>>>   code duplication inside
>>>
>>> Also fixed acl quoted flag parameters syntax. The old code improperly
>>> required quoting both flag and its parameter, e.g., "-m= ," whereas
>>> only parameter should be quoted: -m=" ,".
>>>
>>> Also moved UpdateRequestNotes() from Notes.cc to HttpRequest.cc to
>>> resolve dependency problems while bulding unit tests.
>>>
>>> TODO: transaction annotation matching code (ACLNoteData) performs
>>> parsing in its own way, using ACLStringData::parse(), lacking special
>>> characters/reserved keywords checks. Consider reusing the existing
>>> Notes parsing code instead.
>>>
>>>
>>> The "helper deny message" patch fixes Auth::UserRequest::denyMessage()
>>> misuse.
>>>
>>> I believe this method was improperly used in contexts where actually
>>> Auth::UserRequest::setDenyMessage() expected. Probably the reason is
>>> that both denyMessage() and getDenyMessage() were not constant,
>>> provoking such 'misuse'.
>>>
>>> Also placed some common code into UserRequest::denyMessageFromHelper(),
>>> eliminating code duplication. Though there are many places
>>> inside auth/ntlm/UserRequest.cc and auth/negotiate/UserRequest.cc
>>> where code is still duplicated.
>>>
>>>
>>> Thanks,
>>> Eduard.
>>>
>> _______________________________________________
>> squid-dev mailing list
>> squid-dev at lists.squid-cache.org
>> http://lists.squid-cache.org/listinfo/squid-dev
>
>