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

[opticron]

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



/certified/branches/1.8.15/main/manager.c
<https://reviewboard.asterisk.org/r/2142/#comment13889>

    Assuming this code is targeted toward future changes, this would be better served by a switch that handles all possible values of the enum (no default) since building Asterisk in dev mode will catch this at compile time and force the developer to handle the new case statement instead of handling it at runtime with a generic error.


- opticron


On Oct. 2, 2012, 11:07 a.m., David Lee wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2142/
> -----------------------------------------------------------
> 
> (Updated Oct. 2, 2012, 11:07 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 long-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 374105 
> 
> 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/20121002/f034e5b9/attachment.htm>