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
405 stars 281 forks source link

Incorrect error handling on malformed Command input #154

Open Niflthaena opened 6 years ago

Niflthaena commented 6 years ago

When submitting a Command action where the Command key is unrecognized, PAMI fails to recognize the response and eventually times out, even though an error response is immediately received.

Debugging indicates this is because the error response is formatted in such a way that the ResponseMessage class thinks it's a list. Specifically, ResponseMessage::isList() checks for a Message key containing "follow". While most action errors are formatted with the error data in the Message key, the Command action error is formatted with "Message: Result will follow" and "Output: No such command [etc]". This is not intended as a list, but because the "Message" key looks like it, PAMI times out while waiting for a list that will never come.

This has been tested on AMI 15.1.3.

Demonstration AMI actions follow. First, a malformed Action (demonstrating that the error body is contained in the Message key), and then a malformed Command (demonstrating that the error body is contained in the Output key).

Action: Asdf
ActionID: 1234

Response: Error
ActionID: 1234
Message: Invalid/unknown command: Asdf. Use Action: ListCommands to show available commands.

Action: Command
Command: Asdf
ActionID: 1234

Response: Error
ActionID: 1234
Message: Command output follows
Output: No such command 'Asdf' (type 'core show help Asdf' for other possible commands)
MikeLees commented 5 years ago

I think it's worse than that - it fails for every Command action! It worked against Asterisk version 13.7.2, but I have just upgraded to 16.0.0 and it no longer works.

I have made it work by changing the ResponseMessage::isList() as follows. It works but I don't consider it a proper fix.

 public function isList()
     {
         return
             stristr($this->getKey('EventList'), 'start') !== false
             || (stristr($this->getMessage(), 'follow') !== false 
               && stristr($this->getMessage(), 'Command') === false)
         ;
     }
ldo commented 1 year ago

Checking that the EventList header value is “start” seems to be sufficient for current Asterisk. Then keep including responses until you see one with EventList = “Complete”.

The Command action is a special case: it’s technically a single response, but it can have any number of Output lines.