marcelog / PAMI

PHP Asterisk Manager Interface ( AMI ) supports synchronous command ( action )/ responses and asynchronous events using the pattern observer-listener. Supports commands with responses with multiple events. Very suitable for development of operator consoles and / or asterisk / channels / peers monitoring through SOA, etc
http://marcelog.github.com/PAMI
Apache License 2.0
401 stars 279 forks source link

Incorrect CommandAction response #88

Closed ekzobrain closed 8 years ago

ekzobrain commented 8 years ago

PAMI incorrectly parses Command core show hints response:

object(PAMI\Message\Response\ResponseMessage)#57 (9) {
  ["_events":"PAMI\Message\Response\ResponseMessage":private]=>
  array(0) {
  }
  ["_completed":"PAMI\Message\Response\ResponseMessage":private]=>
  bool(true)
  ["rawContent":protected]=>
  string(3227) "Response: Follows
Privilege: Command
ActionID: 1442565050.0953

    -= Registered Asterisk Dial Plan Hints =-
                   2601@status              : SIP/2601              State:Idle            Watchers  9
                   2602@status              : SIP/2602              State:Unavailable     Watchers 10
                   2803@status              : SIP/2803              State:Unavailable     Watchers  9
                   2802@status              : SIP/2802              State:Idle            Watchers  9
                  _XXXX@status              : SIP/${EXTEN}          State:Unavailable     Watchers  0
                   2098@status              : SIP/2098              State:Unavailable     Watchers  9
                   9090@status              : SIP/9090              State:Idle            Watchers  8
                   2102@status              : SIP/2102              State:Unavailable     Watchers 10
                   2101@status              : SIP/2101              State:Unavailable     Watchers  9
                   2301@status              : SIP/2301              State:Idle            Watchers 10
                   2302@status              : SIP/2302              State:Unavailable     Watchers  9
                   2005@status              : SIP/2005              State:Unavailable     Watchers  2
                   2004@status              : SIP/2004              State:Unavailable     Watchers 10
                   2003@status              : SIP/2003              State:Idle            Watchers  9
                   2002@status              : SIP/2002              State:Unavailable     Watchers 10
                   2001@status              : SIP/2001              State:Idle            Watchers  9
                   3002@status              : SIP/3002              State:Idle            Watchers  9
                   2502@status              : SIP/2502              State:Unavailable     Watchers 11
                   2503@status              : SIP/2503              State:Unavailable     Watchers 10
                   2501@status              : SIP/2501              State:Unavailable     Watchers 10
                   2201@status              : SIP/2201              State:Unavailable     Watchers  8
                   2203@status              : SIP/2203              State:Unavailable     Watchers  9
                   2202@status              : SIP/2202              State:Idle            Watchers  9
                   2701@status              : SIP/2701              State:Idle            Watchers  8
                   2702@status              : SIP/2702              State:Unavailable     Watchers 10
                   2403@status              : SIP/2403              State:Unavailable     Watchers  9
                   2402@status              : SIP/2402              State:Idle            Watchers  9
                   2401@status              : SIP/2401              State:Unavailable     Watchers 10
                   2902@status              : SIP/2902              State:Unavailable     Watchers  9
                   2901@status              : SIP/2901              State:Idle            Watchers  9
----------------
- 30 hints registered
--END COMMAND--"
  ["channelVariables":protected]=>
  array(1) {
    ["default"]=>
    array(0) {
    }
  }
  ["lines":protected]=>
  array(0) {
  }
  ["variables":protected]=>
  array(0) {
  }
  ["keys":protected]=>
  array(4) {
    ["response"]=>
    string(7) "Follows"
    ["privilege"]=>
    string(7) "Command"
    ["actionid"]=>
    string(15) "1442565050.0953"
    ["-= registered asterisk dial plan hints =-
                   2601@status"]=>
    string(3068) "SIP/2601              State:Idle            Watchers  9
                   2602@status              : SIP/2602              State:Unavailable     Watchers 10
                   2803@status              : SIP/2803              State:Unavailable     Watchers  9
                   2802@status              : SIP/2802              State:Idle            Watchers  9
                  _XXXX@status              : SIP/${EXTEN}          State:Unavailable     Watchers  0
                   2098@status              : SIP/2098              State:Unavailable     Watchers  9
                   9090@status              : SIP/9090              State:Idle            Watchers  8
                   2102@status              : SIP/2102              State:Unavailable     Watchers 10
                   2101@status              : SIP/2101              State:Unavailable     Watchers  9
                   2301@status              : SIP/2301              State:Idle            Watchers 10
                   2302@status              : SIP/2302              State:Unavailable     Watchers  9
                   2005@status              : SIP/2005              State:Unavailable     Watchers  2
                   2004@status              : SIP/2004              State:Unavailable     Watchers 10
                   2003@status              : SIP/2003              State:Idle            Watchers  9
                   2002@status              : SIP/2002              State:Unavailable     Watchers 10
                   2001@status              : SIP/2001              State:Idle            Watchers  9
                   3002@status              : SIP/3002              State:Idle            Watchers  9
                   2502@status              : SIP/2502              State:Unavailable     Watchers 11
                   2503@status              : SIP/2503              State:Unavailable     Watchers 10
                   2501@status              : SIP/2501              State:Unavailable     Watchers 10
                   2201@status              : SIP/2201              State:Unavailable     Watchers  8
                   2203@status              : SIP/2203              State:Unavailable     Watchers  9
                   2202@status              : SIP/2202              State:Idle            Watchers  9
                   2701@status              : SIP/2701              State:Idle            Watchers  8
                   2702@status              : SIP/2702              State:Unavailable     Watchers 10
                   2403@status              : SIP/2403              State:Unavailable     Watchers  9
                   2402@status              : SIP/2402              State:Idle            Watchers  9
                   2401@status              : SIP/2401              State:Unavailable     Watchers 10
                   2902@status              : SIP/2902              State:Unavailable     Watchers  9
                   2901@status              : SIP/2901              State:Idle            Watchers  9
----------------
- 30 hints registered
--END COMMAND--"
  }
  ["createdDate":protected]=>
  int(1442565050)
  ["_eventsCount"]=>
  int(0)
}

