fix: re-use of bound connections

When asked to bind the local end of a connection when doing a request,
the code will now disqualify other existing connections from re-use even
if they are connected to the correct remote host.

This will also affect which connections that can be used for pipelining,
so that only connections that aren't bound or bound to the same
device/port you're asking for will be considered.

diff --git a/TODO-RELEASE b/TODO-RELEASE
index 2ced28753bb2473c5b3176f1fb17f5d7cc832a68..36d5e0d427cc28f3f7db79f75d511ccdee7a2802 100644 (file)
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -1,8 +1,6 @@
 To be addressed in 7.21.5
 =========================
 
-272 - re-using connections bound to local port
-
 275 - Introduce a way to avoid sending USER for FTP connections
 
 278 - "Configure  $as_echo does not work"

diff --git a/lib/url.c b/lib/url.c
index 106d4e7ec93938eeae20fbe7ec1b80bf897a54b1..25f03b7e2d418f64c36d010707ea69e4b7888c76 100644 (file)
--- a/lib/url.c
+++ b/lib/url.c
@@ -2606,7 +2606,7 @@ static void conn_free(struct connectdata *conn)
   Curl_safefree(conn->async.hostname);
   Curl_safefree(conn->async.os_specific);
 #endif
-
+  Curl_safefree(conn->localdev);
   Curl_free_ssl_config(&conn->ssl_config);
 
   free(conn); /* free all the connection oriented data */
@@ -2987,6 +2987,25 @@ ConnectionExists(struct SessionHandle *data,
          in use so we skip it */
       continue;
 
+    if(needle->localdev || needle->localport) {
+      /* If we are bound to a specific local end (IP+port), we must not re-use
+         a random other one, although if we didn't ask for a particular one we
+         can reuse one that was bound.
+
+         This comparison is a bit rough and too strict. Since the input
+         parameters can be specified in numerous ways and still end up the
+         same it would take a lot of processing to make it really accurate.
+         Instead, this matching will assume that re-uses of bound connections
+         will most likely also re-use the exact same binding parameters and
+         missing out a few edge cases shouldn't hurt anyone very much.
+      */
+      if((check->localport != needle->localport) ||
+         (check->localportrange != needle->localportrange) ||
+         !check->localdev ||
+         strcmp(check->localdev, needle->localdev))
+        continue;
+    }
+
     if(!needle->bits.httpproxy || needle->handler->flags&PROTOPT_SSL ||
        (needle->bits.httpproxy && check->bits.httpproxy &&
         needle->bits.tunnel_proxy && check->bits.tunnel_proxy &&
@@ -3568,13 +3587,24 @@ static struct connectdata *allocate_conn(struct SessionHandle *data)
   conn->data_prot = PROT_CLEAR;
 #endif
 
+  /* Store the local bind parameters that will be used for this connection */
+  if(data->set.str[STRING_DEVICE]) {
+    conn->localdev = strdup(data->set.str[STRING_DEVICE]);
+    if(!conn->localdev)
+      goto error;
+  }
+  conn->localportrange = data->set.localportrange;
+  conn->localport = data->set.localport;
+
   return conn;
   error:
+
   Curl_llist_destroy(conn->send_pipe, NULL);
   Curl_llist_destroy(conn->recv_pipe, NULL);
   Curl_llist_destroy(conn->pend_pipe, NULL);
   Curl_llist_destroy(conn->done_pipe, NULL);
   Curl_safefree(conn->master_buffer);
+  Curl_safefree(conn->localdev);
   Curl_safefree(conn);
   return NULL;
 }

diff --git a/lib/urldata.h b/lib/urldata.h
index 7588cedc55c1060c5395926ce1edb8d73a792899..d1718a9a46f0dfc4bb20ecf47fee0a3a99071171 100644 (file)
--- a/lib/urldata.h
+++ b/lib/urldata.h
@@ -936,6 +936,16 @@ struct connectdata {
 
   long verifypeer;
   long verifyhost;
+
+  /* When this connection is created, store the conditions for the local end
+     bind. This is stored before the actual bind and before any connection is
+     made and will serve the purpose of being used for comparison reasons so
+     that subsequent bound-requested connections aren't accidentally re-using
+     wrong connections. */
+  char *localdev;
+  unsigned short localport;
+  int localportrange;
+
 };
 
 /* The end of connectdata. */