[asterisk-dev] [Code Review] 2452: Fix Distributed Device State broken at sites using res_xmpp or res_jabber where Secuity Advisory AST-2012-015 is inplace.

[Matt Jordan]

> On April 15, 2013, 1:54 p.m., opticron wrote:
> > branches/11/main/devicestate.c, line 730
> > <https://reviewboard.asterisk.org/r/2452/diff/3/?file=35995#file35995line730>
> >
> >     This should be retreived as a uint.
> 
> Alec Davis wrote:
>     In the ideal world yes.
>     This makes devicestate tolerant, as res_jabber/res_xmpp weren't posting the AST_EVENT_IE_CACHABLE event, there wasn't a cacheable IE.
>     
>     As the 'retrieve as uint' returns 0 if the IE isn't found, that will set cachable to NOT_CACHEABLE, but we want the default to be CACHABLE.
>     So using 'retrieve as str' allows us to check if it's missing.

It isn't intuitively obvious why we need to extract the UINT out as a string. A comment with your rationale would be appropriate.


- Matt


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


On April 15, 2013, 9:35 p.m., Alec Davis wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2452/
> -----------------------------------------------------------
> 
> (Updated April 15, 2013, 9:35 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-21429
>     https://issues.asterisk.org/jira/browse/ASTERISK-21429
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> The site that doesn't update it's devicestates from the remotesite is the site with ASR-2012-015 inplace.
> 
> This fixes the scenario where one site doesn't send the IE cachable=1 in the XMPP/Jabber message, to a newer system that expects to see it.
> 
> If the IE 'cachable' isn't present we need to default to AST_DEVSTATE_CACHABLE.
>  Assigning the cachable variable directly from ast_event_get_ie_uint is wrong, if the IE 'cachable' isn't found ast_event_get_ie_uint() will return 0, 0 = AST_DEVSTATE_NOT_CACHABLE
> 
> We need to read the 'cachable' IE into a string first, allowing us to check that it exists, if it doesn't exist then default to cachable=AST_DEVSTATE_CACHABLE.
>  If the 'cachable' IE does exist, then convert to a uint, validate value and go with it.
> 
> 
> Caused by:
> https://issues.asterisk.org/jira/browse/ASTERISK-20175
> 
> 
> Diffs
> -----
> 
>   branches/11/main/devicestate.c 385594 
>   branches/11/res/res_jabber.c 385594 
>   branches/11/res/res_xmpp.c 385594 
> 
> Diff: https://reviewboard.asterisk.org/r/2452/diff/
> 
> 
> Testing
> -------
> 
> Yes, change device state, at either site, and it's reflected in the other. One site is prior to ASTERISK-21075.
> 
> Tested both sites at r385594, and states correct at both sites.
> 
> 
> Thanks,
> 
> Alec Davis
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130416/3a2a5037/attachment.htm>