[asterisk-dev] [Code Review] Make http.c IPv6-ready

[Mark Michelson]

> On 2010-10-29 15:07:09, Simon Perreault wrote:
> > Two issues:
> > 
> > - The sample config says that the default bindaddr is 127.0.0.1. From the code it doesn't look like it's the case.
> > 
> > - Why isn't there a separate bindaddr for TLS?
> 
> Mark Michelson wrote:
>     1) I think the sample config is wrong. Looking at the code in 1.8 and 1.6.2 at least, it appears to actually default to 0.0.0.0.
>     2) There is. We call ast_tls_read_conf() to determine what the TLS bindaddr is. If none is specified, then we use the same address as the non-TLS bindaddr but use a different port. Note that ast_tls_read_conf() won't resolve hostnames though, and I think that needs to be addressed in a different change.

To expand upon 2), it appears that in 1.6.2, we don't use ast_tls_read_conf(), and we DO accept hostnames there. This appears to be a regression in 1.8. Thus, it is more than just an inconsistency. It is a regression.


- Mark


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


On 2010-10-29 14:56:37, Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/986/
> -----------------------------------------------------------
> 
> (Updated 2010-10-29 14:56:37)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This patch allows for IPv6 addresses to be used when configuring the HTTP server in Asterisk.
> 
> There is a behavior change here as well. Since ast_sockaddr_resolve() returns an array of addresses, we'll iterate over the addresses returned until we're able to properly start the server. Adding logic to fail back to a cached address was not added, as the scope of that is quite different.
> 
> I rewrote a good portion of the configuration-loading function, so when reviewing, be sure that I haven't introduced any subtle behavior changes. As a side note, it would be awesome if we could just get rid of any "bindport" options, but I know that would be hard to wrangle :)
> 
> As a side note, I noticed an odd inconsistency regarding setting the TLS and non-TLS bind addresses. The non-TLS bind address can be a hostname, but the TLS one cannot be, because it uses the built-in option parsing in tcptls.c. I did not address this because I did not want the scope of my changes to be too broad.
> 
> 
> Diffs
> -----
> 
>   /trunk/main/http.c 292864 
> 
> Diff: https://reviewboard.asterisk.org/r/986/diff
> 
> 
> Testing
> -------
> 
> None. I'm not in a position at the moment to be able to test very thoroughly.
> 
> 
> Thanks,
> 
> Mark
> 
>