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
base: master
from

Conversation

Projects
None yet
3 participants
@antlarr
Contributor

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.

Fix invalid "Network is unreachable" errors
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

This comment has been minimized.

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;
Use a error_save variable to hold the error value
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 #794
@jay

This comment has been minimized.

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

This comment has been minimized.

Contributor

antlarr commented May 6, 2016

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

@bagder

This comment has been minimized.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment