On 3/6/2017 3:20 AM, Daniel Stenberg wrote:
>
> No, if it already expired it should return error at once. I'll push this:
>
> @@ -357,10 +357,15 @@ CURLcode Curl_resolver_wait_resolv(struct
> connectdata *conn,
> long timeout;
> struct timeval now = Curl_tvnow();
> struct Curl_dns_entry *temp_entry;
>
> timeout = Curl_timeleft(data, &now, TRUE);
> + if(timeout < 0) {
> + /* already expired! */
> + connclose(conn, "Timed out before name resolve started");
> + return CURLE_OPERATION_TIMEDOUT;
> + }
> if(!timeout)
> timeout = CURL_TIMEOUT_RESOLVE * 1000; /* default name resolve
> timeout */
>
> /* Wait for the name resolve query to complete. */
> for(;;) {

Can you take a closer look at the function, a few things stick out to
me. I agree that's the most logical but the other timeout code does not
operate that way. Maybe there are several bugs?

Documented in part

"Returns CURLE_COULDNT_RESOLVE_HOST if the host was not resolved, and
CURLE_OPERATION_TIMEDOUT if a time-out occurred."

Neither of those are returned though. For a different timeout case it
appears CURLE_OK will be returned and nothing closed [1], and I can't
see how CURLE_COULDNT_RESOLVE_HOST could be returned without checking
Curl_resolver_is_resolved result. This is based on my read of the code,
I don't c-ares so I haven't walked through this at runtime.

diff --git a/lib/asyn-ares.c b/lib/asyn-ares.c
index c038c2a..2cf5c6e 100644
--- a/lib/asyn-ares.c
+++ b/lib/asyn-ares.c
@@ -385,9 +385,9 @@ CURLcode Curl_resolver_wait_resolv(struct
connectdata *conn,
       timeout_ms = 1000;
 
     waitperform(conn, timeout_ms);
- Curl_resolver_is_resolved(conn, &temp_entry);
+ result = Curl_resolver_is_resolved(conn, &temp_entry);
 
- if(conn->async.done)
+ if(result || conn->async.done)
       break;
 
     if(Curl_pgrsUpdate(conn)) {
@@ -404,6 +404,8 @@ CURLcode Curl_resolver_wait_resolv(struct
connectdata *conn,
     if(timeout < 0) {
       /* our timeout, so we cancel the ares operation */
       ares_cancel((ares_channel)data->state.resolver);
+ if(!result)
+ result = CURLE_OPERATION_TIMEDOUT;
       break;
     }
   }

It's unclear to me whether ares_cancel needs to be called if
Curl_resolver_is_resolved returns an error. Or maybe it depends on the
error?

[1]: https://github.com/curl/curl/blob/curl-7_53_1/lib/asyn-ares.c#L397-L423

-------------------------------------------------------------------
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette: https://curl.haxx.se/mail/etiquette.html
Received on 2017-03-06