Summary:	ASTERISK-15687: [patch] From-header parsed twice for each invite or subscription request
Reporter:	nick_lewis (nick_lewis)	Labels:
Date Opened:	2010-02-24 10:32:13.000-0600	Date Closed:	2015-02-25 20:02:52.000-0600
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Channels/chan_sip/General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) chan_sip.c-parseonce.patch ( 1) chan_sip.c-parseonce2.patch
Description:	The handlers in chan_sip.c for invite and subscription requests parse the from-header twice. The first time is in check_user_full and the second time is in get_destination.   ****** ADDITIONAL INFORMATION ****** I suggest that there be added a function (named something like get_source) that is called before check_user(_full) and get_destination. It would parse the from-header and populate pvt->cid_num and pvt->fromdomain as required by the other two functions. (In the case of the handler for options requests it would be called before get_destination because check_user is not yet supported for that method.)
Comments:	By: Russell Bryant (russell) 2010-02-24 23:38:11.000-0600 Is this causing a problem, or is this just a suggestion for some code cleanup? By: nick_lewis (nick_lewis) 2010-02-26 03:52:18.000-0600 Just a performance suggestion By: David Vossel (dvossel) 2010-02-26 11:04:49.000-0600 Shouldn't this be categorized as a tweak then? I could be wrong, but I think code tweaks, like features, require a patch. By: nick_lewis (nick_lewis) 2010-03-01 08:55:24.000-0600 check_peer_ok() seems to want calleridname so I have amended the patch to provide it. I am yet to understand where check_peer_ok actually makes use of calleridname though By: Paul Belanger (pabelanger) 2010-06-01 13:18:30 I would recommend posting this on reviewboard, to help move the issue along. By: nick_lewis (nick_lewis) 2010-06-07 09:06:08 Having now introduced the parse_name_andor_addr function to reqresp_parser.c I probably need to revisit this patch to make use of it By: Matt Jordan (mjordan) 2015-02-25 20:02:34.223-0600 Given that things changed substantially from when this patch was first proposed, I'm going to go ahead and Suspend this issue. If you think there's a benefit in porting the patch to a supported version of Asterisk, please comment here and I'll be happy to re-open the issue. That being said, posting the patch to review board would help a lot in having the patch get merged. I'll comment here with instructions on doing that. By: Matt Jordan (mjordan) 2015-02-25 20:02:45.405-0600 Thanks for the contribution! If you'd like your contribution to be included faster, you should submit your patch for code review by the Asterisk Developer Community. To do so, please follow the Code Review [1] instructions on the wiki. Be sure to: * Verify that your patch conforms to the Coding Guidelines [2] * Review the Code Review Checklist [3] for common items reviewers will look for * If necessary, provide tests for the Asterisk Test Suite that verify the correctness of your patch [4] When ready, submit your patch and any tests to Review Board [5] for code review. Thanks! [1] https://wiki.asterisk.org/wiki/display/AST/Code+Review [2] https://wiki.asterisk.org/wiki/display/AST/Coding+Guidelines [3] https://wiki.asterisk.org/wiki/display/AST/Code+Review+Checklist [4] https://wiki.asterisk.org/wiki/display/AST/Asterisk+Test+Suite+Documentation [5] https://wiki.asterisk.org/wiki/display/AST/Review+Board+Usage