Re: [PATCH] HttpRequest::helperNotes to NotePairs

On 16/03/2013 7:11 a.m., Tsantilas Christos wrote:
> This patch converts the HttpRequest::helperNotes to a NotePairs object
> This patch is similar with the one posted under the "Helper and Notes"
> thread and just add a new method:
> NotePairs::hasPair to check if a key/value pair is stored
>
>
> We need to discuss, how to handle multiple notes with the same key.
> Assume that you have the following answer from a helper:
> OK user="J, Smith" group="group1,A" group=group2
>
> Currently (with or without this patch), formating code "%note{group}"
> will print:
> "group1,A,group2"
>
> 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.
>
> How should we handle this case? Is it OK as is?
>
> We need to decide if the ',' will be used as separator or as valid
> values character.

Both?

IMO when we are printing the syntax key '=' value, it makes a lot of
sense to use the HTTP ABNF format which uses both encoded token or
'bare' quoted-string ("...") for any value with reserved characters such
as '',' or ':' in our notes. The simple way to do that is to just
quoted-string all values on %note and print as duplicate keys in both
formats.

Notice that this also makes both output formats more similar and that
the only limitation on using (key=A,"B,b",c) to represent (key=A
key="B,b" key=c) is bugs in our own strwordtok() implementation.

I audited most of this in the earlier thread and am happy with those
parts for now.

Please don't implement hasPair using hasByNameListMember(). The whole
set of HttpHeader 'strList' manipulator functions are rather nasty in
the way they build the headers into strings before re-parsing the
values. They also take a side-trip through the HTTP reserved headers
matching which is completely unnecessary for Notes.
  The simple loops you have for find() should work a lot faster despite
the string comparision on each key.

Amos
Received on Fri Mar 15 2013 - 23:18:37 MDT