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.
Received on 2014-07-24