On Sat, Mar 10, 2018 at 1:03 PM, Brad House <brad_at_mainstreetsoftworks.com>
wrote:
> Did ares_set_servers_csv() return ARES_EBADSTR as it should?
>
There's a test that implies that it will:
https://github.com/c-ares/c-ares/blob/master/test/ares-test-misc.cc#L86-L88
> It does appear when it does that, it leaves the channel in a "bad" state
> since existing servers are cleared before parse. I'm not sure of any other
> instances where a channel might have no servers, as even in the case where
> it can't determine one from configuration, it uses a localhost fallback.
> It may make more sense to kill the ares__destroy_servers_state() call at
> the beginning of set_servers_csv(), since ares_set_servers_ports() clears
> it which gets called after a successful parse.
>
> I'd agree that there should probably be a sanity check in ares_send() to
> not try an invalid ares_malloc(), ARES_ESERVFAIL would be as good an error
> condition as any in this case.
>
+1
>
> -Brad
>
>
> On 3/9/18 7:07 PM, Francisco Sedano Crippa (fsedanoc) wrote:
>
> Hello,
>
> I noticed today if you pass a string with spaces to set_servers_csv, like:
>
> "127.0.0.1 , 200.0.0.1"
>
> It will take the first server as "127.0.0.1 " (note the space), it will
> notice it's not a valid IP and fail. So far so good.
>
> However, nservers for the channel will stay set to -1, so when ares_send
> is called, this will be executed:
>
> query->server_info = ares_malloc(channel->nservers *
> sizeof(query->server_info[0]));
>
> The negative value will be misinterpreted to a huge number since argument
> is size_t and we agree things smell really bad from here. In practice, such
> a mem allocation fails and we return ENOMEM (which is also misleading), but
> it's a very incorrect behaviour.
>
> I was thinking on just adding a check at the beginning of ares_send() to
> exit if nservers is <= 0.
>
> Do you guys agree with the approach? If that’s the case, which error do
> you suggest to return? No one really matches, I’d say ARES_ENOTFOUND, but
> that implies we tried to contact the server…
>
>
>
> Thanks!
>
>
>
>
>
> *. .:|:.:|:. Francisco Sedano | CCIE 14859, Tech Lead Software
> Engineering | CSG Enterprise Access and Services Group (EASG)*
>
>
>
>
>
Received on 2018-03-11