From: Stefan Eissing <stefan@eissing.org>
Date: Thu, 25 May 2023 13:22:12 +0000 (+0200)
Subject: http3: send EOF indicator early as possible
X-Git-Tag: curl-8_1_2~9
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=c4bd61ddff150d77af302931a12ae00d6c279e4e;p=thirdparty%2Fcurl.git

http3: send EOF indicator early as possible

- ngtcp2 and quiche implementations relied on the DONE_SEND event
  to forward the EOF for uploads to the libraries. This often
  result in a last 0 length EOF data. Tracking the amount of
  data left to upload allows EOF indication earlier.
- refs #11205 where CloudFlare DoH servers did not like to
  receive the initial upload DATA without EOF and returned
  a 400 Bad Request

Reported-by: Sergey Fionov
Fixes #11205
Closes #11207
---

diff --git a/lib/vquic/curl_ngtcp2.c b/lib/vquic/curl_ngtcp2.c
index f59a253263..7627940ff5 100644
--- a/lib/vquic/curl_ngtcp2.c
+++ b/lib/vquic/curl_ngtcp2.c
@@ -178,11 +178,12 @@ struct stream_ctx {
   size_t sendbuf_len_in_flight; /* sendbuf amount "in flight" */
   size_t recv_buf_nonflow; /* buffered bytes, not counting for flow control */
   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 */
   bool resp_hds_complete; /* we have a complete, final response */
   bool closed; /* TRUE on stream close */
   bool reset;  /* TRUE on stream reset */
-  bool upload_done; /* stream is local closed */
+  bool send_closed; /* stream is local closed */
 };
 
 #define H3_STREAM_CTX(d)    ((struct stream_ctx *)(((d) && (d)->req.p.http)? \
