url: fix logic in connection reuse to deny reuse on "unclean" connect…

…ions

- add parameter to `conn_is_alive()` cfilter method that returns
  if there is input data waiting on the connection
- refrain from re-using connnection from the cache that have
  input pending
- adapt http/2 and http/3 alive checks to digest pending input
  to check the connection state
- remove check_cxn method from openssl as that was just doing
  what the socket filter now does.
- add tests for connection reuse with special server configs

Closes #10690

lib/cf-socket.c

@@ -325,20 +325,6 @@ int Curl_socket_close(struct Curl_easy *data, struct connectdata *conn,
   return socket_close(data, conn, FALSE, sock);
 }
 
-bool Curl_socket_is_dead(curl_socket_t sock)
-{
-  int sval;
-  bool ret_val = TRUE;
-
-  sval = SOCKET_READABLE(sock, 0);
-  if(sval == 0)
-    /* timeout */
-    ret_val = FALSE;
-
-  return ret_val;
-}
-
-
 #ifdef USE_WINSOCK
 /* When you run a program that uses the Windows Sockets API, you may
    experience slow performance when you copy data to a TCP server.
@@ -1449,12 +1435,14 @@ static CURLcode cf_socket_cntrl(struct Curl_cfilter *cf,
 }
 
 static bool cf_socket_conn_is_alive(struct Curl_cfilter *cf,
-                                    struct Curl_easy *data)
+                                    struct Curl_easy *data,
+                                    bool *input_pending)
 {
   struct cf_socket_ctx *ctx = cf->ctx;
   struct pollfd pfd[1];
   int r;
 
+  *input_pending = FALSE;
   (void)data;
   if(!ctx || ctx->sock == CURL_SOCKET_BAD)
     return FALSE;
@@ -1479,6 +1467,7 @@ static bool cf_socket_conn_is_alive(struct Curl_cfilter *cf,
   }
 
   DEBUGF(LOG_CF(data, cf, "is_alive: valid events, looks alive"));
+  *input_pending = TRUE;
   return TRUE;
 }
 

lib/cf-socket.h

@@ -70,13 +70,6 @@ CURLcode Curl_socket_open(struct Curl_easy *data,
 int Curl_socket_close(struct Curl_easy *data, struct connectdata *conn,
                       curl_socket_t sock);
 
-/*
- * This function should return TRUE if the socket is to be assumed to
- * be dead. Most commonly this happens when the server has closed the
- * connection due to inactivity.
- */
-bool Curl_socket_is_dead(curl_socket_t sock);
-
 /**
  * Determine the curl code for a socket connect() == -1 with errno.
  */

lib/cfilters.c

@@ -124,10 +124,11 @@ ssize_t  Curl_cf_def_recv(struct Curl_cfilter *cf, struct Curl_easy *data,
 }
 
 bool Curl_cf_def_conn_is_alive(struct Curl_cfilter *cf,
-                               struct Curl_easy *data)
+                               struct Curl_easy *data,
+                               bool *input_pending)
 {
   return cf->next?
-    cf->next->cft->is_alive(cf->next, data) :
+    cf->next->cft->is_alive(cf->next, data, input_pending) :
     FALSE; /* pessimistic in absence of data */
 }
 
@@ -631,10 +632,12 @@ void Curl_conn_report_connect_stats(struct Curl_easy *data,
   }
 }
 
-bool Curl_conn_is_alive(struct Curl_easy *data, struct connectdata *conn)
+bool Curl_conn_is_alive(struct Curl_easy *data, struct connectdata *conn,
+                        bool *input_pending)
 {
   struct Curl_cfilter *cf = conn->cfilter[FIRSTSOCKET];
-  return cf && !cf->conn->bits.close && cf->cft->is_alive(cf, data);
+  return cf && !cf->conn->bits.close &&
+         cf->cft->is_alive(cf, data, input_pending);
 }
 
 CURLcode Curl_conn_keep_alive(struct Curl_easy *data,

lib/cfilters.h

@@ -85,7 +85,8 @@ typedef ssize_t  Curl_cft_recv(struct Curl_cfilter *cf,
                                CURLcode *err);         /* error to return */
 
 typedef bool     Curl_cft_conn_is_alive(struct Curl_cfilter *cf,
-                                        struct Curl_easy *data);
+                                        struct Curl_easy *data,
+                                        bool *input_pending);
 
 typedef CURLcode Curl_cft_conn_keep_alive(struct Curl_cfilter *cf,
                                           struct Curl_easy *data);
