Re: Helpers and Notes

Hi all,
 I bring back this thread because we have a related project we need to
close.

Also I believe that the notes interface and usage is currently a little
broken so I believe we should fix it.
There are thinks which will never work well if we continue using notes
like this.

For example assume that a url-rewriter return a note1 note. So assume
the following configuration:

url_rewrite_program /usr/local/squid/bin/line-helper.sh OK note1=value1
url_rewrite_children 1
request_header_add X-Cookie "cookie=%{note1}note"

This is not expected to work, because we are copying the Notes to the
AccessLogEntry just before logging the request. The formatter gets the
notes to print from the AccessLogEntry object.

Because the %note needs to print both ICAP/eCAP and helper notes we need
to find a common way to use Notes, and store ICAP/eCAP and helper notes
to the same structure.
This is will help us to access the notes any time we need them, without
conversions from NotePairs to Notes or the reverse.

In the future we may need to add more squid features which operates on
both ICAP/eCAP and helpers notes, for example a new ACL.

I am attaching a patch which stores the helper notes to a NotePairs
class (as ICAP/eCAP does).
This patch:
 - Converts the HttpRequest::helperNotes to a NotePairs object
 - Adds three members to NotePairs:
   1) NotePairs::findFirst which return the first matched note for a key
   2) NotePairs::find which return as value a comma separated list of
values of the notes matching the key.
   3) - NotePairs::add to add a key/value pair
 - removes the uneeded methods from Note and Notes classes.
 - Try to hide any references to the HttpHeaders

I do not believe that this patch breaks anything. Moreover I believe
that this patch makes the code a little faster and better.

Maybe requires a little more testing, but if we decide that it is in the
right direction I will do it.

Next steps:
   - Remove the AccessLogEntry::notes
   - Rename HttpRequest::helperNotes to HttpRequest::notes
   - Store ICAP/eCAP notes to HttpRequest::notes
   - Probably rewrite the NotePairs to not a new classes which is not
based on the HttpHeader
   - Rename the Note/Notes/NotePairs classes to avoid confusion in the
future.

Regards,
   Christos

On 01/28/2013 01:51 PM, Tsantilas Christos wrote:
> To summarize, lets try to see what we currently have and then see what
> we need.
>
> Currently:
> 1) the helpers may return multiple kv-pairs
>
> 2) the helper may use non valid characters for HTTP headers for key
> names. This is work well now, but we are worrying that in the future it
> may not work, because the HttpHeader API will be changed.
>
> 3) the helper currently can return a quoted value eg group="J, Smith"
> OR a url encoded value eg group=J%2C%20Smith.
>
> 4) the helper may return multiple kv-pairs with the same key
>
> 5) In most cases(?) used only the first returned value for a key. For
> example if multiple "tag=" kv-pairs returned by the helper only the
> first will be used in tag ACL.
>
> 6) If more than one kv-pairs with the same key returned then in
> access-log we will print them as a list. For example if the helper
> returned 'OK user="J, Smith" group="group1,A" group=group2' the
> formating code "%note{group}" will print:
> "group1,A,group2".
>
> 7) In the above helper reply the formating code %note will print:
> "user: J, Smith\r\ngroup: group1,A\r\ngroup: group2"
> I must note here that the two "group" kv-pair keys will be printed as
> separate entries.
>
>