@@ -998,7 +999,7 @@ static void drain_stream(struct Curl_cfilter *cf,
 
   (void)cf;
   bits = CURL_CSELECT_IN;
-  if(stream && !stream->upload_done)
+  if(stream && !stream->send_closed && stream->upload_left)
     bits |= CURL_CSELECT_OUT;
   if(data->state.dselect_bits != bits) {
     data->state.dselect_bits = bits;
@@ -1028,6 +1029,7 @@ static int cb_h3_stream_close(nghttp3_conn *conn, int64_t stream_id,
   stream->error3 = app_error_code;
   if(app_error_code == NGHTTP3_H3_INTERNAL_ERROR) {
     stream->reset = TRUE;
+    stream->send_closed = TRUE;
   }
   drain_stream(cf, data);
   return 0;
@@ -1423,7 +1425,6 @@ out:
   if(cf_flush_egress(cf, data)) {
     *err = CURLE_SEND_ERROR;
     nread = -1;
-    goto out;
   }
   DEBUGF(LOG_CF(data, cf, "[h3sid=%" PRId64 "] cf_recv(len=%zu) -> %zd, %d",
                 stream? stream->id : -1, len, nread, *err));
@@ -1512,11 +1513,14 @@ cb_h3_read_req_body(nghttp3_conn *conn, int64_t stream_id,
     DEBUGASSERT(nvecs > 0); /* we SHOULD have been be able to peek */
   }
 
+  if(nwritten > 0 && stream->upload_left != -1)
+    stream->upload_left -= nwritten;
+
   /* When we stopped sending and everything in `sendbuf` is "in flight",
    * we are at the end of the request body. */
-  if(stream->upload_done &&
-     stream->sendbuf_len_in_flight == Curl_bufq_len(&stream->sendbuf)) {
+  if(stream->upload_left == 0) {
     *pflags = NGHTTP3_DATA_FLAG_EOF;
+    stream->send_closed = TRUE;
   }
   else if(!nwritten) {
     /* Not EOF, and nothing to give, we signal WOULDBLOCK. */
@@ -1526,9 +1530,10 @@ cb_h3_read_req_body(nghttp3_conn *conn, int64_t stream_id,
   }
 
   DEBUGF(LOG_CF(data, cf, "[h3sid=%" PRId64 "] read req body -> "
-                "%d vecs%s with %zu/%zu", stream->id,
+                "%d vecs%s with %zu (buffered=%zu, left=%zd)", stream->id,
                 (int)nvecs, *pflags == NGHTTP3_DATA_FLAG_EOF?" EOF":"",
-                nwritten, Curl_bufq_len(&stream->sendbuf)));
+                nwritten, Curl_bufq_len(&stream->sendbuf),
+                stream->upload_left));
   return (nghttp3_ssize)nvecs;
 }
 
@@ -1604,12 +1609,17 @@ static ssize_t h3_stream_open(struct Curl_cfilter *cf,
   case HTTPREQ_POST_MIME:
   case HTTPREQ_PUT:
     /* known request body size or -1 */
+    if(data->state.infilesize != -1)
+      stream->upload_left = data->state.infilesize;
+    else
+      /* data sending without specifying the data amount up front */
+      stream->upload_left = -1; /* unknown */
     reader.read_data = cb_h3_read_req_body;
     preader = &reader;
     break;
   default:
     /* there is not request body */
-    stream->upload_done = TRUE;
+    stream->upload_left = 0; /* no request body */
     preader = NULL;
     break;
   }
@@ -2132,8 +2142,9 @@ static CURLcode cf_ngtcp2_data_event(struct Curl_cfilter *cf,
   }
   case CF_CTRL_DATA_DONE_SEND: {
     struct stream_ctx *stream = H3_STREAM_CTX(data);
-    if(stream) {
-      stream->upload_done = TRUE;
+    if(stream && !stream->send_closed) {
+      stream->send_closed = TRUE;
+      stream->upload_left = Curl_bufq_len(&stream->sendbuf);
       (void)nghttp3_conn_resume_stream(ctx->h3conn, stream->id);
     }
     break;
diff --git a/lib/vquic/curl_quiche.c b/lib/vquic/curl_quiche.c
index c63e8e10a2..3a4f9f9f20 100644
--- a/lib/vquic/curl_quiche.c
+++ b/lib/vquic/curl_quiche.c
@@ -188,9 +188,10 @@ struct stream_ctx {
   int64_t id; /* HTTP/3 protocol stream identifier */
   struct bufq recvbuf; /* h3 response */
   uint64_t error3; /* HTTP/3 stream error code */
+  curl_off_t upload_left; /* number of request bytes left to upload */
   bool closed; /* TRUE on stream close */
   bool reset;  /* TRUE on stream reset */
-  bool upload_done; /* stream is locally closed */
+  bool send_closed; /* stream is locally closed */
   bool resp_hds_complete;  /* complete, final response has been received */
   bool resp_got_header; /* TRUE when h3 stream has recvd some HEADER */
 };
@@ -307,7 +308,7 @@ static void drain_stream(struct Curl_cfilter *cf,
 
   (void)cf;
   bits = CURL_CSELECT_IN;
-  if(stream && !stream->upload_done)
+  if(stream && !stream->send_closed && stream->upload_left)
     bits |= CURL_CSELECT_OUT;
   if(data->state.dselect_bits != bits) {
     data->state.dselect_bits = bits;
@@ -464,6 +465,7 @@ static CURLcode cf_recv_body(struct Curl_cfilter *cf,
           nwritten, stream->id);
     stream->closed = TRUE;
     stream->reset = TRUE;
+    stream->send_closed = TRUE;
     streamclose(cf->conn, "Reset of stream");
     return result;
   }
@@ -529,6 +531,7 @@ static CURLcode h3_process_event(struct Curl_cfilter *cf,
     DEBUGF(LOG_CF(data, cf, "[h3sid=%"PRId64"][RESET]", stream3_id));
     stream->closed = TRUE;
     stream->reset = TRUE;
+    stream->send_closed = TRUE;
     streamclose(cf->conn, "Reset of stream");
     break;
 
@@ -951,18 +954,21 @@ static ssize_t h3_open_stream(struct Curl_cfilter *cf,
   case HTTPREQ_POST_MIME:
   case HTTPREQ_PUT:
     if(data->state.infilesize != -1)
-      stream->upload_done = !data->state.infilesize;
+      stream->upload_left = data->state.infilesize;
     else
       /* data sending without specifying the data amount up front */
-      stream->upload_done = FALSE;
+      stream->upload_left = -1; /* unknown */
     break;
   default:
-    stream->upload_done = TRUE;
+    stream->upload_left = 0; /* no request body */
     break;
   }
 
+  if(stream->upload_left == 0)
+    stream->send_closed = TRUE;
+
   stream3_id = quiche_h3_send_request(ctx->h3c, ctx->qconn, nva, nheader,
-                                      stream->upload_done);
+                                      stream->send_closed);
   if(stream3_id < 0) {
     if(QUICHE_H3_ERR_STREAM_BLOCKED == stream3_id) {
       /* quiche seems to report this error if the connection window is
@@ -1022,11 +1028,10 @@ static ssize_t cf_quiche_send(struct Curl_cfilter *cf, struct Curl_easy *data,
     stream = H3_STREAM_CTX(data);
   }
   else {
+    bool eof = (stream->upload_left >= 0 &&
+                (curl_off_t)len >= stream->upload_left);
     nwritten = quiche_h3_send_body(ctx->h3c, ctx->qconn, stream->id,
-                                   (uint8_t *)buf, len, stream->upload_done);
-    DEBUGF(LOG_CF(data, cf, "[h3sid=%" PRId64 "] send body(len=%zu, eof=%d) "
-                  "-> %zd", stream->id, len, stream->upload_done,
-                  nwritten));
+                                   (uint8_t *)buf, len, eof);
     if(nwritten == QUICHE_H3_ERR_DONE || (nwritten == 0 && len > 0)) {
       /* TODO: we seem to be blocked on flow control and should HOLD
        * sending. But when do we open again? */
@@ -1055,6 +1060,16 @@ static ssize_t cf_quiche_send(struct Curl_cfilter *cf, struct Curl_easy *data,
     }
     else {
       /* quiche accepted all or at least a part of the buf */
+      if(stream->upload_left > 0) {
+        stream->upload_left = (nwritten < stream->upload_left)?
+                              (stream->upload_left - nwritten) : 0;
+      }
+      if(stream->upload_left == 0)
+        stream->send_closed = TRUE;
+
+      DEBUGF(LOG_CF(data, cf, "[h3sid=%" PRId64 "] send body(len=%zu, "
+                    "left=%zd) -> %zd",
+                    stream->id, len, stream->upload_left, nwritten));
       *err = CURLE_OK;
     }
   }
@@ -1149,11 +1164,12 @@ static CURLcode cf_quiche_data_event(struct Curl_cfilter *cf,
   }
   case CF_CTRL_DATA_DONE_SEND: {
     struct stream_ctx *stream = H3_STREAM_CTX(data);
-    if(stream) {
+    if(stream && !stream->send_closed) {
       unsigned char body[1];
       ssize_t sent;
-      stream->upload_done = TRUE;
 
+      stream->send_closed = TRUE;
+      stream->upload_left = 0;
       body[0] = 'X';
       sent = cf_quiche_send(cf, data, body, 0, &result);
       DEBUGF(LOG_CF(data, cf, "[h3sid=%"PRId64"] DONE_SEND -> %zd, %d",