connection: never reuse CONNECT_ONLY conections

and make CONNECT_ONLY conections never reuse any existing ones either.

Reported-by: Pavel Löbl
Bug: https://curl.haxx.se/mail/lib-2019-02/0064.html
Closes #3586

5 files changed, 55 insertions, 67 deletions

diff --git a/docs/libcurl/opts/CURLOPT_CONNECT_ONLY.3 b/docs/libcurl/opts/CURLOPT_CONNECT_ONLY.3
index 89a2fc12b..375d5d4d6 100644
--- a/docs/libcurl/opts/CURLOPT_CONNECT_ONLY.3
+++ b/docs/libcurl/opts/CURLOPT_CONNECT_ONLY.3
@@ -5,7 +5,7 @@
 .\" *                            | (__| |_| |  _ <| |___
 .\" *                             \___|\___/|_| \_\_____|
 .\" *
-.\" * Copyright (C) 1998 - 2017, Daniel Stenberg, <daniel@haxx.se>, et al.
+.\" * Copyright (C) 1998 - 2019, Daniel Stenberg, <daniel@haxx.se>, et al.
 .\" *
 .\" * This software is licensed as described in the file COPYING, which
 .\" * you should have received as part of this distribution. The terms
@@ -37,6 +37,9 @@ useful when used with the \fICURLINFO_ACTIVESOCKET(3)\fP option to
 \fIcurl_easy_getinfo(3)\fP as the library can set up the connection and then
 the application can obtain the most recently used socket for special data
 transfers.
+
+Transfers marked connect only will not reuse any existing connections and
+connections marked connect only will not be allowed to get reused.
 .SH DEFAULT
 0
 .SH PROTOCOLS
