[asterisk-dev] [Code Review] chan_sip: Make the session-timers 'require: timer' header an option

[Nick Lewis]

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



/trunk/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/698/#comment4642>

    Does it need the word timer twice. I suppose it is the parameter used in the require-header but global_require_st may be sufficient



/trunk/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/698/#comment4643>

    How about "Session Timers Required:"



/trunk/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/698/#comment4644>

    I think session-timers-required makes more sense



/trunk/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/698/#comment4645>

    Later does there need to be a check a disablement and a warning if this setting is incompatible with st-mode



/trunk/configs/sip.conf.sample
<https://reviewboard.asterisk.org/r/698/#comment4646>

    Perhaps just adding another mode here would be easier. I guess that originate cannot change to drop the require so perhaps "negotiate" could be added as "Run session-timers unless unsupported by other UA"


- Nick


On 2010-06-09 10:36:35, Darren Sessions wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/698/
> -----------------------------------------------------------
> 
> (Updated 2010-06-09 10:36:35)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> I've had several issues with the 'Require: timer' header option with regards to the session timers where vendors (primarily BT) and customers SBCs do not support this header and they reject calls with a '420 Bad Extension'.
> 
> Simply commenting this code out in chan_sip.c in the past has resolved the problem, however, I'm interested in a possibly more permanent solution. From what I can gather, this field is optional anyways (someone correct me if I'm wrong here). I've included an excerpt from RFC4028 (section 8.1) which seems to say that it's even not recommended.
> 
> Either way, this patch defaults the require timers option to true so as to be consistent with the existing default behavior. I've also included changes to the configs/sip.conf.sample.
> 
> 
> From RFC4028:
> 
>    The proxy MUST remember, for the duration of the transaction, whether
>    the request contained the Supported header field with the value
>    'timer'.  If the request did not contain a Supported header field
>    with the value 'timer', the proxy *MAY* insert a Require header field
>    with the value 'timer' into the request.  However, this is *NOT
>    RECOMMENDED*.  This allows the proxy to insist on a session timer for
>    the session.  This header field is not needed if a Supported header
>    field was in the request; in this case, the proxy would already be
>    sure the session timer can be used for the session.
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_sip.c 269269 
>   /trunk/configs/sip.conf.sample 269269 
> 
> Diff: https://reviewboard.asterisk.org/r/698/diff
> 
> 
> Testing
> -------
> 
> Compile
> 
> 
> Thanks,
> 
> Darren
> 
>