mirror of
https://github.com/curl/curl.git
synced 2026-08-25 08:33:41 +03:00
http2 + ngtcp2: pass CURLcode errors from callbacks
- errors returned by Curl_xfer_write_resp() and the header variant are not errors in the protocol. The result needs to be returned on the next recv() from the protocol filter. - make xfer write errors for response data cause the stream to be cancelled - added pytest test_02_14 and test_02_15 to verify that also for parallel processing Reported-by: Laramie Leavitt Fixes #13411 Closes #13424
This commit is contained in:
Stefan Eissing
Daniel Stenberg
parent
926fb00405
commit
5c59f91427
3 changed files with 125 additions and 40 deletions
64
lib/http2.c
64
lib/http2.c
|
|
@ -193,6 +193,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 */
|
||||
|
|
@ -975,6 +976,41 @@ fail:
|
|||
return rv;
|
||||
}
|
||||
|
||||
static void h2_xfer_write_resp_hd(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_hd(data, 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, 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,
|
||||
struct Curl_easy *data,
|
||||
const nghttp2_frame *frame)
|
||||
|
|
@ -982,7 +1018,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(ctx, data);
|
||||
int32_t stream_id = frame->hd.stream_id;
|
||||
CURLcode result;
|
||||
int rv;
|
||||
|
||||
if(!stream) {
|
||||
|
|
@ -1030,9 +1065,7 @@ static CURLcode on_stream_frame(struct Curl_cfilter *cf,
|
|||
stream->status_code = -1;
|
||||
}
|
||||
|
||||
result = Curl_xfer_write_resp_hd(data, STRCONST("\r\n"), stream->closed);
|
||||
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;
|
||||
|
|
@ -1251,7 +1284,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 */
|
||||
|
|
@ -1274,12 +1306,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) {
|
||||
nghttp2_submit_rst_stream(ctx->h2, 0, stream->id,
|
||||
NGHTTP2_ERR_CALLBACK_FAILURE);
|
||||
return 0;
|
||||
}
|
||||
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;
|
||||
|
|
@ -1500,8 +1527,8 @@ static int on_header(nghttp2_session *session, const nghttp2_frame *frame,
|
|||
if(!result)
|
||||
result = Curl_dyn_addn(&ctx->scratch, STRCONST(" \r\n"));
|
||||
if(!result)
|
||||
result = Curl_xfer_write_resp_hd(data_s, Curl_dyn_ptr(&ctx->scratch),
|
||||
Curl_dyn_len(&ctx->scratch), FALSE);
|
||||
h2_xfer_write_resp_hd(cf, data_s, stream, Curl_dyn_ptr(&ctx->scratch),
|
||||
Curl_dyn_len(&ctx->scratch), FALSE);
|
||||
if(result)
|
||||
return NGHTTP2_ERR_CALLBACK_FAILURE;
|
||||
/* if we receive data for another handle, wake that up */
|
||||
|
|
@ -1525,8 +1552,8 @@ static int on_header(nghttp2_session *session, const nghttp2_frame *frame,
|
|||
if(!result)
|
||||
result = Curl_dyn_addn(&ctx->scratch, STRCONST("\r\n"));
|
||||
if(!result)
|
||||
result = Curl_xfer_write_resp_hd(data_s, Curl_dyn_ptr(&ctx->scratch),
|
||||
Curl_dyn_len(&ctx->scratch), FALSE);
|
||||
h2_xfer_write_resp_hd(cf, data_s, stream, Curl_dyn_ptr(&ctx->scratch),
|
||||
Curl_dyn_len(&ctx->scratch), FALSE);
|
||||
if(result)
|
||||
return NGHTTP2_ERR_CALLBACK_FAILURE;
|
||||
/* if we receive data for another handle, wake that up */
|
||||
|
|
@ -1831,7 +1858,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);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -155,6 +155,7 @@ struct h3_stream_ctx {
|
|||
curl_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 */
|
||||
|
|
@ -795,6 +796,41 @@ static int cb_h3_stream_close(nghttp3_conn *conn, int64_t sid,
|
|||
return 0;
|
||||
}
|
||||
|
||||
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)
|
||||
{
|
||||
|
||||
/* If we already encountered an error, skip further writes */
|
||||
if(!stream->xfer_result) {
|
||||
stream->xfer_result = Curl_xfer_write_resp_hd(data, buf, blen, eos);
|
||||
if(stream->xfer_result)
|
||||
CURL_TRC_CF(data, cf, "[%"CURL_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, 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, "[%"CURL_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,
|
||||
const uint8_t *buf, size_t blen,
|
||||
void *user_data, void *stream_user_data)
|
||||
|
|
@ -803,7 +839,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(ctx, data);
|
||||
CURLcode result;
|
||||
|
||||
(void)conn;
|
||||
(void)stream3_id;
|
||||
|
|
@ -811,16 +846,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, "[%" CURL_PRId64 "] DATA len=%zu, ERROR %d",
|
||||
stream->id, blen, result);
|
||||
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;
|
||||
}
|
||||
h3_xfer_write_resp(cf, data, stream, (char *)buf, blen, FALSE);
|
||||
if(blen) {
|
||||
CURL_TRC_CF(data, cf, "[%" CURL_PRId64 "] ACK %zu bytes of DATA",
|
||||
stream->id, blen);
|
||||
|
|
@ -855,7 +881,6 @@ static int cb_h3_end_headers(nghttp3_conn *conn, int64_t sid,
|
|||
struct Curl_easy *data = stream_user_data;
|
||||
curl_int64_t stream_id = (curl_int64_t)sid;
|
||||
struct h3_stream_ctx *stream = H3_STREAM_CTX(ctx, data);
|
||||
CURLcode result = CURLE_OK;
|
||||
(void)conn;
|
||||
(void)stream_id;
|
||||
(void)fin;
|
||||
|
|
@ -864,10 +889,7 @@ static int cb_h3_end_headers(nghttp3_conn *conn, int64_t sid,
|
|||
if(!stream)
|
||||
return 0;
|
||||
/* add a CRLF only if we've received some headers */
|
||||
result = Curl_xfer_write_resp_hd(data, STRCONST("\r\n"), stream->closed);
|
||||
if(result) {
|
||||
return -1;
|
||||
}
|
||||
h3_xfer_write_resp_hd(cf, data, stream, STRCONST("\r\n"), stream->closed);
|
||||
|
||||
CURL_TRC_CF(data, cf, "[%" CURL_PRId64 "] end_headers, status=%d",
|
||||
stream_id, stream->status_code);
|
||||
|
|
@ -915,8 +937,8 @@ static int cb_h3_recv_header(nghttp3_conn *conn, int64_t sid,
|
|||
if(!result)
|
||||
result = Curl_dyn_addn(&ctx->scratch, STRCONST(" \r\n"));
|
||||
if(!result)
|
||||
result = Curl_xfer_write_resp_hd(data, Curl_dyn_ptr(&ctx->scratch),
|
||||
Curl_dyn_len(&ctx->scratch), FALSE);
|
||||
h3_xfer_write_resp_hd(cf, data, stream, Curl_dyn_ptr(&ctx->scratch),
|
||||
Curl_dyn_len(&ctx->scratch), FALSE);
|
||||
CURL_TRC_CF(data, cf, "[%" CURL_PRId64 "] status: %s",
|
||||
stream_id, Curl_dyn_ptr(&ctx->scratch));
|
||||
if(result) {
|
||||
|
|
@ -939,11 +961,8 @@ static int cb_h3_recv_header(nghttp3_conn *conn, int64_t sid,
|
|||
if(!result)
|
||||
result = Curl_dyn_addn(&ctx->scratch, STRCONST("\r\n"));
|
||||
if(!result)
|
||||
result = Curl_xfer_write_resp_hd(data, Curl_dyn_ptr(&ctx->scratch),
|
||||
Curl_dyn_len(&ctx->scratch), FALSE);
|
||||
if(result) {
|
||||
return -1;
|
||||
}
|
||||
h3_xfer_write_resp_hd(cf, data, stream, Curl_dyn_ptr(&ctx->scratch),
|
||||
Curl_dyn_len(&ctx->scratch), FALSE);
|
||||
}
|
||||
return 0;
|
||||
}
|
||||
|
|
@ -1128,7 +1147,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, "[%" CURL_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;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue