[asterisk-dev] [Code Review] 4391: Add blank line between headers and output for Command action response

[Matt Jordan]

> On April 13, 2015, 2:41 a.m., Corey Farrell wrote:
> > So I think this looks pretty good.  Next steps:
> > * We've migrated to git.  Take a look at [1] for information on how to use gerrit to post a git review.  Don't worry you won't be facing the full review again, we've already dealt with the code issues.
> > * We need to prepare a patch for starpy [2].  I don't have time to work on this right now.  Do you know python or know someone who does and has time/willingness to help?
> > * Verify the full testsuite passes against the latest Asterisk with this change.
> > * Once starpy supports the change we will coordinate with Digium to ensure starpy is updated before we commit this to trunk.
> > 
> > I know this seems like a lot, but we are changing the protocol, so we need to make sure we do not break the testsuite.
> > 
> > [1] https://wiki.asterisk.org/wiki/display/AST/Gerrit+Usage
> > [2] https://github.com/asterisk/starpy/

I'd be willing to take on the starpy issues if/when this gets pulled over.


- Matt


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


On April 13, 2015, midnight, gareth wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4391/
> -----------------------------------------------------------
> 
> (Updated April 13, 2015, midnight)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24730
>     https://issues.asterisk.org/jira/browse/ASTERISK-24730
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This patch adds a blank line between the headers and the output in the Command action response. The reason for this change is to make it easier to determine where the headers end and the output from the command starts.
> 
> Currently, to parse a response to a Command action, one has to:
> 
> 1. Read one line, if line is "Response: Error", parse the remaining as a standard AMI response and stop.
> 2. Read one more line - or two if you used an ActionID - those lines are the headers.
> 3. Then read everything up to "--END COMMAND--\r\n\r\n".
> 
> That could be changed to:
> 
> 1. Read standard AMI response.
> 2. If "Response: Follows", then read everything up to "--END COMMAND--\r\n\r\n".
> 
> The AMI version has been increased to 2.8.0 as this is a protocol change and so that clients detect the new behavior.
> 
> Adding a blank line should not cause older parsers to fail as they have to read everything up to "--END COMMAND--\r\n\r\n" anyway.
> 
> Adding a blank line will also not cause the AMI to HTML/XML encoder to fail to separate the headers from the output as it checks for the presence of a ":" character, which a blank line does not contain.
> 
> 
> Diffs
> -----
> 
>   /trunk/main/manager.c 434448 
>   /trunk/main/cli.c 434448 
>   /trunk/include/asterisk/manager.h 434448 
>   /trunk/UPGRADE.txt 434448 
> 
> Diff: https://reviewboard.asterisk.org/r/4391/diff/
> 
> 
> Testing
> -------
> 
> Connected to manager, issued 'core show uptime' command and verified that there was a blank line between the headers and output.
> 
> 
> Thanks,
> 
> gareth
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20150414/ab37b591/attachment.html>