pipobscure / NodeJS-AsteriskManager

NodeJS Asterisk Manager API
Other
248 stars 103 forks source link

Multiple fix #48

Closed garronej closed 7 years ago

garronej commented 7 years ago

There is four commit, the first one sould be merged, the other are important in my opignions but you may want to keep the code as it is.

d1b5630: It is the cause of the issue @alexandremiguelabreu had in the first place.

b26b168: In asterisk 14+ the Command action has changed: result of commands are now sent as an array of outputs headers, removing the space at the begining and at the end break formating.

05dfd05: I think it's a bad idea to leave unecesary timers, it prevent node.js of ending when it should. In this case using once seems a beter option.

b92d5d2: In the current implementation multiple variables are set like this in the Action sent to asterisk: Variable: FOO=BAR,BAR=BAZ\r\n With this fix: Variable: FOO=BAR\r\n Variable: BAR=BAZ\r\n

AMI use a 1024 byte buffer to parse lines, in the current implem we quickly overload this buffer if we want to pass a lot of variable and we got "manager.c: line too long"

Cheers,

pipobscure commented 7 years ago

Resolved conflict & other stuff in #49 could you review that and let me know if that would cover what you meant. I'll then merge and release tonight.

garronej commented 7 years ago

@igorescobar

Regarding asterisk's backward compatibility:

My only concern was if multiple VARIABLE headers had always been supported or if in old version of asterisk we where forced to submit values in a sigle line ( e.g. : Variable: foo=bar,hello=world\r\n )

I did not check it my self but for what I have find in the web it has always been supported. So I am 97% shure it is fine.

Now could this update break user code:

Most certainly not but still I made a change that may theoricaly do. In the previous release spaces where striped at the begining and the end of every header's value. In my opignion it is not the expected behaviour because if asterisk send spaces they mean something. It never randomely send usless spaces. So I think it should be changed but let me know it you disagree.

Regards,

igorescobar commented 7 years ago

@garronej 97% it's all I need ;D