[asterisk-dev] [Code Review]: Fix broken SIP realtime update of lastms (and a couple other SIP realtime issues)

[Terry Wilson]

> On Feb. 1, 2012, 11:26 a.m., Mark Michelson wrote:
> > /branches/1.8/contrib/realtime/postgresql/realtime.sql, line 68
> > <https://reviewboard.asterisk.org/r/1703/diff/1/?file=23823#file23823line68>
> >
> >     Why this change?

Because 40 characters is too few to hold a full IPv6 address and the call to ast_realtime_require requires INET6_ADDRSTRLEN - 1 characters at  a minimum to be allowed, which works out to be 45 (I'm nut sure why it does - 1, though). I rounded up to 50.


- Terry


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


On Jan. 31, 2012, 10:09 p.m., Terry Wilson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1703/
> -----------------------------------------------------------
> 
> (Updated Jan. 31, 2012, 10:09 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> SIP realtime, upon peer destruction, tries to clear some values by updating the peer with empty string values. This works if those values are character fields or if the database automagically translates an empty string to a NULL or 0 when confronted with an integer field. PostgreSQL does not do this translation and returns an error (even if called through res_config_odbc). lastms is defined as an integer in both the mysql and postgresql realtime SQL files.
> 
> Looking at where lastms is set in chan_sip (handle_response_peerpoke) it is clear that the default value of lastms is 0 and that 0 means unknown, while -1 means unreachable. So, like regseconds, the update for destruction should be passing "0" for a value instead of "". The fields could be changed to character fields, but this would still require us to interpret an empty string as a 0 value. This patch changes the destruction update to pass a value of "0" for lastms.
> 
> Other issues in chan_sip include setting the ipaddr and port to the string "(null)" when they are null instead of an empty string which is what is expected (thanks to ast_sockaddr_stringify returning "(null)" for a null sockaddr). This patch checks the values of ast_sockaddr_isnull() and ast_sockaddr_port and passes empty strings when appropriate.
> 
> The postgresql realtime.sql file has been updated to set the correct default of '0' for lastms, lengthen the ipaddr field (which was too short for the longest possible IPv6 address), and add the missing defaultuser, fullcontact, regserver, and useragent fields.
> 
> 
> This addresses bug ASTERISK-19172.
>     https://issues.asterisk.org/jira/browse/ASTERISK-19172
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/channels/chan_sip.c 353501 
>   /branches/1.8/contrib/realtime/postgresql/realtime.sql 353501 
> 
> Diff: https://reviewboard.asterisk.org/r/1703/diff
> 
> 
> Testing
> -------
> 
> Under res_config_pgsql and res_config_odbc, verified that registering and unregistering a peer no longer produces any SQL errors.
> 
> 
> Thanks,
> 
> Terry
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120201/78c6778a/attachment.htm>