diff --git a/lib/url.c b/lib/url.c
index 335889903..61971a4ea 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -1133,6 +1133,10 @@ ConnectionExists(struct Curl_easy *data,
       check = curr->ptr;
       curr = curr->next;
 
+      if(check->bits.connect_only)
+        /* connect-only connections will not be reused */
+        continue;
+
       if(extract_if_dead(check, data)) {
         /* disconnect it */
         (void)Curl_disconnect(data, check, /* dead_connection */TRUE);
@@ -1893,8 +1897,8 @@ static struct connectdata *allocate_conn(struct Curl_easy *data)
     data->set.proxy_ssl.primary.verifystatus;
   conn->proxy_ssl_config.verifypeer = data->set.proxy_ssl.primary.verifypeer;
   conn->proxy_ssl_config.verifyhost = data->set.proxy_ssl.primary.verifyhost;
-
   conn->ip_version = data->set.ipver;
+  conn->bits.connect_only = data->set.connect_only;
 
 #if !defined(CURL_DISABLE_HTTP) && defined(USE_NTLM) && \
     defined(NTLM_WB_ENABLED)
@@ -3871,8 +3875,9 @@ static CURLcode create_conn(struct Curl_easy *data,
   /* reuse_fresh is TRUE if we are told to use a new connection by force, but
      we only acknowledge this option if this is not a re-used connection
      already (which happens due to follow-location or during a HTTP
-     authentication phase). */
-  if(data->set.reuse_fresh && !data->state.this_is_a_follow)
+     authentication phase). CONNECT_ONLY transfers also refuse reuse. */
+  if((data->set.reuse_fresh && !data->state.this_is_a_follow) ||
+     data->set.connect_only)
     reuse = FALSE;
   else
     reuse = ConnectionExists(data, conn, &conn_temp, &force_reuse, &waitpipe);
diff --git a/lib/urldata.h b/lib/urldata.h
index b194fde23..cdedb8fb1 100644
--- a/lib/urldata.h
+++ b/lib/urldata.h
@@ -452,6 +452,7 @@ struct ConnectBits {
   bool proxy_ssl_connected[2]; /* TRUE when SSL initialization for HTTPS proxy
                                   is complete */
   bool socksproxy_connecting; /* connecting through a socks proxy */
+  bool connect_only;
 };
 
 struct hostname {
diff --git a/tests/data/test597 b/tests/data/test597
index 458bb6453..3eb9ed8b8 100644
--- a/tests/data/test597
+++ b/tests/data/test597
@@ -31,7 +31,6 @@ ftp://%HOSTIP:%FTPPORT
 USER anonymous

 PASS ftp@example.com

 PWD

-QUIT

 </protocol>
 </verify>
 </testcase>
diff --git a/tests/libtest/lib597.c b/tests/libtest/lib597.c
index d7f38c4c8..e34505cd2 100644
--- a/tests/libtest/lib597.c
+++ b/tests/libtest/lib597.c
@@ -5,7 +5,7 @@
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 1998 - 2017, Daniel Stenberg, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2019, Daniel Stenberg, <daniel@haxx.se>, et al.
  *
  * This software is licensed as described in the file COPYING, which
  * you should have received as part of this distribution. The terms
@@ -32,20 +32,12 @@
 /*
  * Test case for below scenario:
  *   - Connect to an FTP server using CONNECT_ONLY option
- *   - transfer some files with re-using the connection (omitted in test case)
- *   - Disconnect from FTP server with sending QUIT command
  *
  * The test case originated for verifying CONNECT_ONLY option shall not
  * block after protocol connect is done, but it returns the message
  * with function curl_multi_info_read().
  */
 
-enum {
-  CONNECT_ONLY_PHASE = 0,
-  QUIT_PHASE,
-  LAST_PHASE
-};
-
 int test(char *URL)
 {
   CURL *easy = NULL;
@@ -53,7 +45,6 @@ int test(char *URL)
   int res = 0;
   int running;
   int msgs_left;
-  int phase;
   CURLMsg *msg;
 
   start_test_timing();
@@ -64,76 +55,65 @@ int test(char *URL)
 
   multi_init(multi);
 
-  for(phase = CONNECT_ONLY_PHASE; phase < LAST_PHASE; ++phase) {
-    /* go verbose */
-    easy_setopt(easy, CURLOPT_VERBOSE, 1L);
-
-    /* specify target */
-    easy_setopt(easy, CURLOPT_URL, URL);
-
-    /* enable 'CONNECT_ONLY' option when in connect phase */
-    if(phase == CONNECT_ONLY_PHASE)
-      easy_setopt(easy, CURLOPT_CONNECT_ONLY, 1L);
-
-    /* enable 'NOBODY' option to send 'QUIT' command in quit phase */
-    if(phase == QUIT_PHASE) {
-      easy_setopt(easy, CURLOPT_CONNECT_ONLY, 0L);
-      easy_setopt(easy, CURLOPT_NOBODY, 1L);
-      easy_setopt(easy, CURLOPT_FORBID_REUSE, 1L);
-    }
+  /* go verbose */
+  easy_setopt(easy, CURLOPT_VERBOSE, 1L);
 
-    multi_add_handle(multi, easy);
+  /* specify target */
+  easy_setopt(easy, CURLOPT_URL, URL);
 
-    for(;;) {
-      struct timeval interval;
-      fd_set fdread;
-      fd_set fdwrite;
-      fd_set fdexcep;
-      long timeout = -99;
-      int maxfd = -99;
+  easy_setopt(easy, CURLOPT_CONNECT_ONLY, 1L);
 
-      multi_perform(multi, &running);
+  multi_add_handle(multi, easy);
 
-      abort_on_test_timeout();
+  for(;;) {
+    struct timeval interval;
+    fd_set fdread;
+    fd_set fdwrite;
+    fd_set fdexcep;
+    long timeout = -99;
+    int maxfd = -99;
 
-      if(!running)
-        break; /* done */
+    multi_perform(multi, &running);
 
-      FD_ZERO(&fdread);
-      FD_ZERO(&fdwrite);
-      FD_ZERO(&fdexcep);
+    abort_on_test_timeout();
 
-      multi_fdset(multi, &fdread, &fdwrite, &fdexcep, &maxfd);
+    if(!running)
+      break; /* done */
 
-      /* At this point, maxfd is guaranteed to be greater or equal than -1. */
+    FD_ZERO(&fdread);
+    FD_ZERO(&fdwrite);
+    FD_ZERO(&fdexcep);
 
-      multi_timeout(multi, &timeout);
+    multi_fdset(multi, &fdread, &fdwrite, &fdexcep, &maxfd);
 
-      /* At this point, timeout is guaranteed to be greater or equal than
-         -1. */
+    /* At this point, maxfd is guaranteed to be greater or equal than -1. */
 
-      if(timeout != -1L) {
-        int itimeout = (timeout > (long)INT_MAX) ? INT_MAX : (int)timeout;
-        interval.tv_sec = itimeout/1000;
-        interval.tv_usec = (itimeout%1000)*1000;
-      }
-      else {
-        interval.tv_sec = TEST_HANG_TIMEOUT/1000 + 1;
-        interval.tv_usec = 0;
-      }
+    multi_timeout(multi, &timeout);
 
-      select_test(maxfd + 1, &fdread, &fdwrite, &fdexcep, &interval);
+    /* At this point, timeout is guaranteed to be greater or equal than
+       -1. */
 
-      abort_on_test_timeout();
+    if(timeout != -1L) {
+      int itimeout = (timeout > (long)INT_MAX) ? INT_MAX : (int)timeout;
+      interval.tv_sec = itimeout/1000;
+      interval.tv_usec = (itimeout%1000)*1000;
+    }
+    else {
+      interval.tv_sec = TEST_HANG_TIMEOUT/1000 + 1;
+      interval.tv_usec = 0;
     }
 
-    msg = curl_multi_info_read(multi, &msgs_left);
-    if(msg)
-      res = msg->data.result;
+    select_test(maxfd + 1, &fdread, &fdwrite, &fdexcep, &interval);
 
-    multi_remove_handle(multi, easy);
+    abort_on_test_timeout();
   }
 
+  msg = curl_multi_info_read(multi, &msgs_left);
+  if(msg)
+    res = msg->data.result;
+
+  multi_remove_handle(multi, easy);
+
 test_cleanup:
 
   /* undocumented cleanup sequence - type UA */