http2: h2 and h2-PROXY connection alive check fixes

- fix HTTP/2 check to not declare a connection dead when
  the read attempt results in EAGAIN
- add H2-PROXY alive check as for HTTP/2 that was missing
  and is needed
- add attach/detach around Curl_conn_is_alive() and remove
  these in filter methods
- add checks for number of connections used in some test_10
  proxy tunneling tests

Closes #11368

diff --git a/lib/cf-h2-proxy.c b/lib/cf-h2-proxy.c
index aa6b144df20fdc09f3ba589197ba82097f7c73e4..f6acfc520255b9362d8fa22bcfd3a01d56c725b7 100644 (file)
--- a/lib/cf-h2-proxy.c
+++ b/lib/cf-h2-proxy.c
@@ -221,6 +221,24 @@ static void cf_h2_proxy_ctx_free(struct cf_h2_proxy_ctx *ctx)
   }
 }
 
+static void drain_tunnel(struct Curl_cfilter *cf,
+                         struct Curl_easy *data,
+                         struct tunnel_stream *tunnel)
+{
+  unsigned char bits;
+
+  (void)cf;
+  bits = CURL_CSELECT_IN;
+  if(!tunnel->closed && !tunnel->reset && tunnel->upload_blocked_len)
+    bits |= CURL_CSELECT_OUT;
+  if(data->state.dselect_bits != bits) {
+    DEBUGF(LOG_CF(data, cf, "[h2sid=%d] DRAIN dselect_bits=%x",
+                  tunnel->stream_id, bits));
+    data->state.dselect_bits = bits;
+    Curl_expire(data, 0, EXPIRE_RUN_NOW);
+  }
+}
+
 static ssize_t proxy_nw_in_reader(void *reader_ctx,
                                   unsigned char *buf, size_t buflen,
                                   CURLcode *err)
@@ -230,7 +248,7 @@ static ssize_t proxy_nw_in_reader(void *reader_ctx,
   ssize_t nread;
 
   nread = Curl_conn_cf_recv(cf->next, data, (char *)buf, buflen, err);
-  DEBUGF(LOG_CF(data, cf, "nw_in recv(len=%zu) -> %zd, %d",
+  DEBUGF(LOG_CF(data, cf, "nw_in_reader(len=%zu) -> %zd, %d",
          buflen, nread, *err));
   return nread;
 }
@@ -244,7 +262,8 @@ static ssize_t proxy_h2_nw_out_writer(void *writer_ctx,
   ssize_t nwritten;
 
   nwritten = Curl_conn_cf_send(cf->next, data, (const char *)buf, buflen, err);
-  DEBUGF(LOG_CF(data, cf, "nw_out send(len=%zu) -> %zd", buflen, nwritten));
+  DEBUGF(LOG_CF(data, cf, "nw_out_writer(len=%zu) -> %zd, %d",
+                buflen, nwritten, *err));
   return nwritten;
 }
 
@@ -437,14 +456,6 @@ static int proxy_h2_process_pending_input(struct Curl_cfilter *cf,
     }
   }
 
-  if(nghttp2_session_check_request_allowed(ctx->h2) == 0) {
-    /* No more requests are allowed in the current session, so
-       the connection may not be reused. This is set when a
-       GOAWAY frame has been received or when the limit of stream
-       identifiers has been reached. */
-    connclose(cf->conn, "http/2: No new requests allowed");
-  }
-
   return 0;
 }
 
@@ -1230,6 +1241,12 @@ static ssize_t cf_h2_proxy_recv(struct Curl_cfilter *cf,
   }
 
 out:
+  if(!Curl_bufq_is_empty(&ctx->tunnel.recvbuf) &&
+     (nread >= 0 || *err == CURLE_AGAIN)) {
+    /* data pending and no fatal error to report. Need to trigger
+     * draining to avoid stalling when no socket events happen. */
+    drain_tunnel(cf, data, &ctx->tunnel);
+  }
   DEBUGF(LOG_CF(data, cf, "[h2sid=%u] cf_recv(len=%zu) -> %zd %d",
                 ctx->tunnel.stream_id, len, nread, *err));
   CF_DATA_RESTORE(cf, save);
@@ -1367,6 +1384,59 @@ out:
   return nwritten;
 }
 
+static bool proxy_h2_connisalive(struct Curl_cfilter *cf,
+                                 struct Curl_easy *data,
+                                 bool *input_pending)
+{
+  struct cf_h2_proxy_ctx *ctx = cf->ctx;
+  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" */
+    CURLcode result;
+    ssize_t nread = -1;
+
+    *input_pending = FALSE;
+    nread = Curl_bufq_slurp(&ctx->inbufq, proxy_nw_in_reader, cf, &result);
+    if(nread != -1) {
+      if(proxy_h2_process_pending_input(cf, data, &result) < 0)
+        /* immediate error, considered dead */
+        alive = FALSE;
+      else {
+        alive = !should_close_session(ctx);
+      }
+    }
+    else if(result != CURLE_AGAIN) {
+      /* the read failed so let's say this is dead anyway */
+      alive = FALSE;
+    }
+  }
+
+  return alive;
+}
+
+static bool cf_h2_proxy_is_alive(struct Curl_cfilter *cf,
+                                 struct Curl_easy *data,
+                                 bool *input_pending)
+{
+  struct cf_h2_proxy_ctx *ctx = cf->ctx;
+  CURLcode result;
+  struct cf_call_data save;
+
+  CF_DATA_SAVE(save, cf, data);
+  result = (ctx && ctx->h2 && proxy_h2_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;
+}
+
 struct Curl_cftype Curl_cft_h2_proxy = {
   "H2-PROXY",
   CF_TYPE_IP_CONNECT,
@@ -1380,7 +1450,7 @@ struct Curl_cftype Curl_cft_h2_proxy = {
   cf_h2_proxy_send,
   cf_h2_proxy_recv,
   Curl_cf_def_cntrl,
-  Curl_cf_def_conn_is_alive,
+  cf_h2_proxy_is_alive,
   Curl_cf_def_conn_keep_alive,
   Curl_cf_def_query,
 };

diff --git a/lib/http2.c b/lib/http2.c
index ed831f6a191e82f7dedb3e79dbd703ac8fa626e3..52ae8ce3df29624ad4d0d3061b203ec62e350785 100644 (file)
--- a/lib/http2.c
+++ b/lib/http2.c
@@ -588,7 +588,6 @@ static bool http2_connisalive(struct Curl_cfilter *cf, struct Curl_easy *data,
     ssize_t nread = -1;
 
     *input_pending = FALSE;
-    Curl_attach_connection(data, cf->conn);
     nread = Curl_bufq_slurp(&ctx->inbufq, nw_in_reader, cf, &result);
     if(nread != -1) {
       DEBUGF(LOG_CF(data, cf, "%zd bytes stray data read before trying "
@@ -600,11 +599,10 @@ static bool http2_connisalive(struct Curl_cfilter *cf, struct Curl_easy *data,
         alive = !should_close_session(ctx);
       }
     }
-    else {
+    else if(result != CURLE_AGAIN) {
       /* the read failed so let's say this is dead anyway */
       alive = FALSE;
     }
-    Curl_detach_connection(data);
   }
 
   return alive;

diff --git a/lib/url.c b/lib/url.c
index c939a1146083481e874b2dc08832ec7fdbf277ed..0d5e7d0caf99cada51f6b9280023a2f98dd49987 100644 (file)
--- a/lib/url.c
+++ b/lib/url.c
@@ -941,6 +941,7 @@ static bool extract_if_dead(struct connectdata *conn,
     else {
       bool input_pending;
 
+      Curl_attach_connection(data, conn);
       dead = !Curl_conn_is_alive(data, conn, &input_pending);
       if(input_pending) {
         /* For reuse, we want a "clean" connection state. The includes
@@ -953,6 +954,7 @@ static bool extract_if_dead(struct connectdata *conn,
          */
         dead = TRUE;
       }
+      Curl_detach_connection(data);
     }
 
     if(dead) {

diff --git a/lib/vquic/curl_ngtcp2.c b/lib/vquic/curl_ngtcp2.c
index 459e40bb8b93ed0124742d10d56f81cd37d6f606..34d16013d67ce984ccb881416316723895f8c3e6 100644 (file)
--- a/lib/vquic/curl_ngtcp2.c
+++ b/lib/vquic/curl_ngtcp2.c
@@ -2535,13 +2535,11 @@ static bool cf_ngtcp2_conn_is_alive(struct Curl_cfilter *cf,
        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, NULL))
       alive = FALSE;
     else {
       alive = TRUE;
     }
-    Curl_detach_connection(data);
   }
 
   return alive;

diff --git a/lib/vquic/curl_quiche.c b/lib/vquic/curl_quiche.c
index e373a12f8fcc325d1461361f4e44cbab3a092241..d906ebad866d53187ecfb299dfb0ada83e986509 100644 (file)
--- a/lib/vquic/curl_quiche.c
+++ b/lib/vquic/curl_quiche.c
@@ -1555,13 +1555,11 @@ static bool cf_quiche_conn_is_alive(struct Curl_cfilter *cf,
        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;

diff --git a/lib/vtls/vtls.c b/lib/vtls/vtls.c
index d905f02c99f31406ac87aaf75f311163fde75c79..fc35697a5352f655ba79aba6ffba5d7fbe9ce7fa 100644 (file)
--- a/lib/vtls/vtls.c
+++ b/lib/vtls/vtls.c
@@ -1592,6 +1592,7 @@ static ssize_t ssl_cf_recv(struct Curl_cfilter *cf,
   ssize_t nread;
 
   CF_DATA_SAVE(save, cf, data);
+  *err = CURLE_OK;
   nread = Curl_ssl->recv_plain(cf, data, buf, len, err);
   if(nread > 0) {
     DEBUGASSERT((size_t)nread <= len);

diff --git a/tests/http/test_10_proxy.py b/tests/http/test_10_proxy.py
index 116ccb7d1019cfe540e8396db83bce4051a39e44..9cbf35d9663f8b236354ffdc81470d6375780e7d 100644 (file)
--- a/tests/http/test_10_proxy.py
+++ b/tests/http/test_10_proxy.py
@@ -194,6 +194,7 @@ class TestProxy:
         for i in range(count):
             dfile = curl.download_file(i)
             assert filecmp.cmp(srcfile, dfile, shallow=False)
+        assert r.total_connects == 1, r.dump_logs()
 
     # upload many https: with proto via https: proxytunnel
     @pytest.mark.skipif(condition=not Env.have_ssl_curl(), reason=f"curl without SSL")
@@ -223,6 +224,7 @@ class TestProxy:
         for i in range(count):
             respdata = open(curl.response_file(i)).readlines()
             assert respdata == indata
+        assert r.total_connects == 1, r.dump_logs()
 
     @pytest.mark.skipif(condition=not Env.have_ssl_curl(), reason=f"curl without SSL")
     @pytest.mark.parametrize("tunnel", ['http/1.1', 'h2'])

diff --git a/tests/http/testenv/curl.py b/tests/http/testenv/curl.py
index cabf1c560dbe04839e1e5edeae54b6e237317998..54622b5d85b73b710ebe200346e023d1d6e4384a 100644 (file)
--- a/tests/http/testenv/curl.py
+++ b/tests/http/testenv/curl.py
@@ -483,15 +483,15 @@ class CurlClient:
         if not isinstance(urls, list):
             urls = [urls]
 
-        args = [self._curl, "-s", "--path-as-is"]
+        args = [self._curl, "-s", "--path-as-is", '--trace-time', '--trace-ids']
         if with_headers:
             args.extend(["-D", self._headerfile])
         if with_trace or self.env.verbose > 2:
-            args.extend(['--trace', self._tracefile, '--trace-time'])
-        elif self.env.verbose > 1:
             args.extend(['--trace', self._tracefile])
+        elif self.env.verbose > 1:
+            args.extend(['--trace-ascii', self._tracefile])
         elif not self._silent:
-            args.extend(['-v', '--trace-time', '--trace-ids'])
+            args.extend(['-v'])
 
         for url in urls:
             u = urlparse(urls[0])