[asterisk-dev] [Code Review]: AMI version 1.4 Specification Review Request

Matt Jordan reviewboard at asterisk.org
Sun Jan 13 21:45:09 CST 2013



> On Jan. 8, 2013, 2:57 p.m., tim_ringenbach wrote:
> > In section 4.2, I'd like to see the message format spelled out a little bit more. What characters are allowed in the key, and in the value? Is there any escaping mechanism for embedded new lines, or colons in the key? I'm also thinking about this bug I filed a while back:  https://issues.asterisk.org/jira/browse/ASTERISK-20369

In general, I'd prefer to just define that keys don't contain ':'. Values can contain a ':' without error, as the end of a value is denoted by '\r\n'. Values should not contain '\r\n'. The problem with trying to handle these kinds of escaping issues with the current syntax as that we're going to end up creating mechanisms that exist in better forms of syntax (be it something akin to URI encoding, CDATA tags, or what-have-you), and if we do that, we might as well just drop-kick the current syntax into the trash can and use XML or JSON. Part of me is inclined to do that, but there's breaking existing implementations, and then there's setting them on fire and watching them burn. I was going for the former rather than the latter :-)

On to the channelvars setting...

I think using the nomenclature "ChannelVars({channel_name}): value" was probably a mistake. First, the channel name is usually complex and only obtained once a NewChannel event is received. Using a value in an event to parse out keys in the same event feels wrong. If the reason for using this nomenclature was for events that contain multiple channels, the nomenclature "ChannelVars1","ChannelVars2" could have been used and - while silly - would at least be consistent with "Channel1", "Channel2".

tl;dr: we should define more explicitly how to parse a line in an action/event; re-think ChannelVars entirely; and not try to kill ourselves too much on the syntax since we'd be better served by just supporting JSON/XML instead.


- Matt


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2269/#review7638
-----------------------------------------------------------


On Jan. 8, 2013, 8:41 a.m., Matt Jordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2269/
> -----------------------------------------------------------
> 
> (Updated Jan. 8, 2013, 8:41 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> A proposed revamp of the AMI protocol has been written and is available for discussion:
> 
> https://wiki.asterisk.org/wiki/display/AST/AMI+1.4+Specification
> 
> Please note that this will change the AMI protocol significantly, and represents a major shift in how the protocol represents operations in Asterisk to consumers of the protocol.
> 
> 
> Diffs
> -----
> 
> 
> Diff: https://reviewboard.asterisk.org/r/2269/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Matt
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130114/eba50d9d/attachment.htm>


More information about the asterisk-dev mailing list