Created
April 26, 2024 08:49
-
-
Save icing/873a18d24230cf1ee18963305e167c09 to your computer and use it in GitHub Desktop.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
diff --git a/lib/http2.c b/lib/http2.c | |
index 99d7f3b0e..5eca075d7 100644 | |
--- a/lib/http2.c | |
+++ b/lib/http2.c | |
@@ -187,6 +187,7 @@ struct h2_stream_ctx { | |
int status_code; /* HTTP response status code */ | |
uint32_t error; /* stream error code */ | |
+ CURLcode xfer_result; /* Result of writing out response */ | |
uint32_t local_window_size; /* the local recv window size */ | |
int32_t id; /* HTTP/2 protocol identifier for stream */ | |
BIT(resp_hds_complete); /* we have a complete, final response */ | |
@@ -945,12 +946,39 @@ fail: | |
return rv; | |
} | |
-static CURLcode recvbuf_write_hds(struct Curl_cfilter *cf, | |
+static void h2_xfer_write_resp_hd(struct Curl_cfilter *cf, | |
struct Curl_easy *data, | |
- const char *buf, size_t blen) | |
+ struct h2_stream_ctx *stream, | |
+ const char *buf, size_t blen, bool eos) | |
{ | |
- (void)cf; | |
- return Curl_xfer_write_resp(data, (char *)buf, blen, FALSE); | |
+ | |
+ /* If we already encountered an error, skip further writes */ | |
+ if(!stream->xfer_result) { | |
+ stream->xfer_result = Curl_xfer_write_resp(data, (char *)buf, blen, eos); | |
+ if(stream->xfer_result) | |
+ CURL_TRC_CF(data, cf, "[%d] error %d writing %zu bytes of headers", | |
+ stream->id, stream->xfer_result, blen); | |
+ } | |
+} | |
+ | |
+static void h2_xfer_write_resp(struct Curl_cfilter *cf, | |
+ struct Curl_easy *data, | |
+ struct h2_stream_ctx *stream, | |
+ const char *buf, size_t blen, bool eos) | |
+{ | |
+ | |
+ /* If we already encountered an error, skip further writes */ | |
+ if(!stream->xfer_result) | |
+ stream->xfer_result = Curl_xfer_write_resp(data, (char *)buf, blen, eos); | |
+ /* If the transfer write is errored, we do not want any more data */ | |
+ if(stream->xfer_result) { | |
+ struct cf_h2_ctx *ctx = cf->ctx; | |
+ CURL_TRC_CF(data, cf, "[%d] error %d writing %zu bytes of data, " | |
+ "RST-ing stream", | |
+ stream->id, stream->xfer_result, blen); | |
+ nghttp2_submit_rst_stream(ctx->h2, 0, stream->id, | |
+ NGHTTP2_ERR_CALLBACK_FAILURE); | |
+ } | |
} | |
static CURLcode on_stream_frame(struct Curl_cfilter *cf, | |
@@ -960,7 +988,6 @@ static CURLcode on_stream_frame(struct Curl_cfilter *cf, | |
struct cf_h2_ctx *ctx = cf->ctx; | |
struct h2_stream_ctx *stream = H2_STREAM_CTX(data); | |
int32_t stream_id = frame->hd.stream_id; | |
- CURLcode result; | |
int rv; | |
if(!stream) { | |
@@ -1008,9 +1035,7 @@ static CURLcode on_stream_frame(struct Curl_cfilter *cf, | |
stream->status_code = -1; | |
} | |
- result = recvbuf_write_hds(cf, data, STRCONST("\r\n")); | |
- if(result) | |
- return result; | |
+ h2_xfer_write_resp_hd(cf, data, stream, STRCONST("\r\n"), stream->closed); | |
if(stream->status_code / 100 != 1) { | |
stream->resp_hds_complete = TRUE; | |
@@ -1229,7 +1254,6 @@ static int on_data_chunk_recv(nghttp2_session *session, uint8_t flags, | |
struct cf_h2_ctx *ctx = cf->ctx; | |
struct h2_stream_ctx *stream; | |
struct Curl_easy *data_s; | |
- CURLcode result; | |
(void)flags; | |
DEBUGASSERT(stream_id); /* should never be a zero stream ID here */ | |
@@ -1252,9 +1276,7 @@ static int on_data_chunk_recv(nghttp2_session *session, uint8_t flags, | |
if(!stream) | |
return NGHTTP2_ERR_CALLBACK_FAILURE; | |
- result = Curl_xfer_write_resp(data_s, (char *)mem, len, FALSE); | |
- if(result && result != CURLE_AGAIN) | |
- return NGHTTP2_ERR_CALLBACK_FAILURE; | |
+ h2_xfer_write_resp(cf, data_s, stream, (char *)mem, len, FALSE); | |
nghttp2_session_consume(ctx->h2, stream_id, len); | |
stream->nrcvd_data += (curl_off_t)len; | |
@@ -1465,16 +1487,12 @@ static int on_header(nghttp2_session *session, const nghttp2_frame *frame, | |
result = Curl_headers_push(data_s, buffer, CURLH_PSEUDO); | |
if(result) | |
return NGHTTP2_ERR_CALLBACK_FAILURE; | |
- result = recvbuf_write_hds(cf, data_s, STRCONST("HTTP/2 ")); | |
- if(result) | |
- return NGHTTP2_ERR_CALLBACK_FAILURE; | |
- result = recvbuf_write_hds(cf, data_s, (const char *)value, valuelen); | |
- if(result) | |
- return NGHTTP2_ERR_CALLBACK_FAILURE; | |
+ h2_xfer_write_resp_hd(cf, data_s, stream, STRCONST("HTTP/2 "), FALSE); | |
+ h2_xfer_write_resp_hd(cf, data_s, stream, | |
+ (const char *)value, valuelen, FALSE); | |
/* the space character after the status code is mandatory */ | |
- result = recvbuf_write_hds(cf, data_s, STRCONST(" \r\n")); | |
- if(result) | |
- return NGHTTP2_ERR_CALLBACK_FAILURE; | |
+ h2_xfer_write_resp_hd(cf, data_s, stream, STRCONST(" \r\n"), FALSE); | |
+ | |
/* if we receive data for another handle, wake that up */ | |
if(CF_DATA_CURRENT(cf) != data_s) | |
Curl_expire(data_s, 0, EXPIRE_RUN_NOW); | |
@@ -1487,18 +1505,13 @@ static int on_header(nghttp2_session *session, const nghttp2_frame *frame, | |
/* nghttp2 guarantees that namelen > 0, and :status was already | |
received, and this is not pseudo-header field . */ | |
/* convert to an HTTP1-style header */ | |
- result = recvbuf_write_hds(cf, data_s, (const char *)name, namelen); | |
- if(result) | |
- return NGHTTP2_ERR_CALLBACK_FAILURE; | |
- result = recvbuf_write_hds(cf, data_s, STRCONST(": ")); | |
- if(result) | |
- return NGHTTP2_ERR_CALLBACK_FAILURE; | |
- result = recvbuf_write_hds(cf, data_s, (const char *)value, valuelen); | |
- if(result) | |
- return NGHTTP2_ERR_CALLBACK_FAILURE; | |
- result = recvbuf_write_hds(cf, data_s, STRCONST("\r\n")); | |
- if(result) | |
- return NGHTTP2_ERR_CALLBACK_FAILURE; | |
+ h2_xfer_write_resp_hd(cf, data_s, stream, | |
+ (const char *)name, namelen, FALSE); | |
+ h2_xfer_write_resp_hd(cf, data_s, stream, STRCONST(": "), FALSE); | |
+ h2_xfer_write_resp_hd(cf, data_s, stream, | |
+ (const char *)value, valuelen, FALSE); | |
+ h2_xfer_write_resp_hd(cf, data_s, stream, STRCONST("\r\n"), FALSE); | |
+ | |
/* if we receive data for another handle, wake that up */ | |
if(CF_DATA_CURRENT(cf) != data_s) | |
Curl_expire(data_s, 0, EXPIRE_RUN_NOW); | |
@@ -1799,7 +1812,12 @@ static ssize_t stream_recv(struct Curl_cfilter *cf, struct Curl_easy *data, | |
(void)buf; | |
*err = CURLE_AGAIN; | |
- if(stream->closed) { | |
+ if(stream->xfer_result) { | |
+ CURL_TRC_CF(data, cf, "[%d] xfer write failed", stream->id); | |
+ *err = stream->xfer_result; | |
+ nread = -1; | |
+ } | |
+ else if(stream->closed) { | |
CURL_TRC_CF(data, cf, "[%d] returning CLOSE", stream->id); | |
nread = http2_handle_stream_close(cf, data, stream, err); | |
} | |
diff --git a/lib/vquic/curl_ngtcp2.c b/lib/vquic/curl_ngtcp2.c | |
index 6b6b8874c..ff3b4e742 100644 | |
--- a/lib/vquic/curl_ngtcp2.c | |
+++ b/lib/vquic/curl_ngtcp2.c | |
@@ -152,6 +152,7 @@ struct h3_stream_ctx { | |
uint64_t error3; /* HTTP/3 stream error code */ | |
curl_off_t upload_left; /* number of request bytes left to upload */ | |
int status_code; /* HTTP status code */ | |
+ CURLcode xfer_result; /* result from xfer_resp_write(_hd) */ | |
bool resp_hds_complete; /* we have a complete, final response */ | |
bool closed; /* TRUE on stream close */ | |
bool reset; /* TRUE on stream reset */ | |
@@ -759,10 +760,39 @@ static int cb_h3_stream_close(nghttp3_conn *conn, int64_t stream_id, | |
return 0; | |
} | |
-static CURLcode write_resp_hds(struct Curl_easy *data, | |
- const char *buf, size_t blen) | |
+static void h3_xfer_write_resp_hd(struct Curl_cfilter *cf, | |
+ struct Curl_easy *data, | |
+ struct h3_stream_ctx *stream, | |
+ const char *buf, size_t blen, bool eos) | |
{ | |
- return Curl_xfer_write_resp(data, (char *)buf, blen, FALSE); | |
+ | |
+ /* If we already encountered an error, skip further writes */ | |
+ if(!stream->xfer_result) { | |
+ stream->xfer_result = Curl_xfer_write_resp(data, (char *)buf, blen, eos); | |
+ if(stream->xfer_result) | |
+ CURL_TRC_CF(data, cf, "[%"PRId64"] error %d writing %zu " | |
+ "bytes of headers", stream->id, stream->xfer_result, blen); | |
+ } | |
+} | |
+ | |
+static void h3_xfer_write_resp(struct Curl_cfilter *cf, | |
+ struct Curl_easy *data, | |
+ struct h3_stream_ctx *stream, | |
+ const char *buf, size_t blen, bool eos) | |
+{ | |
+ | |
+ /* If we already encountered an error, skip further writes */ | |
+ if(!stream->xfer_result) | |
+ stream->xfer_result = Curl_xfer_write_resp(data, (char *)buf, blen, eos); | |
+ /* If the transfer write is errored, we do not want any more data */ | |
+ if(stream->xfer_result) { | |
+ struct cf_ngtcp2_ctx *ctx = cf->ctx; | |
+ CURL_TRC_CF(data, cf, "[%"PRId64"] error %d writing %zu bytes " | |
+ "of data, cancelling stream", | |
+ stream->id, stream->xfer_result, blen); | |
+ nghttp3_conn_close_stream(ctx->h3conn, stream->id, | |
+ NGHTTP3_H3_REQUEST_CANCELLED); | |
+ } | |
} | |
static int cb_h3_recv_data(nghttp3_conn *conn, int64_t stream3_id, | |
@@ -773,7 +803,6 @@ static int cb_h3_recv_data(nghttp3_conn *conn, int64_t stream3_id, | |
struct cf_ngtcp2_ctx *ctx = cf->ctx; | |
struct Curl_easy *data = stream_user_data; | |
struct h3_stream_ctx *stream = H3_STREAM_CTX(data); | |
- CURLcode result; | |
(void)conn; | |
(void)stream3_id; | |
@@ -781,12 +810,7 @@ static int cb_h3_recv_data(nghttp3_conn *conn, int64_t stream3_id, | |
if(!stream) | |
return NGHTTP3_ERR_CALLBACK_FAILURE; | |
- result = Curl_xfer_write_resp(data, (char *)buf, blen, FALSE); | |
- if(result) { | |
- CURL_TRC_CF(data, cf, "[%" PRId64 "] DATA len=%zu, ERROR receiving %d", | |
- stream->id, blen, result); | |
- return NGHTTP3_ERR_CALLBACK_FAILURE; | |
- } | |
+ h3_xfer_write_resp(cf, data, stream, (char *)buf, blen, FALSE); | |
if(blen) { | |
CURL_TRC_CF(data, cf, "[%" PRId64 "] ACK %zu bytes of DATA", | |
stream->id, blen); | |
@@ -819,7 +843,6 @@ static int cb_h3_end_headers(nghttp3_conn *conn, int64_t stream_id, | |
struct Curl_cfilter *cf = user_data; | |
struct Curl_easy *data = stream_user_data; | |
struct h3_stream_ctx *stream = H3_STREAM_CTX(data); | |
- CURLcode result = CURLE_OK; | |
(void)conn; | |
(void)stream_id; | |
(void)fin; | |
@@ -828,10 +851,7 @@ static int cb_h3_end_headers(nghttp3_conn *conn, int64_t stream_id, | |
if(!stream) | |
return 0; | |
/* add a CRLF only if we've received some headers */ | |
- result = write_resp_hds(data, "\r\n", 2); | |
- if(result) { | |
- return -1; | |
- } | |
+ h3_xfer_write_resp_hd(cf, data, stream, STRCONST("\r\n"), stream->closed); | |
CURL_TRC_CF(data, cf, "[%" PRId64 "] end_headers, status=%d", | |
stream_id, stream->status_code); | |
@@ -874,7 +894,7 @@ static int cb_h3_recv_header(nghttp3_conn *conn, int64_t stream_id, | |
ncopy = msnprintf(line, sizeof(line), "HTTP/3 %03d \r\n", | |
stream->status_code); | |
CURL_TRC_CF(data, cf, "[%" PRId64 "] status: %s", stream_id, line); | |
- result = write_resp_hds(data, line, ncopy); | |
+ h3_xfer_write_resp_hd(cf, data, stream, line, ncopy, FALSE); | |
if(result) { | |
return -1; | |
} | |
@@ -884,22 +904,12 @@ static int cb_h3_recv_header(nghttp3_conn *conn, int64_t stream_id, | |
CURL_TRC_CF(data, cf, "[%" PRId64 "] header: %.*s: %.*s", | |
stream_id, (int)h3name.len, h3name.base, | |
(int)h3val.len, h3val.base); | |
- result = write_resp_hds(data, (const char *)h3name.base, h3name.len); | |
- if(result) { | |
- return -1; | |
- } | |
- result = write_resp_hds(data, ": ", 2); | |
- if(result) { | |
- return -1; | |
- } | |
- result = write_resp_hds(data, (const char *)h3val.base, h3val.len); | |
- if(result) { | |
- return -1; | |
- } | |
- result = write_resp_hds(data, "\r\n", 2); | |
- if(result) { | |
- return -1; | |
- } | |
+ h3_xfer_write_resp_hd(cf, data, stream, | |
+ (const char *)h3name.base, h3name.len, FALSE); | |
+ h3_xfer_write_resp_hd(cf, data, stream, ": ", 2, FALSE); | |
+ h3_xfer_write_resp_hd(cf, data, stream, ( | |
+ const char *)h3val.base, h3val.len, FALSE); | |
+ h3_xfer_write_resp_hd(cf, data, stream, "\r\n", 2, FALSE); | |
} | |
return 0; | |
} | |
@@ -1083,7 +1093,13 @@ static ssize_t cf_ngtcp2_recv(struct Curl_cfilter *cf, struct Curl_easy *data, | |
goto out; | |
} | |
- if(stream->closed) { | |
+ if(stream->xfer_result) { | |
+ CURL_TRC_CF(data, cf, "[%" PRId64 "] xfer write failed", stream->id); | |
+ *err = stream->xfer_result; | |
+ nread = -1; | |
+ goto out; | |
+ } | |
+ else if(stream->closed) { | |
nread = recv_closed_stream(cf, data, stream, err); | |
goto out; | |
} | |
diff --git a/tests/http/test_02_download.py b/tests/http/test_02_download.py | |
index 4db9c9d36..4096d424d 100644 | |
--- a/tests/http/test_02_download.py | |
+++ b/tests/http/test_02_download.py | |
@@ -257,6 +257,34 @@ class TestDownload: | |
]) | |
r.check_response(count=count, http_status=200) | |
+ @pytest.mark.parametrize("proto", ['h2', 'h3']) | |
+ def test_02_14_not_found(self, env: Env, httpd, nghttpx, repeat, proto): | |
+ if proto == 'h3' and not env.have_h3(): | |
+ pytest.skip("h3 not supported") | |
+ if proto == 'h3' and env.curl_uses_lib('msh3'): | |
+ pytest.skip("msh3 stalls here") | |
+ count = 10 | |
+ urln = f'https://{env.authority_for(env.domain1, proto)}/not-found?[0-{count-1}]' | |
+ curl = CurlClient(env=env) | |
+ r = curl.http_download(urls=[urln], alpn_proto=proto, extra_args=[ | |
+ '--parallel' | |
+ ]) | |
+ r.check_stats(count=count, http_status=404, exitcode=0) | |
+ | |
+ @pytest.mark.parametrize("proto", ['h2', 'h3']) | |
+ def test_02_15_fail_not_found(self, env: Env, httpd, nghttpx, repeat, proto): | |
+ if proto == 'h3' and not env.have_h3(): | |
+ pytest.skip("h3 not supported") | |
+ if proto == 'h3' and env.curl_uses_lib('msh3'): | |
+ pytest.skip("msh3 stalls here") | |
+ count = 10 | |
+ urln = f'https://{env.authority_for(env.domain1, proto)}/not-found?[0-{count-1}]' | |
+ curl = CurlClient(env=env) | |
+ r = curl.http_download(urls=[urln], alpn_proto=proto, extra_args=[ | |
+ '--fail' | |
+ ]) | |
+ r.check_stats(count=count, http_status=404, exitcode=22) | |
+ | |
@pytest.mark.skipif(condition=Env().slow_network, reason="not suitable for slow network tests") | |
@pytest.mark.skipif(condition=Env().ci_run, reason="not suitable for CI runs") | |
def test_02_20_h2_small_frames(self, env: Env, httpd, repeat): |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment