diff --git a/lib/cf-h2-proxy.c b/lib/cf-h2-proxy.c index aa6b144df..f6acfc520 100644 --- 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 ed831f6a1..52ae8ce3d 100644 --- 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 c939a1146..0d5e7d0ca 100644 --- 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 459e40bb8..34d16013d 100644 --- 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 e373a12f8..d906ebad8 100644 --- 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 d905f02c9..fc35697a5 100644 --- 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 116ccb7d1..9cbf35d96 100644 --- 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 cabf1c560..54622b5d8 100644 --- 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])