On Thu, Jul 24, 2014 at 3:12 PM, Jakub Hrozek <jhrozek_at_redhat.com> wrote:
> On Thu, Jul 24, 2014 at 11:24:23AM +0100, David Drysdale wrote:
> > On Thu, Jul 24, 2014 at 10:53 AM, Jakub Hrozek <jhrozek_at_redhat.com>
> wrote:
> >
> > > On Thu, Jul 24, 2014 at 09:47:10AM +0100, David Drysdale wrote:
> > > > On Wed, Jul 23, 2014 at 4:59 PM, Jakub Hrozek <jhrozek_at_redhat.com>
> > > wrote:
> > > >
> > > > > Hi,
> > > > >
> > > > > I'm seeing the behaviour described in $SUBJECT with git head of
> c-ares
> > > > > and I'd like to discuss the best way of fixing it.
> > > > >
> > > > > If you search with ares_gethostbyname() with lookup family set to
> > > > > AF_UNSPEC,
> > > > > the first family to try would be AF_INET6. The code would jump to
> > > > > host_callback() on completing the query and try to parse the
> returned
> > > abuf:
> > > > >
> > > > > 188 else if (hquery->sent_family == AF_INET6)
> > > > > 189 {
> > > > > 190 status = ares_parse_aaaa_reply(abuf, alen, &host,
> NULL,
> > > > > NULL);
> > > > > 191 if ((status == ARES_ENODATA || status ==
> ARES_EBADRESP)
> > > &&
> > > > > 192 hquery->want_family == AF_UNSPEC) {
> > > > > 193 /* The query returned something but either there
> were
> > > no
> > > > > AAAA
> > > > > 194 records (e.g. just CNAME) or the response was
> > > > > malformed. Try
> > > > > 195 looking up A instead. */
> > > > >
> > > > > But since commit b3afe9cbdee797652d8cfba49aa2e9d42bf781fe parsing a
> > > query
> > > > > that only contains CNAMES returns ARES_SUCCESS and a hostent that
> > > contains
> > > > > the aliases, so the condition on list 191 never matches and
> AF_INET is
> > > > > never retried. I don't see a way for ares_parse_aaaa_reply()
> caller to
> > > > > see if the data returned contain a real AAAA response or just
> CNAMEs.
> > > > >
> > > > > I think ares_parse_aaaa_reply() behaviour is correct now and
> > > > > I think host_callback() here is buggy, or rather relies on
> > > > > bug in ares_parse_aaaa_reply(). Since we apparently can't use
> > > > > ares_parse_aaaa_reply(), I'd like to suggest a new internal
> function,
> > > > > something like:
> > > > >
> > > > > int ares_peek_aaaa_reply(const unsigned char *abuf, int alen,
> > > > > int *naaaa, int *ncname);
> > > > >
> > > > > And change the fallback condition to:
> > > > > status = ares_peek_aaaa_reply(abuf, alen, &naaaa, &ncname);
> > > > > if ((status == ARES_ENODATA || status == ARES_EBADRESP ||
> > > > > (status == ARES_SUCCESS && naaaa == 0)) &&
> > > > > hquery->want_family == AF_UNSPEC) {
> > > > > }
> > > > >
> > > > > Would that be an acceptable solution? Is there any other/better
> way?
> > > > >
> > > > > Thanks for any suggestions.
> > > > >
> > > >
> > > > Could you just extend the check on 191-192 to explicitly look in
> host for
> > > > returned addresses?
> > > > Something like:
> > > > if ((status == ARES_ENODATA || status == ARES_EBADRESP ||
> > > > (status == ARES_SUCCESS && host &&
> host->h_addr_list[0] ==
> > > > NULL)) &&
> > > > hquery->want_family == AF_UNSPEC) {
> > >
> > > Sure, I was just wondering if this is not considered relying on yet
> > > another 'side effect'. The 'peek' would really return exactly the data
> > > host_callback is interested in.
> > >
> > > But on the other hand you're right that we should strive for minimal
> > > changes.
> > >
> > > Thanks for the suggestion, David, I've attached a patch.
> > >
> >
> > That seems to work for me -- I've attached a patch to ahost that lets us
> > test it out (e.g.
> > "./ahost -t u www.comcast.com.", which is a domain with a CNAME but no
> AAAA
> > records).
>
> Thanks for the review!
>
> > From fcb2d96724c7bcc0efe311c54732d2225f0ac179 Mon Sep 17 00:00:00 2001
> > From: David Drysdale <drysdale_at_google.com>
> > Date: Thu, 24 Jul 2014 11:18:46 +0100
> > Subject: [PATCH] Add -t u option to ahost
>
> Can you also fix the usage output? I'm still seeing:
>
> ./ahost --help
> usage: ahost [-t {a|aaaa}] {host|addr} ...
>
Ah, I missed that. Updated patch attached.