@@ -216,7 +217,8 @@ CURLcode Curl_cf_def_cntrl(struct Curl_cfilter *cf,
                                 struct Curl_easy *data,
                                 int event, int arg1, void *arg2);
 bool     Curl_cf_def_conn_is_alive(struct Curl_cfilter *cf,
-                                   struct Curl_easy *data);
+                                   struct Curl_easy *data,
+                                   bool *input_pending);
 CURLcode Curl_cf_def_conn_keep_alive(struct Curl_cfilter *cf,
                                      struct Curl_easy *data);
 CURLcode Curl_cf_def_query(struct Curl_cfilter *cf,
@@ -443,7 +445,8 @@ void Curl_conn_report_connect_stats(struct Curl_easy *data,
 /**
  * Check if FIRSTSOCKET's cfilter chain deems connection alive.
  */
-bool Curl_conn_is_alive(struct Curl_easy *data, struct connectdata *conn);
+bool Curl_conn_is_alive(struct Curl_easy *data, struct connectdata *conn,
+                        bool *input_pending);
 
 /**
  * Try to upkeep the connection filters at sockindex.

lib/http2.c

@@ -366,58 +366,63 @@ static void http2_stream_free(struct HTTP *stream)
   }
 }
 
+/*
+ * Returns nonzero if current HTTP/2 session should be closed.
+ */
+static int should_close_session(struct cf_h2_ctx *ctx)
+{
+  return ctx->drain_total == 0 && !nghttp2_session_want_read(ctx->h2) &&
+    !nghttp2_session_want_write(ctx->h2);
+}
+
 /*
  * The server may send us data at any point (e.g. PING frames). Therefore,
  * we cannot assume that an HTTP/2 socket is dead just because it is readable.
  *
  * Check the lower filters first and, if successful, peek at the socket
  * and distinguish between closed and data.
  */
-static bool http2_connisdead(struct Curl_cfilter *cf, struct Curl_easy *data)
+static bool http2_connisalive(struct Curl_cfilter *cf, struct Curl_easy *data,
+                              bool *input_pending)
 {
   struct cf_h2_ctx *ctx = cf->ctx;
-  int sval;
-  bool dead = TRUE;
+  bool alive = TRUE;
 
-  if(!cf->next || !cf->next->cft->is_alive(cf->next, data))
-    return TRUE;
+  *input_pending = FALSE;
+  if(!cf->next || !cf->next->cft->is_alive(cf->next, data, input_pending))
+    return FALSE;
 
-  sval = SOCKET_READABLE(Curl_conn_cf_get_socket(cf, data), 0);
-  if(sval == 0) {
-    /* timeout */
-    dead = FALSE;
-  }
-  else if(sval & CURL_CSELECT_ERR) {
-    /* socket is in an error state */
-    dead = TRUE;
-  }
-  else if(sval & CURL_CSELECT_IN) {
+  if(*input_pending) {
     /* This happens before we've sent off a request and the connection is
        not in use by any other transfer, there shouldn't be any data here,
        only "protocol frames" */
     CURLcode result;
     ssize_t nread = -1;
 
+    *input_pending = FALSE;
     Curl_attach_connection(data, cf->conn);
     nread = Curl_conn_cf_recv(cf->next, data,
                               ctx->inbuf, H2_BUFSIZE, &result);
-    dead = FALSE;
     if(nread != -1) {
       DEBUGF(LOG_CF(data, cf, "%d bytes stray data read before trying "
                     "h2 connection", (int)nread));
       ctx->nread_inbuf = 0;
       ctx->inbuflen = nread;
       if(h2_process_pending_input(cf, data, &result) < 0)
         /* immediate error, considered dead */
-        dead = TRUE;
+        alive = FALSE;
+      else {
+        alive = !should_close_session(ctx);
+      }
     }
-    else
+    else {
       /* the read failed so let's say this is dead anyway */
-      dead = TRUE;
+      alive = FALSE;
+    }
     Curl_detach_connection(data);
   }
 
-  return dead;
+  return alive;
 }
 
 static CURLcode http2_send_ping(struct Curl_cfilter *cf,
@@ -1451,15 +1456,6 @@ CURLcode Curl_http2_request_upgrade(struct dynbuf *req,
   return result;
 }
 
-/*
- * Returns nonzero if current HTTP/2 session should be closed.
- */
-static int should_close_session(struct cf_h2_ctx *ctx)
-{
-  return ctx->drain_total == 0 && !nghttp2_session_want_read(ctx->h2) &&
-    !nghttp2_session_want_write(ctx->h2);
-}
-
 /*
  * h2_process_pending_input() processes pending input left in
  * httpc->inbuf.  Then, call h2_session_send() to send pending data.
@@ -2359,14 +2355,17 @@ static bool cf_h2_data_pending(struct Curl_cfilter *cf,
 }
 
 static bool cf_h2_is_alive(struct Curl_cfilter *cf,
-                           struct Curl_easy *data)
+                           struct Curl_easy *data,
+                           bool *input_pending)
 {
   struct cf_h2_ctx *ctx = cf->ctx;
   CURLcode result;
   struct cf_call_data save;
 
   CF_DATA_SAVE(save, cf, data);
-  result = (ctx && ctx->h2 && !http2_connisdead(cf, data));
+  result = (ctx && ctx->h2 && http2_connisalive(cf, data, input_pending));
+  DEBUGF(LOG_CF(data, cf, "conn alive -> %d, input_pending=%d",
+         result, *input_pending));
   CF_DATA_RESTORE(cf, save);
   return result;
 }

lib/rtsp.c

@@ -145,7 +145,8 @@ static unsigned int rtsp_conncheck(struct Curl_easy *data,
   (void)data;
 
   if(checks_to_perform & CONNCHECK_ISDEAD) {
-    if(!Curl_conn_is_alive(data, conn))
+    bool input_pending;
+    if(!Curl_conn_is_alive(data, conn, &input_pending))
       ret_val |= CONNRESULT_DEAD;
   }
 

lib/url.c

@@ -965,7 +965,20 @@ static bool extract_if_dead(struct connectdata *conn,
 
     }
     else {
-      dead = !Curl_conn_is_alive(data, conn);
+      bool input_pending;
+
+      dead = !Curl_conn_is_alive(data, conn, &input_pending);
+      if(input_pending) {
+        /* For reuse, we want a "clean" connection state. The includes
+         * that we expect - in general - no waiting input data. Input
+         * waiting might be a TLS Notify Close, for example. We reject
+         * that.
+         * For protocols where data from other other end may arrive at
+         * any time (HTTP/2 PING for example), the protocol handler needs
+         * to install its own `connection_check` callback.
+         */
+        dead = TRUE;
+      }
     }
 
     if(dead) {

lib/vquic/curl_msh3.c

@@ -769,11 +769,13 @@ static CURLcode cf_msh3_query(struct Curl_cfilter *cf,
 }
 
 static bool cf_msh3_conn_is_alive(struct Curl_cfilter *cf,
-                                  struct Curl_easy *data)
+                                  struct Curl_easy *data,
+                                  bool *input_pending)
 {
   struct cf_msh3_ctx *ctx = cf->ctx;
 
   (void)data;
+  *input_pending = FALSE;
   return ctx && ctx->sock[SP_LOCAL] != CURL_SOCKET_BAD && ctx->qconn &&
          ctx->connected;
 }

lib/vquic/curl_ngtcp2.c

@@ -2444,6 +2444,32 @@ static CURLcode cf_ngtcp2_query(struct Curl_cfilter *cf,
     CURLE_UNKNOWN_OPTION;
 }
 
+static bool cf_ngtcp2_conn_is_alive(struct Curl_cfilter *cf,
+                                    struct Curl_easy *data,
+                                    bool *input_pending)
+{
+  bool alive = TRUE;
+
+  *input_pending = FALSE;
+  if(!cf->next || !cf->next->cft->is_alive(cf->next, data, input_pending))
+    return FALSE;
+
+  if(*input_pending) {
+    /* This happens before we've sent off a request and the connection is
+       not in use by any other transfer, there shouldn't be any data here,
+       only "protocol frames" */
+    *input_pending = FALSE;
+    Curl_attach_connection(data, cf->conn);
+    if(cf_process_ingress(cf, data))
+      alive = FALSE;
+    else {
+      alive = TRUE;
+    }
+    Curl_detach_connection(data);
+  }
+
+  return alive;
+}
 
 struct Curl_cftype Curl_cft_http3 = {
   "HTTP/3",
@@ -2458,7 +2484,7 @@ struct Curl_cftype Curl_cft_http3 = {
   cf_ngtcp2_send,
   cf_ngtcp2_recv,
   cf_ngtcp2_data_event,
-  Curl_cf_def_conn_is_alive,
+  cf_ngtcp2_conn_is_alive,
   Curl_cf_def_conn_keep_alive,
   cf_ngtcp2_query,
 };

lib/vquic/curl_quiche.c

@@ -1359,6 +1359,32 @@ static CURLcode cf_quiche_query(struct Curl_cfilter *cf,
     CURLE_UNKNOWN_OPTION;
 }
 
+static bool cf_quiche_conn_is_alive(struct Curl_cfilter *cf,
+                                    struct Curl_easy *data,
+                                    bool *input_pending)
+{
+  bool alive = TRUE;
+
+  *input_pending = FALSE;
+  if(!cf->next || !cf->next->cft->is_alive(cf->next, data, input_pending))
+    return FALSE;
+
+  if(*input_pending) {
+    /* This happens before we've sent off a request and the connection is
+       not in use by any other transfer, there shouldn't be any data here,
+       only "protocol frames" */
+    *input_pending = FALSE;
+    Curl_attach_connection(data, cf->conn);
+    if(cf_process_ingress(cf, data))
+      alive = FALSE;
+    else {
+      alive = TRUE;
+    }
+    Curl_detach_connection(data);
+  }
+
+  return alive;
+}
 
 struct Curl_cftype Curl_cft_http3 = {
   "HTTP/3",
@@ -1373,7 +1399,7 @@ struct Curl_cftype Curl_cft_http3 = {
   cf_quiche_send,
   cf_quiche_recv,
   cf_quiche_data_event,
-  Curl_cf_def_conn_is_alive,
+  cf_quiche_conn_is_alive,
   Curl_cf_def_conn_keep_alive,
   cf_quiche_query,
 };