> On Jan. 29, 2015, 8:22 p.m., George Joseph wrote:
> > If you run the Testsuite, you'll get a good indication of whether this is 
> > truly backwards compatible.
> 
> gareth wrote:
>     I ran the test apps/queue/set_penalty which makes use of ami.command and 
> it failed.
>     
>     However this appears to be a bug with starpy, it is treading the "\r\n" 
> as the end of message. Changing it to output just "\n" results in a 
> successful test.
>     
>     Breaking an existing client library isn't ideal, but the correct 
> delimiter for command output is "--END COMMAND--\r\n\r\n", not "\r\n\r\n".

If we do consider modifying the AMI protocol to support text blobs like this, 
I'd prefer we think about how that should be implemented generically, rather 
than looking at this one action.  So maybe adding 'Text-Terminator: --END 
COMMAND--' or 'Content-Length: XX' to the headers would allow the next packet 
to be read as a text block.  This way the AMI client libraries could have 
generic support for this type of response.  On the other hand AMI is not HTTP.  
This is a perfect example of something ARI is better suited for.  For that 
reason I'm not actually in favor of having AMI support text blobs.

I'd rather see the command output converted to headers:
Response: Success
ActionID: <actionid>
Output: line 1 of cli output
Output: more Output headers per line of cli output

This would of course break existing clients, but would get rid of the exception 
to the AMI protocol.


- Corey


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


On Jan. 29, 2015, 7:25 p.m., gareth wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4391/
> -----------------------------------------------------------
> 
> (Updated Jan. 29, 2015, 7:25 p.m.)
> 
> 
> 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 431113 
>   /trunk/include/asterisk/manager.h 431113 
>   /trunk/CHANGES 431113 
> 
> 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
> 
>

-- 
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Reply via email to