<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/2400/">https://reviewboard.asterisk.org/r/2400/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 18th, 2013, 12:44 p.m., <b>elguero</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://reviewboard.asterisk.org/r/2400/diff/1/?file=34745#file34745line16478" style="color: black; font-weight: bold; text-decoration: underline;">/branches/1.8/channels/chan_sip.c</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">static void check_via(struct sip_pvt *p, struct sip_request *req)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">16475</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">ast_log</span><span class="p">(</span><span class="n">LOG_WARNING</span><span class="p">,</span> <span class="s">"Could not resolve socket address for '%s'</span><span class="se">\n</span><span class="s">"</span><span class="p">,</span> <span class="n">c</span><span class="p">);</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">16476</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="p">(</span><span class="n">ast_sockaddr_split_hostport</span><span class="p">(</span><span class="n">tmp</span><span class="p">,</span> <span class="o">&</span><span class="n">tmphost</span><span class="p">,</span> <span class="o">&</span><span class="n">tmpport</span><span class="p">,</span> <span class="mi">0</span><span class="p">)</span> <span class="o">&&</span></pre></td>
</tr>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">16476</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">port</span> <span class="o">=</span> <span class="n">STANDARD_SIP_PORT</span><span class="p">;</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">16477</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">tmpport</span> <span class="o">&&</span> <span class="p">(</span><span class="n">port</span> <span class="o">=</span> <span class="n">strtol</span><span class="p">(</span><span class="n">tmpport</span><span class="p">,</span> <span class="o">&</span><span class="n">tmpport2</span><span class="p">,</span> <span class="mi">10</span><span class="p">))</span> <span class="o">&&</span></pre></td>
</tr>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">16477</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">}</span> <span class="k">else</span> <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="p">(</span><span class="n">port</span> <span class="o">=</span> <span class="n">ast_sockaddr_port</span><span class="p">(</span><span class="o">&</span><span class="n">tmp</span><span class="p">)))</span> <span class="p">{</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">16478</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="o">*</span><span class="n">tmpport2</span> <span class="o">==</span> <span class="sc">'\0'</span> <span class="o">&&</span> <span class="n">port</span> <span class="o">></span> <span class="mi">0</span> <span class="o">&&</span> <span class="n">port</span> <span class="o"><</span> <span class="mi">65536</span><span class="p">))</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Curious if we could use what is in asterisk code for checking if ports are valid, although it is pretty clear as it is.
You could call ast_sockaddr_split_hostport with the flag PARSE_PORT_REQUIRED which would fail immediately without any extra checks needed.
if (!ast_sockaddr_split_hostport(tmp, &tmphost, &tmpport, PARSE_PORT_REQUIRED)) {
ast_log(LOG_WARNING, "Could not split host/port for '%s'\n", c);
}
Upon success of the above, you could also check that the port returned is valid by using ast_parse_arg().
else if (ast_parse_arg(&tmpport, PARSE_UINT32|PARSE_IN_RANGE, &port, 1024, 65535)) {
ast_log(LOG_WARNING, "Invalid port number, '%s', in VIA header\n", tmpport);
port = STANDARD_SIP_PORT;
}
In other parts of the code, when we check for valid ports we are using the range 1024-65535, not 0-65535.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I meant 1-65535.</pre>
<br />
<p>- elguero</p>
<br />
<p>On March 18th, 2013, 9:16 a.m., wdoekes 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.</div>
<div>By wdoekes.</div>
<p style="color: grey;"><i>Updated March 18, 2013, 9:16 a.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;">The sent-by-addr address in the Via is supposed to get ignored and the sent-by-port is only used if we're not doing force_rport.
Nevertheless did check_via do hostname lookups even though it only wanted to get at the port.
I've changed the code around so it uses ast_sockaddr_split_hostport() instead of ast_sockaddr_resolve_first(). This also removes an ERROR notice which isn't worthy of such severity.
Before, when someone sent something resolvable in the Via:
chan_sip did an DNS A lookup, but ignored the results.
Now:
chan_sip just looks at the port.
Before, when someone sent a bad via header:
ERROR[29769]: netsock2.c:269 ast_sockaddr_resolve: getaddrinfo("127.0.1.1", "5061branch=z9hG4bK-11063-1-0", ...): Servname not supported for ai_socktype
WARNING[29769]: chan_sip.c:16921 check_via: Could not resolve socket address for '127.0.1.1:5061branch=z9hG4bK-11063-1-0'
Now:
WARNING[10229]: chan_sip.c:16928 check_via: Could not split host/port for '127.0.1.1:5061branch=z9hG4bK-11049-1-0'
</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;">57 SIP tests ran successfully with and without the patch.</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">(383308)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2400/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>