> On March 25, 2015, 9:24 a.m., Corey Farrell wrote: > > /trunk/main/manager.c, lines 4908-4911 > > <https://reviewboard.asterisk.org/r/4391/diff/2/?file=72690#file72690line4908> > > > > astman_start_ack(s, m) seems to be the standard way to begin success > > responses. We would send Response: Success instead of Follows, but this is > > more accurate now that this is a normal response packet anyways. > > astman_start_ack also doesn't send a Privilege: Command header, we could > > add that after (or not?). The Privilege header is mostly just used for > > event packets, not action responses.
Agreed. > On March 25, 2015, 9:24 a.m., Corey Farrell wrote: > > /trunk/main/manager.c, lines 4914-4916 > > <https://reviewboard.asterisk.org/r/4391/diff/2/?file=72690#file72690line4914> > > > > Is it safe to assume that CLI commands will never output "\r"? If not > > we need to improve new-line detection to find and strip "\r". I believe > > the regex we need is: > > (\n|\r\n|\r) > > > > This would ensure we don't get an any extra "\r" in output, and all > > text is put into headers. > > > > This idea is based on the belief that it's not valid for a header to > > have "\r" within its value, though I'm not positive on this. If the \r character occurs in the middle of the line, eg: "Hello\rWorld\n", splitting that into two output headers does not seem like the expected behaviour. And by splitting on more than one character you could end up with a difference when reassembling the output: join('\n', output1, output2, ...) != output; - gareth ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4391/#review14781 ----------------------------------------------------------- On March 20, 2015, 3:34 p.m., gareth wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/4391/ > ----------------------------------------------------------- > > (Updated March 20, 2015, 3:34 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 433198 > /trunk/include/asterisk/manager.h 433198 > /trunk/UPGRADE.txt 433198 > > 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
