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

diff --git a/lib/http2.c b/lib/http2.c
index 1ee57a4320ac3e0f20b9d0c8edcf7913cc66bea4..50dd878bb02e1de508dd77b95afb409f99f9bf52 100644 (file)
--- a/lib/http2.c
+++ b/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);
   }

diff --git a/lib/vquic/curl_ngtcp2.c b/lib/vquic/curl_ngtcp2.c
index 50fe7af701ac3956eac679ef48851a24308109ff..5171125f2fc4f3f7167a8b8c8a1ddaa120181fa7 100644 (file)
--- a/lib/vquic/curl_ngtcp2.c
+++ b/lib/vquic/curl_ngtcp2.c
@@ -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;
   }

diff --git a/tests/http/test_02_download.py b/tests/http/test_02_download.py
index 95a30e114b27673fc8f9998d285e4a61a7919d40..74f2e1e20fec7e8da7ce2b68b5517eee02f28536 100644 (file)
--- 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):