On 4/07/2012 4:54 p.m., Alex Rousskov wrote:
> On 07/03/2012 08:10 PM, Amos Jeffries wrote:
>
>>>> [channel-ID] (OK/ERR/BH) [key-pairs] <blob> <terminator>
>>> How will the generic code be able to tell where key-pairs end and blob
>>> begins?
>> The generic code will have a format for key-pair of ( 1#token "="
>> 1#(token / quoted-string) ) [to be finalized still] with a set of known
>> key names. If the bytes being parsed do not match that format and keys
>> its not part of the key-pairs.
> There are two problems with this approach, IMO:
>
> * It cannot handle a body that just happened to start with supported
> key-value pair characters.
This is shared with any body which just happens to start with the "BS"
token as well. This is a good reason to use key-pair across the board
and not document any <blob> support.
>
> * It cannot detect a case of helper sending unsupported key-value
> pairs (if that helper may send a body too).
Yes.
> It also requires extra code/work to pre-register all known key names,
> but that is minor.
This is not an issue. We have to code operations for handling any
accepted key-pair in the handler callbacks anyway.
>
>
>> The plan for patch-#2 is to discuss key-pair syntax (which we seem to
>> have started), and move some particular key-pair parsing from helper
>> handlers into the shared one. Opening up user=, tag=, password=, log=,
>> message= keys to non-ACL helper responses will be VERY useful for RADIUS
>> and systems needing to maintain cross-helper request state.
> I think we should keep it as simple as possible without causing problems
> with existing parsers and leaving some room for future extensions:
>
> * key/name: alphanumeric char followed by anything except (whitespace or
> '=' or terminator)
>
> * value: either quoted-string or anything except (whitespace or
> terminator); could be empty
>
> The parser strips whitespace and strips quotes around quoted strings.
> Quoted strings use backslash to escape any octet.
Okay. So modelling on the ssl_crtd parser instead of external_acl_type
parser.
>
>
>>> If you want one format for all, you probably need something like
>>>
>>> [channel-ID] (OK/ERR/BH) [key-pairs] [BS <body>] <terminator>
>>>
>>> where "BS" is some special marker that starts all bodies and cannot be
>>> found in key-pairs. Since any body-carrying message is likely to have
>>> new lines (and, hence, would need a non-newline terminator), this BS
>>> sequence could be something like "body:\n", I guess.
>> We REQUIRE a format that does not rely on new special markers like that
>> in order to cope with the existing helpers responses in a
>> backward-compatible way without additional user configuration.
> When your parser sees "n=v " after ERR and "n" is a known key name, how
> would it distinguish these two possibilities:
>
> * This is a start of a <blob>
> * This is a key=value pair
The second. *IF* it was in the [key-pair] area directly after ERR. Once
<blob> starts everything is <blob>.
>> Or would you advocate moving the external_acl_type "protocol=N.N" config
>> option into the helper child object for use by all helpers on
>> determining what key-pairs and format is accepted?
> I am probably missing some important use cases, but my generic parser
> would just call a virtual method for each key=value pair found (with no
> pre-registration of supported keys) and another virtual method for the
> <body> blob, if any. It will parse everything using the key, value, and
> BS formats discussed above.
You mean function pointers right? not virtual methods in the HelperReply
object?
and um, this is a high-use code path - both in parsing and in
retrieving the details out of HelperReply.
That style of generic parser is good for new formats but not so much for
backward-compatibility from old formats. I am going to have to manually
receive things like the TT/LD/AF/NA NTLM result codes and map the
existing format into record of the HelperReply object.
Amos
Received on Wed Jul 04 2012 - 06:02:24 MDT
This archive was generated by hypermail 2.2.0 : Thu Jul 05 2012 - 12:00:03 MDT