As you can see - some part of the response body wa put into key name... Using latest PAMI release

ekzobrain commented 8 years ago

I think, that CommandAction response should have a special parser, which would return just plain text with "--END COMMAND--" stripped off from the end. Applying standard parsers may lead to unexpected results till response format may vary significantly.

marcelog commented 8 years ago

Hello, thanks for the report!

Mmm it seems that respones actually violates the ami protocol (there's an empty new line after the ActionId key and that marks the end of the message, although that wouldn't be the first inconsistency found so far :P).. What asterisk version is that?

@dkgroot How would your new patch handle this case? Looks like a good test case! :)

ekzobrain commented 8 years ago

Asterisk 11

marcelog commented 8 years ago

Referencing @dkgroot patch in #73 for ResponseFactory, as it is related

dkgroot commented 8 years ago

dkgroot: How would your new patch handle this case? Looks like a good test case! :)

@marcelog: The/your core message parser is still the same (Did not change anything there, as far as i can remember).

"--END COMMAND--" does not follow the normal asterisk API for returning multiple values. The problem is that CLI and AMI commands don't use one and the same source/api. That's what we tried to fix in chan-sccp-b (using a whole slew of macro's ;-(). It would have made sense if they would have created a dictionary to assemble the output and then have a generic generator which is able to convert that dictionary to and arbitrairy number of outputs like AMI/CLI (or JSON/XML/Whatever). Alas that's not the case. The have started doing it more like that in asterisk-13. But it would have been better if there was a core design change in both modules.

Maybe step one should be to send a patch to make the 'Command' AMI message behave a little better.

I think, that CommandAction response should have a special parser, which would return just plain text with "--END COMMAND--" stripped off from the end. Applying standard parsers may lead to unexpected results till response format may vary significantly.

I would be able to do just that, by providing a CommandResponse.php parse you would be able to catch this response and parse it seperately. The problem with the commandresponse is that the content is completely unstructured (namely cli output which can come in any number of formats), so i am not sure how much use it will be.

In my opinion the Command AMI Message should never have been included in AMI, which would have prevented developers from depending on it, causing them to create specific AMI Commands to get the Data out in a structured way.

BTW: For this 'command' you could/should have used ExtensionStateList and DeviceStateList, instead.

ekzobrain commented 8 years ago

@dkgroot

BTW: For this 'command' you could/should have used ExtensionStateList and DeviceStateList, instead.

These commands are not available in Asterisk 11 - https://wiki.asterisk.org/wiki/display/AST/Asterisk+11+AMI+Actions. As far as I know (i am new to Asterisk) - I could only use SIPpeers action, but it depends on "sip reload", which makes it inaccurate in some cases.

In my opinion the Command AMI Message should never have been included in AMI, which would have prevented developers from depending on it, causing them to create specific AMI Commands to get the Data out in a structured way.

May be in latest Asterisk it os not so use usefull and really makes so mess, but in older versions it is not, as there are much less actions available. I currently call getRawContent() on response for this action and parse it with regular expressions. It is absolutely OK, except that I have to get rid of headers (Response: Follows Privilege: Command ActionID: 1442565050.0953) and "--END COMMAND--". It would make sense to add something like getContent() methot with this data already stripped off.

dkgroot commented 8 years ago

Hi Dmitry,

On 19-09-15 04:19, Dmitry wrote:

@dkgroot https://github.com/dkgroot

BTW: For this 'command' you could/should have used ExtensionStateList and DeviceStateList, instead.

These commands are not available in Asterisk 11 - https://wiki.asterisk.org/wiki/display/AST/Asterisk+11+AMI+Actions. As far as I know (i am new to Asterisk) - I could only use SIPpeers action, but it depends on "sip reload", which makes it inaccurate in some cases.

In my opinion the Command AMI Message should never have been included in AMI, which would have prevented developers from depending on it, causing them to create specific AMI Commands to get the Data out in a structured way.

I did not mean it should not have been include in PAMI, but it would have been wise not to add it to asterisk, just to prevent this sort of thing. And that would have also meant that ExtensionStateList and DeviceStateList would have been added back in asterisk-1.6 (or even earlier maybe).

May be in latest Asterisk it os not so use usefull and really makes so mess, but in older versions it is not, as there are much less actions available. I currently call getRawContent() on response for this action and parse it with regular expressions. It is absolutely OK, except that I have to get rid of headers (Response: Follows Privilege: Command ActionID: 1442565050.0953) and "--END COMMAND--". It would make sense to add something like getContent() methot with this data already stripped off.

You could clone my branch and add a new CommandReponseHandler to it, which could use your regular expression to remove the header and footer. That would also show it how usefull this extension could be, in situations like this. (Not trying to advertise a split of any kind)

BTW: You do have ExtensionState and PresenceState in Asterisk-11, Does that not contain the information you need, or is it only the list you are after ?

— Reply to this email directly or view it on GitHub https://github.com/marcelog/PAMI/issues/88#issuecomment-141609737.

marcelog commented 8 years ago

Thanks for your feedback guys. Unfortunately, I don't see a nice way to implement this in the short term, sorry. Closing this one for now.