Summary:	ASTERISK-12965: [patch] Cannot register with sip providers that require '@' in the username
Reporter:	Navin Kumar (navkumar)	Labels:
Date Opened:	2008-10-25 12:44:22	Date Closed:	2008-12-10 10:44:33.000-0600
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	Channels/chan_sip/Registration
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) stringfields.diff
Description:	With 1.4.22, you cannot have a registration line like the following: register=>7123231@sip.host.com:1234223:7123231@sip.proxy.net/7123231 I believe there was an issue (ASTERISK-12786) where a patch was committed to not allowing 'reserved' characters; however, looking at the patch and the svn diff, I saw that it must have guessed the first '@' is reserved; however, this registration is perfectly valid for many sip providers.  It is how you register with particular sip hosts using a full URI instead of using a simple host name, as well as how you register through outbound proxies.  This kind of register line is absolutely necessary though several sip providers. ****** ADDITIONAL INFORMATION ****** Link to svn diff in /branches/1.4 http://svn.digium.com/view/asterisk/branches/1.4/channels/chan_sip.c?r1=146799&r2=149130&pathrev=149130 Link to original patch in bug report ASTERISK-12786 http://bugs.digium.com/view.php?id=13570 One should create a better way of handling malformed registration strings.  This kind of preliminary error handling which is not tested properly can lead to these scenarios.
Comments:	By: Olle Johansson (oej) 2008-10-25 14:00:54 If you set the fromdomain in a peer and use that for registration, does it work? By: Navin Kumar (navkumar) 2008-10-25 22:50:17 It doesn't seem to work for me. I tried with both SIP realtime and in the config file.  It doesn't seem to be using the fromdomain when constructing the registration. By: James McCoy (jamessan) 2008-12-02 16:00:17.000-0600 This is actually a problem introduced by mis-use of the ast_string_field_set macro.  In chan_sip.c:transmit_register there is the following line of code:  ast_string_field_set(p, fromdomain, ++fromdomain); The body of the macro uses the 3rd argument (in this case "++fromdomain") in multiple places, thus causing the pointer to be incremented multiple times. The attached stringfields.diff changes both ast_string_field_index_set and ast_string_field_index_logset to assign the data argument of the macro to a new variable (__data__) in the macro body and then use that variable instead. By: Mark Michelson (mmichelson) 2008-12-04 16:43:04.000-0600 It appears that in my fix for ASTERISK-12786, I did not take into account that the "username" portion of the register line may contain the @ symbol. Before any work is committed, are there any other of the "reserved" characters which may appear legally in the user portion of the register line? jamessan's change for the string field macros seems like it should be committed too, to prevent such side effects from occurring. By: Leif Madsen (lmadsen) 2008-12-09 07:52:44.000-0600 putnopvut: added a relation to an issue that mentions about other reserved characters in the register line. I've also added a 'has duplicate' for another issue that complains about this same issue, but I'm going to close the duplicate. By: Mark Michelson (mmichelson) 2008-12-10 10:23:09.000-0600 This was introduced because of the fix for issue 13570. That issue probably affected maybe about 3 people total, and since the fix for it is now breaking SIP registrations for lots of people, I am going to revert that change and re-open issue 13570 instead. By: Mark Michelson (mmichelson) 2008-12-10 10:24:14.000-0600 Oh, and I will also be committing jamessan's patch too. Don't think I've forgotten about that one :) By: Digium Subversion (svnbot) 2008-12-10 10:24:51.000-0600 Repository: asterisk Revision: 162663 U   branches/1.4/channels/chan_sip.c ------------------------------------------------------------------------ r162663 | mmichelson | 2008-12-10 10:24:51 -0600 (Wed, 10 Dec 2008) | 11 lines Revert fix for issue 13570. It has caused more problems than it helped to fix. (closes issue ASTERISK-12965) Reported by: navkumar (closes issue ASTERISK-13170) Reported by: ffs ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=162663 By: Digium Subversion (svnbot) 2008-12-10 10:44:33.000-0600 Repository: asterisk Revision: 162670 U   branches/1.4/include/asterisk/stringfields.h ------------------------------------------------------------------------ r162670 | mmichelson | 2008-12-10 10:44:33 -0600 (Wed, 10 Dec 2008) | 14 lines Update to stringfield handling so that side-effects on parameters are not evaluated multiple times. An example where this caused a problem was in chan_sip.c, with the line  ast_string_field_set(p, fromdomain, ++fromdomain); This patch was originally uploaded to issue ASTERISK-12965 by jamessan. While the issue was closed for other reasons, this patch is valid and fixes a separate problem, and is thus being committed. ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=162670