Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix invalid "Network is unreachable" errors #794

Closed
wants to merge 2 commits into from

Conversation

antlarr
Copy link
Contributor

@antlarr antlarr commented May 5, 2016

Sometimes, in systems with both ipv4 and ipv6 addresses but where the
network doesn't support ipv6, Curl_is_connected returns an error
(intermittently) even if the ipv4 socket connects successfully.

This happens because there's a for-loop that iterates on the sockets
but the error variable is not resetted when the ipv4 is checked and
is ok.

This patch fixes this problem by setting error to 0 when checking the
second socket and not having a result yet.

Sometimes, in systems with both ipv4 and ipv6 addresses but where the
network doesn't support ipv6, Curl_is_connected returns an error
(intermittently) even if the ipv4 socket connects successfully.

This happens because there's a for-loop that iterates on the sockets
but the error variable is not resetted when the ipv4 is checked and
is ok.

This patch fixes this problem by setting error to 0 when checking the
second socket and not having a result yet.
@jay
Copy link
Member

jay commented May 5, 2016

Ref: https://curl.haxx.se/mail/lib-2016-05/0019.html

We use that variable later on though and I think in that case we don't want to reset it

diff --git a/lib/connect.c b/lib/connect.c
index 8dfe9e2..39fd064 100644
--- a/lib/connect.c
+++ b/lib/connect.c
@@ -722,7 +722,7 @@ CURLcode Curl_is_connected(struct connectdata *conn,
   struct SessionHandle *data = conn->data;
   CURLcode result = CURLE_OK;
   long allow;
-  int error = 0;
+  int error_save = 0;
   struct timeval now;
   int rc;
   int i;
@@ -749,6 +749,7 @@ CURLcode Curl_is_connected(struct connectdata *conn,
   }

   for(i=0; i<2; i++) {
+    int error = 0;
     const int other = i ^ 1;
     if(conn->tempsock[i] == CURL_SOCKET_BAD)
       continue;
@@ -817,6 +818,7 @@ CURLcode Curl_is_connected(struct connectdata *conn,
      * address" for the given host. But first remember the latest error.
      */
     if(error) {
+      error_save = error;
       data->state.os_errno = error;
       SET_SOCKERRNO(error);
       if(conn->tempaddr[i]) {
@@ -859,7 +861,7 @@ CURLcode Curl_is_connected(struct connectdata *conn,
       hostname = conn->host.name;

     failf(data, "Failed to connect to %s port %ld: %s",
-        hostname, conn->port, Curl_strerror(conn, error));
+        hostname, conn->port, Curl_strerror(conn, error_save));
   }

   return result;

We use the error variable later on though and in that case we don't want to reset it.
As suggested by Jay Satiro on curl#794
@jay
Copy link
Member

jay commented May 6, 2016

I took another look at this just now and it seems that with your patch there's no path for what I thought could happen to actually happen.

@antlarr
Copy link
Contributor Author

antlarr commented May 6, 2016

I thought so, but just in case I commited your patch too. Should I revert it?

@bagder
Copy link
Member

bagder commented May 8, 2016

No need, I'll just merge the first patch and leave out the second one. Thanks!

@bagder bagder closed this in ae8f662 May 8, 2016
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

None yet

3 participants