[asterisk-dev] [Code Review]: Improve AMI long line error handling

[David Lee]

> On Oct. 4, 2012, 1:19 p.m., opticron wrote:
> > /certified/branches/1.8.15/main/manager.c, lines 4903-4918
> > <https://reviewboard.asterisk.org/r/2142/diff/2/?file=31720#file31720line4903>
> >
> >     Perhaps something along the lines of this:
> >     } else {
> >         switch (s->parsing) {
> >         case MESSAGE_OKAY:
> >             res = process_message...;
> >             break;
> >         case ...:
> >             /* error handling */
> >             ...
> >             break;
> >         }
> >     }
> >     
> >     This avoids the awkward handling of s->parsing, the assert, and the avoidance of the warning.

Yeah; I thought about that. But that would encourage duplication of the error handling (which I was happy to get rid of when I put in the switch case). I'll think about it.


- David


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


On Oct. 4, 2012, 11:09 a.m., David Lee wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2142/
> -----------------------------------------------------------
> 
> (Updated Oct. 4, 2012, 11:09 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> In AMI's parser, when it receives a long line (> 1024 characters), it discards that line, but continues to process the message normally.
> 
> Typically, this is not a problem because a) who has lines that long and b) usually a discarded line results in an invalid message. But if that line is specifying an optional field, then the message will be processed, you get a 'Response: Success', but things don't work the way you expected them to.
> 
> This patch changes the behavior when a line-too-long parse error occurs.
> * Changes the log message to avoid way-too-long (and truncated anyways) log messages
> * Adds a 'parsing' status flag to Response: Success
> * Sets parsing = MESSAGE_LINE_TOO_LONG if, well, a line is too long
> * Responds with an appropriate error if parsing != MESSAGE_OKAY
> 
> 
> This addresses bug AST-961.
>     https://issues.asterisk.org/jira/browse/AST-961
> 
> 
> Diffs
> -----
> 
>   /certified/branches/1.8.15/main/manager.c 374429 
> 
> Diff: https://reviewboard.asterisk.org/r/2142/diff
> 
> 
> Testing
> -------
> 
> Used netcat to send commands with AMI, verifying that commands with long lines were rejected, and commands without long lines received after that were accepted.
> 
> 
> Thanks,
> 
> David
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20121004/6f629b9b/attachment-0001.htm>