Did ares_set_servers_csv() return ARES_EBADSTR as it should? 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.
-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)*
