From: Stefan Eissing Date: Mon, 15 Apr 2024 12:34:32 +0000 (+0200) Subject: http3: extend download abort tests, fixes in ngtcp2 X-Git-Tag: curl-8_8_0~211 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=08d10d2a0003d796467a17fce0d7857809b31d63;p=thirdparty%2Fcurl.git http3: extend download abort tests, fixes in ngtcp2 - fix flow handling in ngtcp2 to ACK data on streams we abort ourself. - extend test_02_23* cases to also run for h3 - skip test_02_23* for OpenSSL QUIC as it gets stalled on progressing the connection Closes #13374 --- diff --git a/lib/vquic/curl_ngtcp2.c b/lib/vquic/curl_ngtcp2.c index 74006b4727..e25ae258c1 100644 --- a/lib/vquic/curl_ngtcp2.c +++ b/lib/vquic/curl_ngtcp2.c @@ -393,6 +393,7 @@ static int cb_recv_stream_data(ngtcp2_conn *tconn, uint32_t flags, nghttp3_ssize nconsumed; int fin = (flags & NGTCP2_STREAM_DATA_FLAG_FIN) ? 1 : 0; struct Curl_easy *data = stream_user_data; + struct h3_stream_ctx *stream = H3_STREAM_CTX(data); (void)offset; (void)data; @@ -401,10 +402,14 @@ static int cb_recv_stream_data(ngtcp2_conn *tconn, uint32_t flags, CURL_TRC_CF(data, cf, "[%" CURL_PRId64 "] read_stream(len=%zu) -> %zd", stream_id, buflen, nconsumed); if(nconsumed < 0) { - if(!data) { + /* consume all bytes */ + ngtcp2_conn_extend_max_stream_offset(tconn, stream_id, buflen); + ngtcp2_conn_extend_max_offset(tconn, buflen); + if(!data || (stream && stream->reset) || + NGHTTP3_ERR_H3_STREAM_CREATION_ERROR == (int)nconsumed) { struct Curl_easy *cdata = CF_DATA_CURRENT(cf); - CURL_TRC_CF(cdata, cf, "[%" CURL_PRId64 "] nghttp3 error on stream not " - "used by us, ignored", stream_id); + CURL_TRC_CF(cdata, cf, "[%" CURL_PRId64 "] discard data for stream %s", + stream_id, (data && stream)? "reset" : "unknown"); return 0; } ngtcp2_ccerr_set_application_error( @@ -786,7 +791,11 @@ static int cb_h3_recv_data(nghttp3_conn *conn, int64_t stream3_id, if(result) { CURL_TRC_CF(data, cf, "[%" CURL_PRId64 "] DATA len=%zu, ERROR %d", stream->id, blen, result); - return NGHTTP3_ERR_CALLBACK_FAILURE; + nghttp3_conn_close_stream(ctx->h3conn, stream->id, + NGHTTP3_H3_REQUEST_CANCELLED); + ngtcp2_conn_extend_max_stream_offset(ctx->qconn, stream->id, blen); + ngtcp2_conn_extend_max_offset(ctx->qconn, blen); + return 0; } if(blen) { CURL_TRC_CF(data, cf, "[%" CURL_PRId64 "] ACK %zu bytes of DATA", @@ -1288,6 +1297,7 @@ static ssize_t h3_stream_open(struct Curl_cfilter *cf, if(rc) { failf(data, "can get bidi streams"); *err = CURLE_SEND_ERROR; + nwritten = -1; goto out; } stream->id = (curl_int64_t)sid; diff --git a/tests/http/clients/h2-download.c b/tests/http/clients/h2-download.c index 606eda3d5d..4ae283d2c3 100644 --- a/tests/http/clients/h2-download.c +++ b/tests/http/clients/h2-download.c @@ -44,41 +44,101 @@ static int verbose = 1; -static -int my_trace(CURL *handle, curl_infotype type, - char *data, size_t size, - void *userp) +static void log_line_start(FILE *log, const char *idsbuf, curl_infotype type) { - const char *text; - (void)handle; /* prevent compiler warning */ - (void)userp; + /* + * This is the trace look that is similar to what libcurl makes on its + * own. + */ + static const char * const s_infotype[] = { + "* ", "< ", "> ", "{ ", "} ", "{ ", "} " + }; + if(idsbuf && *idsbuf) + fprintf(log, "%s%s", idsbuf, s_infotype[type]); + else + fputs(s_infotype[type], log); +} + +#define TRC_IDS_FORMAT_IDS_1 "[%" CURL_FORMAT_CURL_OFF_T "-x] " +#define TRC_IDS_FORMAT_IDS_2 "[%" CURL_FORMAT_CURL_OFF_T "-%" \ + CURL_FORMAT_CURL_OFF_T "] " +/* +** callback for CURLOPT_DEBUGFUNCTION +*/ +static int debug_cb(CURL *handle, curl_infotype type, + char *data, size_t size, + void *userdata) +{ + FILE *output = stderr; + static int newl = 0; + static int traced_data = 0; + char idsbuf[60]; + curl_off_t xfer_id, conn_id; + + (void)handle; /* not used */ + (void)userdata; + + if(!curl_easy_getinfo(handle, CURLINFO_XFER_ID, &xfer_id) && xfer_id >= 0) { + if(!curl_easy_getinfo(handle, CURLINFO_CONN_ID, &conn_id) && + conn_id >= 0) { + curl_msnprintf(idsbuf, sizeof(idsbuf), TRC_IDS_FORMAT_IDS_2, + xfer_id, conn_id); + } + else { + curl_msnprintf(idsbuf, sizeof(idsbuf), TRC_IDS_FORMAT_IDS_1, xfer_id); + } + } + else + idsbuf[0] = 0; switch(type) { - case CURLINFO_TEXT: - fprintf(stderr, "== Info: %s", data); - return 0; case CURLINFO_HEADER_OUT: - text = "=> Send header"; - break; - case CURLINFO_DATA_OUT: - if(verbose <= 1) - return 0; - text = "=> Send data"; + if(size > 0) { + size_t st = 0; + size_t i; + for(i = 0; i < size - 1; i++) { + if(data[i] == '\n') { /* LF */ + if(!newl) { + log_line_start(output, idsbuf, type); + } + (void)fwrite(data + st, i - st + 1, 1, output); + st = i + 1; + newl = 0; + } + } + if(!newl) + log_line_start(output, idsbuf, type); + (void)fwrite(data + st, i - st + 1, 1, output); + } + newl = (size && (data[size - 1] != '\n')) ? 1 : 0; + traced_data = 0; break; + case CURLINFO_TEXT: case CURLINFO_HEADER_IN: - text = "<= Recv header"; + if(!newl) + log_line_start(output, idsbuf, type); + (void)fwrite(data, size, 1, output); + newl = (size && (data[size - 1] != '\n')) ? 1 : 0; + traced_data = 0; break; + case CURLINFO_DATA_OUT: case CURLINFO_DATA_IN: - if(verbose <= 1) - return 0; - text = "<= Recv data"; + case CURLINFO_SSL_DATA_IN: + case CURLINFO_SSL_DATA_OUT: + if(!traced_data) { + if(!newl) + log_line_start(output, idsbuf, type); + fprintf(output, "[%ld bytes data]\n", (long)size); + newl = 0; + traced_data = 1; + } + break; + default: /* nada */ + newl = 0; + traced_data = 1; break; - default: /* in case a new one is introduced to shock us */ - return 0; } - fprintf(stderr, "%s, %lu bytes (0x%lx)\n", - text, (unsigned long)size, (unsigned long)size); return 0; } @@ -183,7 +243,7 @@ static int setup(CURL *hnd, const char *url, struct transfer *t, /* please be verbose */ if(verbose) { curl_easy_setopt(hnd, CURLOPT_VERBOSE, 1L); - curl_easy_setopt(hnd, CURLOPT_DEBUGFUNCTION, my_trace); + curl_easy_setopt(hnd, CURLOPT_DEBUGFUNCTION, debug_cb); } #if (CURLPIPE_MULTIPLEX > 0) @@ -272,6 +332,9 @@ int main(int argc, char *argv[]) argc -= optind; argv += optind; + curl_global_init(CURL_GLOBAL_DEFAULT); + curl_global_trace("ids,time,http/2,http/3"); + if(argc != 1) { usage("not enough arguments"); return 2; diff --git a/tests/http/test_02_download.py b/tests/http/test_02_download.py index e0010a96ff..95a30e114b 100644 --- a/tests/http/test_02_download.py +++ b/tests/http/test_02_download.py @@ -328,9 +328,11 @@ class TestDownload: self.check_downloads(client, srcfile, count) # download, several at a time, pause and abort paused - @pytest.mark.parametrize("proto", ['http/1.1', 'h2']) + @pytest.mark.parametrize("proto", ['http/1.1', 'h2', 'h3']) def test_02_23a_lib_abort_paused(self, env: Env, httpd, nghttpx, proto, repeat): - if proto == 'h2': + if proto == 'h3' and env.curl_uses_ossl_quic(): + pytest.skip('OpenSSL QUIC fails here') + if proto in ['h2', 'h3']: count = 200 max_parallel = 100 pause_offset = 64 * 1024 @@ -353,9 +355,11 @@ class TestDownload: self.check_downloads(client, srcfile, count, complete=False) # download, several at a time, abort after n bytes - @pytest.mark.parametrize("proto", ['http/1.1', 'h2']) + @pytest.mark.parametrize("proto", ['http/1.1', 'h2', 'h3']) def test_02_23b_lib_abort_offset(self, env: Env, httpd, nghttpx, proto, repeat): - if proto == 'h2': + if proto == 'h3' and env.curl_uses_ossl_quic(): + pytest.skip('OpenSSL QUIC fails here') + if proto in ['h2', 'h3']: count = 200 max_parallel = 100 abort_offset = 64 * 1024 @@ -378,9 +382,11 @@ class TestDownload: self.check_downloads(client, srcfile, count, complete=False) # download, several at a time, abort after n bytes - @pytest.mark.parametrize("proto", ['http/1.1', 'h2']) + @pytest.mark.parametrize("proto", ['http/1.1', 'h2', 'h3']) def test_02_23c_lib_fail_offset(self, env: Env, httpd, nghttpx, proto, repeat): - if proto == 'h2': + if proto == 'h3' and env.curl_uses_ossl_quic(): + pytest.skip('OpenSSL QUIC fails here') + if proto in ['h2', 'h3']: count = 200 max_parallel = 100 fail_offset = 64 * 1024 diff --git a/tests/http/test_03_goaway.py b/tests/http/test_03_goaway.py index 1f4a3d7844..e0319b5595 100644 --- a/tests/http/test_03_goaway.py +++ b/tests/http/test_03_goaway.py @@ -85,6 +85,8 @@ class TestGoAway: pytest.skip("msh3 stalls here") if proto == 'h3' and env.curl_uses_lib('quiche'): pytest.skip("does not work in CI, but locally for some reason") + if proto == 'h3' and env.curl_uses_ossl_quic(): + pytest.skip('OpenSSL QUIC fails here') count = 3 self.r = None def long_run(): diff --git a/tests/http/testenv/env.py b/tests/http/testenv/env.py index 13c5d6bd46..e8c5872daf 100644 --- a/tests/http/testenv/env.py +++ b/tests/http/testenv/env.py @@ -260,6 +260,12 @@ class Env: def curl_uses_lib(libname: str) -> bool: return libname.lower() in Env.CONFIG.curl_props['libs'] + @staticmethod + def curl_uses_ossl_quic() -> bool: + if Env.have_h3_curl(): + return not Env.curl_uses_lib('ngtcp2') and Env.curl_uses_lib('nghttp3') + return False + @staticmethod def curl_has_feature(feature: str) -> bool: return feature.lower() in Env.CONFIG.curl_props['features']