|Summary:||ASTERISK-02895: [patch] Run script upon registration chan_sip.c|
|Date Opened:||2004-11-26 15:34:41.000-0600||Date Closed:||2011-06-07 14:05:07|
|Environment:||Attachments:||( 0) chan_sip_new.c|
( 1) chan_sippatch.diff.txt
( 2) patch.diff.txt
|Description:||I desired a way to run a script upon registration. This can provide an additional method of authentication by passing the authuser and userid. Also, it can be used to do more complex logging into a database. The logging facilities of Asterisk were not enough for me so I decided to add the ability to run a script to perform external functions.|
****** ADDITIONAL INFORMATION ******
The new functionality runs by declaring regscript=/scriptpath/scriptfile in /etc/asterisk/sip.conf
|Comments:||By: brian22942 (brian22942) 2004-11-26 15:36:10.000-0600|
I have not signed a disclaimer as of 11/26/2004.
By: Brian West (bkw918) 2004-11-26 16:10:33.000-0600
This isn't needed as It does fire manager events on register.
By: Brian West (bkw918) 2004-11-26 16:11:31.000-0600
Also diff -u and whats up with the massive format changes.
By: khb (khb) 2004-11-26 20:51:02.000-0600
If I read this correctly (the diff is almost incomprehensible) you are adding a popen to the script in sip_register?
sip_register does not perform a registration, it only inserts a registration structure into the registry (later on these get queued via the scheduler.)
It does this only when the config file is parsed and never anytime after.
So there is no way be sure that the registration will succeed at this point or when re-registration takes place.
What happens if the script should block? Seems it would block the monitor thread, not? There are no timeout functions in case of failure.
As bkw already suggested, the manager events handle this much better already.
If additional triggers are needed that same facility can be expanded. It could be interesting to have configurable triggers for certain applications.
By: brian22942 (brian22942) 2004-11-27 08:23:11.000-0600
Sorry for the incomprehensible diff file. I don't know what happened. Either way, this could be a useful patch. While the manager API can monitor events, it cannot reject registrations. Also, opening an additional socket connection for a manager causes security concerns if the Asterisk box is sitting on the open internet. I am attempting to allow an interface between Asterisk and external programs because I do not think the socket connection to be a good idea. I have also read additional notes on crashing due because of too many socket connections open on the manager interface. If I were to actually add this in the correct section, where would I add it? I think it would be useful as an additional application.
By: Olle Johansson (oej) 2004-11-27 22:38:46.000-0600
Do you mean that you are going to disclaim or that you are not going to disclaim this code?
Also please provide a "cvs diff -u" patch file so we can understand what is happening here. The diff is changing too much of the source, so it is too hard to figure out what you are doing...
By: brian22942 (brian22942) 2004-11-28 07:09:35.000-0600
I've uploaded another diff -u of the patch. Hopefully you guys can read it this time. oej: I have not signed a disclaimer, no. But I do disclaim it.
By: khb (khb) 2004-11-28 11:33:10.000-0600
Again, rarely anyone will be able to discern the crux of your patch if you make formatting changes or move hundreds of lines of code that don't relate to your intended improvements. Your patch is 163kBytes for, hmm, maybe 20 lines of code?
Are the comments I made earlier correct? Can you answer the objections I raised?
If there isn't more to the patch than I described, it lacks technical merits to be included, IMHO.
By: brian22942 (brian22942) 2004-11-28 18:06:46.000-0600
You're correct, khb. It could be useful, but not how I implemented it. I will make the necessary improvements and resubmit at a later date.
Please close this patch.
By: Brian West (bkw918) 2004-11-28 18:07:22.000-0600
Close per request... making it better for later submit.