<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/1574/">https://reviewboard.asterisk.org/r/1574/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On November 8th, 2011, 8 p.m., <b>Simon Perreault</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Another thing: The term "domain" comes from the SIP RFC. It has a precise meaning. IMHO changing it to "hostport" is wrong and we should prefer using the SIP terms instead of defining our own equivalent terms.
Another reason I'm opposed to the name change is that it creates a lot of frivolous changes in the code which make svn blame|diff|merge|... harder to use.</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">> Another thing: The term "domain" comes from the SIP RFC. It has a precise meaning. IMHO changing it to "hostport" is wrong and we should prefer using the SIP terms instead of defining our own equivalent terms.
Actually, as far as parsing is concerned, there is no such thing as a "domain". Please see the ABNF of RFC 3261. There is a "hostport" though, and it is exactly what parse_uri returns in what was labeled as "domain". There is a domainlabel which is potentially part of the hostname, which is potentially part of the host, which is potentially part of hostport. The "domain" in SIP should absolutely never contain a port (or a ':').
> Another reason I'm opposed to the name change is that it creates a lot of frivolous changes in the code which make svn blame|diff|merge|... harder to use.
I would rather svn blame be harder to use than for bugs to be introduced because someone actually thinks that a domain is being returned when it isn't due to poor labeling.</pre>
<br />
<p>- Terry</p>
<br />
<p>On November 8th, 2011, 7:41 p.m., Terry Wilson wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers and wdoekes.</div>
<div>By Terry Wilson.</div>
<p style="color: grey;"><i>Updated Nov. 8, 2011, 7:41 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">In reviewing https://reviewboard.asterisk.org/r/1533/ I noticed that going from 1.4 to 1.8, setting domain=x.x.x.x stopped matching when receiving a request to x.x.x.x. The reason is that the parse_uri function returns host:port in what was labeled the "domain" variable in 1.8 when in 1.4 the parsing was done in each individual function and the port truncated manually. For example, peer->fromdomain could be set to a host:port, even though it is specifically split into peer->fromdomain and peer->fromport other places in the code.
We can't change the parsing function because sometimes we actually use the host:port form of the result (as in when passing it to ast_sockaddr_resolve_first), so we need to do the truncation manually when appropriate. This patch does two things: 1) it renames the "domain" variables in the API functions to "hostport" to more accurately reflect what we are really returning and 2) implements a truncate_hostport function and uses it where appropriate.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Ran the SIP unit tests and SIP-related testsuite tests and saw no difference in behavior. Set domain=127.0.0.1 and verified register requests started working.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/branches/1.8/channels/chan_sip.c <span style="color: grey">(343673)</span></li>
<li>/branches/1.8/channels/sip/include/reqresp_parser.h <span style="color: grey">(343673)</span></li>
<li>/branches/1.8/channels/sip/include/sip.h <span style="color: grey">(343673)</span></li>
<li>/branches/1.8/channels/sip/reqresp_parser.c <span style="color: grey">(343673)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1574/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>