Summary:	ASTERISK-29663: messaging: AMI MessageSend does not support same parameters as dialplan application
Reporter:	Brian J. Murrell (brian_j_murrell)	Labels:	patch
Date Opened:	2021-09-20 13:25:56	Date Closed:	2021-10-12 15:25:06
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Resources/res_pjsip_messaging
Versions:	18.6.0	Frequency of Occurrence	Constant
Related Issues:
Environment:		Attachments:	( 0) 0001-message.c-Support-To-header-override-with-AMI-s-Mess.patch
Description:	The patch in https://gerrit.asterisk.org/c/asterisk/+/15828 seems to achieve the desired behaviour of allowing the {{To:}} header to be specified for the recipient.  I have successfully used the new third field of {{MessageSend()}} to override the address in the {{To:}} header. When I use AMI {{Action: MessageSend}}, setting the {{To:}} field of the action to the same value as I use for the third field of {{MessageSend()}}, the {{To:}} header is not overwritten with the value of the {{To:}} field from the action fields. For example, via AMI I send: {noformat} Action: MessageSend ActionID: 22525 To: pjsip:my_sip_account@example.com From: <sip:asterisk@example.com> Base64Body: [redacted] {noformat} But what gets sent in the SIP {{MESSAGE}} is: {noformat} MESSAGE sip:my_sip_account@[2001:123:aa:123:0:21db:34e6:9b08:176b]:45016;transport=tcp;pn-tok=[redacted];pn-type=firebase;app-id=[redacted];pn-silent=1;pn-timeout=0 SIP/2.0 Via: SIP/2.0/TCP [2001:123:aa:123::2]:5060;rport;branch=z9hG4bKPj30ad245f-1b2f-40f6-96b9-58e8d3a182b0;alias From: <sip:asterisk@example.com>;tag=5db4ff03-6d42-4b96-ab67-149859ec1073 To: <sip:my_sip_account@[2001:123:aa:123:0:21db:34e6:9b08:176b];pn-tok=[redacted];pn-type=firebase;app-id=[redacted];pn-silent=1;pn-timeout=0> Contact: <sip:my_sip_account@[2001:123:aa:123::2]:5060;transport=TCP> ... {noformat}
Comments:	By: Asterisk Team (asteriskteam) 2021-09-20 13:26:00.444-0500 Thanks for creating a report! The issue has entered the triage process. That means the issue will wait in this status until a Bug Marshal has an opportunity to review the issue. Once the issue has been reviewed you will receive comments regarding the next steps towards resolution. Please note that log messages and other files should not be sent to the Sangoma Asterisk Team unless explicitly asked for. All files should be placed on this issue in a sanitized fashion as needed. A good first step is for you to review the [Asterisk Issue Guidelines|https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines] if you haven't already. The guidelines detail what is expected from an Asterisk issue report. Then, if you are submitting a patch, please review the [Patch Contribution Process|https://wiki.asterisk.org/wiki/display/AST/Patch+Contribution+Process]. Please note that once your issue enters an open state it has been accepted. As Asterisk is an open source project there is no guarantee or timeframe on when your issue will be looked into. If you need expedient resolution you will need to find and pay a suitable developer. Asking for an update on your issue will not yield any progress on it and will not result in a response. All updates are posted to the issue when they occur. Please note that by submitting data, code, or documentation to Sangoma through JIRA, you accept the Terms of Use present at [https://www.asterisk.org/terms-of-use/|https://www.asterisk.org/terms-of-use/]. By: Joshua C. Colp (jcolp) 2021-09-20 13:34:46.481-0500 The MessageSend AMI action does not have support for specifying such information. It would have to be added. By: Brian J. Murrell (brian_j_murrell) 2021-09-20 14:11:22.843-0500 That's not what the [documentation|https://wiki.asterisk.org/wiki/display/AST/Asterisk+18+ManagerAction_MessageSend] ​says.  Or at least how I interpret it.  Specifically: Technology: PJSIP The to parameter is used to specity the To: header in the outgoing SIP MESSAGE. It will override the value specified in MESSAGE(to) which itself will override any to value from an incoming SIP MESSAGE. Spelling error(s) are on the documentation page itself, FWIW. So I am specifying a {{To:}} value and what I am specifying is NOT going into the outgoing SIP {{To:}} header, as the documentation specifies. In any case, any difference in parameters/behaviour/etc across implementations of the same functionality should be considered a bug itself.  It's rather unfortunate that the implementation abstractions don't results in changes made to the core functionality just automatically bubbling up to every implementation. By: Brian J. Murrell (brian_j_murrell) 2021-09-20 14:26:50.004-0500 The {{To:}} parameter's documentation referenced above [changed|https://wiki.asterisk.org/wiki/pages/diffpagesbyversion.action?pageId=44796450&selectedPageVersions=3&selectedPageVersions=2] to modify the description of {{To:}} from simply _Ignored_ to the above, apparently in Asterisk Version GIT-18 a40e58a on Jun. 09.  The day after the above referenced patch landed to change {{To:}} behaviour. Are you sure it's not \[suppposed to be\] implemented? By: Sean Bright (seanbright) 2021-09-20 14:33:20.412-0500 Josh did not say this was not a bug. By: Kevin Harwell (kharwell) 2021-09-20 15:31:52.874-0500 Is "my_sip_account" defined as an endpoint in your _pjsip.conf_? If so then yes this behavior is expected. Parsing of a {{To:}} value in this scenario with all its possible destination variations is complicated to say the least. The pjsip [message_destination_parsing|https://github.com/asterisk/testsuite/blob/master/tests/channels/pjsip/message/message_destination_parsing/test-config.yaml] test attempts to test many of these scenarios. Checkout around line [122|https://github.com/asterisk/testsuite/blob/master/tests/channels/pjsip/message/message_destination_parsing/test-config.yaml#L122] as it appears to be the use case described here: {code}        # Send to endpoint sipp with domain somedomain.com appended.        # Domain should be discarded and default aor used.        ami-actions: { action: { To: 'pjsip:sipp@somedomain.com',            Body: 'Test Message', Action: 'MessageSend', ActionID: '12343' } }    -   ami-events: { conditions: { match: { Endpoint: 'sipp',            RequestURI: 'sip:127.0.0.2',            MdataDestination: '^sipp@somedomain.com$',            Event: 'TestEvent', State: 'MSG_FROMTO_URI' } }, count: 1 } {code} So, I think in the case where you have an endpoint defined in your configuration that matches the {{To:}} (in this test example "sipp") then the domain is discarded, and an associated AOR contact is used. From what I can tell though, if the endpoint is not specified, and a {{default_outbound_endpoint}} is configured then it'll use the URL in the {{To:}} as is. By: Joshua C. Colp (jcolp) 2021-09-21 04:52:28.800-0500 The To: parameter is equivalent to the destination parameter of the MessageSend dialplan application from what I can tell, it's just named differently in the MessageSend dialplan application because different people did them. The MessageSend AMI action does not have a second field for overriding the To. By: Brian J. Murrell (brian_j_murrell) 2021-09-21 07:44:55.398-0500 @jcolp Did you read the explanation I provided in a [previous comment|https://issues.asterisk.org/jira/browse/ASTERISK-29663?focusedCommentId=256367&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-256367]? The timing of the change to the documentation for {{ManagerAction_MessageSend()}} is the day after the patch for {{Application_MessageSend()}}'s {{to}} argument was landed, and was changed the same day and in the same commit as {{Application_MessageSend()}}. The documentation change for {{ManagerAction_MessageSend()}}'s {{to}} parameter was explicitly changed from _Ignored_ to the *exact* same text, word for word, as the *newly added* {{to}} parameter of {{Application_MessageSend()}}, not the text for the {{destination}} parameter of {{Application_MessageSend()}}. This appears to overwhelmingly indicate that the patch landed was intended to change the behaviour of {{ManagerAction_MessageSend()}}'s {{to}} parameter from doing nothing (_Ignored_ per the documentation before the change) to doing the same thing as {{Application_MessageSend()}}'s new {{to}} parameter.  Wouldn't you agree? By: Brian J. Murrell (brian_j_murrell) 2021-09-21 07:49:48.007-0500 {quote} Is "my_sip_account" defined as an endpoint in your pjsip.conf? If so then yes this behavior is expected. {quote} Yes. {quote} So, I think in the case where you have an endpoint defined in your configuration that matches the To: (in this test example "sipp") then the domain is discarded, and an associated AOR contact is used {quote} That's not how {{Application_MessageSend()}} is treating the {{to}} argument.  It is putting it's value into the {{To:}} header of the outgoing message verbatim, which is what the ticket (and the proposed patches from the community) that the patch is supposed to be addressing was requesting.  The ticket and patches were specifically trying to *undo* the behaviour of using the endpoint's IP address (which can vary, of course) as the domain value of the {{To:}} header, to provide consistency in how the endpoint collates received messages. Maybe it would be useful to get the author of that patch (George Joseph) to chime in here to clarify the questions? By: Joshua C. Colp (jcolp) 2021-09-21 08:01:21.185-0500 My comments were in regard to the actual code as it exists today, not in regards to the documentation. It is certainly possible the intention was otherwise. I don't know. By: Brian J. Murrell (brian_j_murrell) 2021-09-21 08:18:40.398-0500 Indeed!  And this ticket is to report as a *bug* that the implementation as described by the documentation does not actually work as the documentation describes it and that there is a functionality mis-match between the {{Application}} function and the corresponding {{ManagerAction}}. Ultimately the goal is that whatever bug is causing the mismatch and the lack of documented functionality get fixed. By: Joshua C. Colp (jcolp) 2021-09-21 08:26:10.120-0500 Okay? As Sean Bright said, I never stated that it wasn't a bug or closed out the issue. I was merely commenting on what the code currently actually does. When this is through triage, there is still no guarantee on time frame for any resolution. By: Kevin Harwell (kharwell) 2021-09-21 09:50:32.440-0500 I think there is some debate on whether the misalignment between the AMI action and dialplan app is a bug, but it seems that at the very least the documentation is a bit ambiguous. So opening the issue. By: Brian J. Murrell (brian_j_murrell) 2021-09-21 09:54:49.671-0500 Indeed.  The questions of misalignment, patch intention, ambiguous documentation, etc. could probably all be answered authoritatively by the patch author, George Joseph.  Is it really not possible to get his input here? By: Sean Bright (seanbright) 2021-09-21 09:59:05.405-0500 [Initial patch attached|^0001-message.c-Support-To-header-override-with-AMI-s-Mess.patch] If a new header, {{Destination}}, is supplied in the {{MessageSend}} action it will be used as the destination address of the {{MESSAGE}}. If the {{To}} action header is also included, that will override the {{To:}} address of the generated message in the same way that the {{MessageSend}} dialplan application does. If only the {{To}} manager header is supplied, it will behave as it does today for backwards compatibility. The documentation will need to be updated to reflect all of this. By: Joshua C. Colp (jcolp) 2021-09-21 10:01:05.695-0500 If George wishes to chime in here, he will. The issue as-is has been accepted, and Sean already attached a patch. By: Brian J. Murrell (brian_j_murrell) 2021-09-21 10:58:14.054-0500 Initial patch appears to work!  Awesome turn-around! By: Friendly Automation (friendly-automation) 2021-09-22 10:16:11.692-0500 Change 16496 merged by Friendly Automation: message.c: Support 'To' header override with AMI's MessageSend. [https://gerrit.asterisk.org/c/asterisk/+/16496|https://gerrit.asterisk.org/c/asterisk/+/16496] By: Friendly Automation (friendly-automation) 2021-09-22 10:16:20.090-0500 Change 16498 merged by Friendly Automation: message.c: Support 'To' header override with AMI's MessageSend. [https://gerrit.asterisk.org/c/asterisk/+/16498|https://gerrit.asterisk.org/c/asterisk/+/16498] By: Brian J. Murrell (brian_j_murrell) 2021-09-22 10:22:28.953-0500 Woot!  Thanks much for the quick turn-around on this.  Appreciate it! By: Asterisk Team (asteriskteam) 2021-09-22 10:22:29.245-0500 This issue has been reopened as a result of your commenting on it as the reporter. It will be triaged once again as applicable. By: Friendly Automation (friendly-automation) 2021-09-22 10:44:12.765-0500 Change 16532 merged by George Joseph: message.c: Support 'To' header override with AMI's MessageSend. [https://gerrit.asterisk.org/c/asterisk/+/16532|https://gerrit.asterisk.org/c/asterisk/+/16532] By: Friendly Automation (friendly-automation) 2021-09-22 10:44:31.097-0500 Change 16531 merged by George Joseph: message.c: Support 'To' header override with AMI's MessageSend. [https://gerrit.asterisk.org/c/asterisk/+/16531|https://gerrit.asterisk.org/c/asterisk/+/16531] By: Brian J. Murrell (brian_j_murrell) 2021-10-12 14:47:21.275-0500 Don't know if comments in closed gerrit issues get read or not, so just copying here... Where did this land (for 18) to exactly, given that it doesn't seem to be included in https://github.com/asterisk/asterisk/compare/18.6.0...18.7.1 What has to happen to get it into 18.7.2 or 18.8.0, ideally whichever comes first? By: Asterisk Team (asteriskteam) 2021-10-12 14:47:21.632-0500 This issue has been reopened as a result of your commenting on it as the reporter. It will be triaged once again as applicable. By: Joshua C. Colp (jcolp) 2021-10-12 15:01:30.092-0500 It will not going into an 18.7.2, because this was not a regression. It will automatically go into 18.8.0 when a release candidate is created for that, and this issue will be automatically updated with that as the target fix version. By: Brian J. Murrell (brian_j_murrell) 2021-10-12 15:14:03.024-0500 Wasn't that (is it a bug or not) the debate in the comment history to which both you and Sean responded that nobody was saying it wasn't a bug?  So if nobody wasn't saying it's not a bug, it's a bug right?  Does that not qualify it for a bug-fix release? Also wondering why it didn't make it into 18.7.0 which was tagged on Oct. 7 when the fix landed to the 18 branch on Sep. 22. Just trying to understand release criteria here. By: Asterisk Team (asteriskteam) 2021-10-12 15:14:03.161-0500 This issue has been reopened as a result of your commenting on it as the reporter. It will be triaged once again as applicable. By: Joshua C. Colp (jcolp) 2021-10-12 15:17:28.828-0500 18.8.0 is the next bug fix release and it will automatically be in it. While 18.7.0 was tagged on Oct 7, the first release candidate was done on September 16th - before this was merged in. Once the first release candidate is created then only fixes for regressions found during testing of the release candidate are included. As well, point releases (such as 18.7.2) occur as a result of a critical regression found after release that need to be fixed immediately or as a release of security fixes. By: Brian J. Murrell (brian_j_murrell) 2021-10-12 15:23:12.040-0500 Ahh.  I see.  Yes, I missed the -rc*s for 18.7.0.  It's annoying that github doesn't let you filter the tags/releases on their release page. It's also new to me that your versions are Major.Bugfix.Crtical-bug-fix.  I thought they were the more typical Major-feature.Minor-feature.Bugfix form. It all makes more sense now.