[asterisk-dev] [Code Review] 3482: func_presencestate: Make base64 encoded-ness consistent for consumers of presence state

[Joshua Colp]

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

Ship it!


Ship It!

- Joshua Colp


On April 25, 2014, 9:34 p.m., Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3482/
> -----------------------------------------------------------
> 
> (Updated April 25, 2014, 9:34 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-23671
>     https://issues.asterisk.org/jira/browse/ASTERISK-23671
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> The 'e' option for the PRESENCE_STATE() function is not very well defined. Specifically, when using the function in write mode, it is unclear whether consumers of presence state events should expect to receive base64-encoded values or not. Further, the behavior is not consistent within the module. When the initial presence state is written, base64-encoded values are written to stasis and consumers receive these encoded values. However, if the ast_presence_state() function is called to retrieve the current presence values, decoded values are returned.
> 
> With this patch, if the subtype and message given in the PRESENCE_STATE() function are base64-encoded, these values are decoded before being sent to stasis. This way, consumers of presence state will always be guaranteed to get decoded values.
> 
> So with this patch, you can do the following:
> 
> exten => blah,1,Set(PRESENCE_STATE(CustomPresence:blah)=away,bHVuY2g=,Q2xlbSdzIENsYW1z,e) ; Sends consumers state=away, subtype="lunch", message="Clem's Clams". Stores base64 in astdb
> exten => blah,n,Set(subtype=${PRESENCE_STATE(CustomPresence:blah,subtype)}) ; Sets subtype to "lunch"
> exten => blah,n,Set(message=${PRESENCE_STATE(CustomPresence:blah,message)}) ; Sets message to "Clem's Clams"
> 
> If you actually want to be sending Base64-encoded data to consumers, then omit the e option.
> 
> exten => blah,1,Set(PRESENCE_STATE(CustomPresence:blah)=away,bHVuY2g=,Q2xlbSdzIENsYW1z) ; Sends consumers state=away, subtype="bHVuY2g=", message=Q2xlbSdzIENsYW1z. Stores base64 in astdb
> exten => blah,n,Set(subtype=${PRESENCE_STATE(CustomPresence:blah,subtype)}) ; Sets subtype to "bHVuY2g="
> exten => blah,n,Set(message=${PRESENCE_STATE(CustomPresence:blah,message)}) ; Sets message to "Q2xlbSdzIENsYW1z"
> exten => blah,n,Set(subtype=${BASE64_DECODE(${PRESENCE_STATE(CustomPresence:blah,subtype)})}) ; Sets subtype to "lunch"
> exten => blah,n,Set(message=${BASE64_DECODe(${PRESENCE_STATE(CustomPresence:blah,message)})}) ; Sets message to "Clem's Clams"
> 
> To me, this behavior seems at the very least more consistent than what was being done before. I'm certainly willing to hear objections, though.
> 
> 
> Diffs
> -----
> 
>   /trunk/funcs/func_presencestate.c 412583 
> 
> Diff: https://reviewboard.asterisk.org/r/3482/diff/
> 
> 
> Testing
> -------
> 
> I have added a unit test that ensures this behaves as expected. In doing so, I realized it was nearly identical to the previous test_presence_state_change test, so I refactored the code to be reusable and to plug some memory leaks and stasis subscription leaks.
> 
> 
> Thanks,
> 
> Mark Michelson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140429/f49fd17e/attachment.html>