| Summary: | ASTERISK-24934: [patch]Asterisk manager output does not escape control characters | ||||
| Reporter: | warren smith (somedood) | Labels: | |||
| Date Opened: | 2015-04-02 16:38:16 | Date Closed: | 2015-06-08 13:18:01 | ||
| Priority: | Major | Regression? | No | ||
| Status: | Closed/Complete | Components: | Core/ManagerInterface | ||
| Versions: | 11.17.0 12.8.1 13.3.0 | Frequency of Occurrence | Frequent | ||
| Related Issues: |
| ||||
| Environment: | Tested on most recent asterisk 11 branch | Attachments: | ( 0) asterisk_manager_escaped_output.patch | ||
| Description: | Asterisk manager output is created using printf formatting, like:
manager_event(SOME_EVENT_FLAG, "EventName", "KeyOne: %s\r\nKeyTwo: %s\r\n", val1, val2); This causes problems when the values themselves contain control characters like carriage return and newline, so that applications parsing the output will interpret this as a new key, or the end of an event. An example of this is having a callerid contain "\r\n\r\n". This ends the event, and the keys for the same event are interpreted as a new message, and any keys below are missed for the real event. I've included a patch that provides a ast_escape_c() function which takes a string, then returns a pointer to a new string that has the c characters escaped (i.e., newline into \n). I've modified the calls to the manager_event functions (manager_event, ast_manager_event, ast_manager_event_multichan) so that values that could be set by a user are escaped. The string values that as far as I know aren't user-created were left as-is, like channel names and uniqueid. There are quite a few calls to the manager event functions and I've double checked to make sure all memory allocations are freed after creating the escaped string. I also had added an ast_replace_string function which i didn't end up using, and added an ast_escape_output function which just calls ast_escape_c. An alternative would be to replace the sequence "\r\n" with the escaped version, rather than the individual characters. I'm testing on our asterisk 11 install and this fixes the parsing bugs we run into from messed up callerids and things like agent names containing return + newline. | ||||
| Comments: | By: Rusty Newton (rnewton) 2015-04-03 14:01:53.079-0500 Thanks for the contribution! If you'd like your contribution to be included faster, you should submit your patch for code review by the Asterisk Developer Community. To do so, please follow the Code Review [1] instructions on the wiki. Be sure to: * Verify that your patch conforms to the Coding Guidelines [2] * Review the Code Review Checklist [3] for common items reviewers will look for * If necessary, provide tests for the Asterisk Test Suite that verify the correctness of your patch [4] When ready, submit your patch and any tests to Review Board [5] for code review. Thanks! [1] https://wiki.asterisk.org/wiki/display/AST/Code+Review [2] https://wiki.asterisk.org/wiki/display/AST/Coding+Guidelines [3] https://wiki.asterisk.org/wiki/display/AST/Code+Review+Checklist [4] https://wiki.asterisk.org/wiki/display/AST/Asterisk+Test+Suite+Documentation [5] https://wiki.asterisk.org/wiki/display/AST/Review+Board+Usage By: Rusty Newton (rnewton) 2015-04-03 14:02:52.077-0500 Also, note that the patch you attached is not visible to any others as you have posted it before your submission agreement has been processed. Once it has been processed then please re-attach the patch to the issue and proceed with the review process detailed above. Thanks! By: warren smith (somedood) 2015-04-03 14:04:29.192-0500 Rusty, thanks for the feedback! How can I tell when the submission agreement has been processed? By: warren smith (somedood) 2015-04-07 15:45:10.823-0500 I got the approval email, and this is now up for code review By: Rusty Newton (rnewton) 2015-04-13 19:00:53.592-0500 Forgot to unassign myself! | ||||