[asterisk-dev] [Code Review]: Just remove the silly SIP registertrying option

Terry Wilson reviewboard at asterisk.org
Thu Nov 3 10:14:34 CDT 2011



> On Nov. 2, 2011, 5:44 p.m., wdoekes wrote:
> > /branches/1.8/channels/sip/include/sip.h, lines 339-342
> > <https://reviewboard.asterisk.org/r/1562/diff/1/?file=21579#file21579line339>
> >
> >     http://lists.digium.com/pipermail/asterisk-dev/2011-October/051596.html
> >     
> >     Removing 1<<24 would've been sufficient/better.
> 
> wdoekes wrote:
>     http://lists.digium.com/pipermail/asterisk-commits/2011-November/052320.html
>     +#define SIP_PAGE2_UDPTL_DESTINATION		(1 << 24)  /*!< DP: Use source IP of RTP as destination if NAT is enabled */
>     
>     Perhaps I shouldn't've checked the ship-it ;)
>

Oh, will you look at that. I missed a chance to fix two bugs with one commit. Oh well. I'll re-number stuff to account for the extremely long-standing bug that was registertrying.


- Terry


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1562/#review4644
-----------------------------------------------------------


On Nov. 2, 2011, 5:27 p.m., Terry Wilson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1562/
> -----------------------------------------------------------
> 
> (Updated Nov. 2, 2011, 5:27 p.m.)
> 
> 
> Review request for Asterisk Developers and wdoekes.
> 
> 
> Summary
> -------
> 
> Way back in the day, Asterisk sent a 100 Trying when receiving a register request, despite RFC 3261 stating:
> 8.2.6.1 Sending a Provisional Response
> 
> One largely non-method-specific guideline for the generation of
> responses is that UASs SHOULD NOT issue a provisional response for a
> non-INVITE request. Rather, UASs SHOULD generate a final response to
> a non-INVITE request as soon as possible.
> 
> Someone initially tried to remove the 100 Trying in https://issues.asterisk.org/jira/browse/ASTERISK-9157 , but cries of "there might be phones that require this behavior!" were made and YAUO (yet another useless option) was added to chan_sip. There is no conceivable way that someone would write code that requires a provisional 100 response to a REGISTER. It would be an absurd amount of going out of one's way to write broken code. Add to that the fact (as wdoekes points out on the mailing list) that the flag is set on the peer, not copied to the pvt, but checked on the pvt anyway--it hasn't even worked all these years.
> 
> Even better, fixing the code would lead to the same kind of username disclosure as the nat= debacle we are trying to clean up. This leads me to the happy conclusion that we should finally just rip this code out. This patch does that.
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/channels/chan_sip.c 343161 
>   /branches/1.8/channels/sip/include/sip.h 343161 
> 
> Diff: https://reviewboard.asterisk.org/r/1562/diff
> 
> 
> Testing
> -------
> 
> It compiles.
> 
> 
> Thanks,
> 
> Terry
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20111103/00b3132e/attachment-0001.htm>


More information about the asterisk-dev mailing list