[squid-dev] [PATCH] annotate_transaction ACL

[Alex Rousskov]

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