[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/#review7222
-----------------------------------------------------------



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

    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.


- opticron


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/584820cf/attachment-0001.htm>