]> git.ipfire.org Git - thirdparty/curl.git/commitdiff
ngtcp2: fix stall or busy loop on STOP_SENDING with upload data
authorTatsuhiro Tsujikawa <tatsuhiro.t@gmail.com>
Fri, 8 Jul 2022 09:48:09 +0000 (18:48 +0900)
committerDaniel Stenberg <daniel@haxx.se>
Sun, 10 Jul 2022 21:18:00 +0000 (23:18 +0200)
Fixes #9122
Closes #9123

lib/http.h
lib/transfer.c
lib/vquic/ngtcp2.c

index 9eff6b1ff90b02014449ec2db1556163bf4515b1..2ac287eca80f890ef0259d8cf88ba693675105d1 100644 (file)
@@ -227,13 +227,11 @@ struct HTTP {
   /*********** for HTTP/2 we store stream-local data here *************/
   int32_t stream_id; /* stream we are interested in */
 
-  bool bodystarted;
   /* We store non-final and final response headers here, per-stream */
   struct dynbuf header_recvbuf;
   size_t nread_header_recvbuf; /* number of bytes in header_recvbuf fed into
                                   upper layer */
   struct dynbuf trailer_recvbuf;
-  int status_code; /* HTTP status code */
   const uint8_t *pausedata; /* pointer to data received in on_data_chunk */
   size_t pauselen; /* the number of bytes left in data */
   bool close_handled; /* TRUE if stream closure is handled by libcurl */
@@ -244,6 +242,8 @@ struct HTTP {
   uint32_t error; /* HTTP/2 stream error code */
 #endif
 #if defined(USE_NGHTTP2) || defined(USE_NGHTTP3)
+  bool bodystarted;
+  int status_code; /* HTTP status code */
   bool closed; /* TRUE on HTTP2 stream close */
   char *mem;     /* points to a buffer in memory to store received data */
   size_t len;    /* size of the buffer 'mem' points to */
@@ -260,6 +260,7 @@ struct HTTP {
 #ifndef USE_MSH3
   /*********** for HTTP/3 we store stream-local data here *************/
   int64_t stream3_id; /* stream we are interested in */
+  uint64_t error3; /* HTTP/3 stream error code */
   bool firstheader;  /* FALSE until headers arrive */
   bool firstbody;  /* FALSE until body arrives */
   bool h3req;    /* FALSE until request is issued */
index 1720b24b123e3eb679e078764e5f932267de9e1e..6560d9607d7ca6bc47cc38d09b926bec89f59e5f 100644 (file)
@@ -539,6 +539,13 @@ static CURLcode readwrite_data(struct Curl_easy *data,
     bool is_http2 = ((conn->handler->protocol & PROTO_FAMILY_HTTP) &&
                      (conn->httpversion == 20));
 #endif
+    bool is_http3 =
+#ifdef ENABLE_QUIC
+      ((conn->handler->protocol & PROTO_FAMILY_HTTP) &&
+       (conn->httpversion == 30));
+#else
+      FALSE;
+#endif
 
     if(
 #ifdef USE_NGHTTP2
@@ -549,6 +556,7 @@ static CURLcode readwrite_data(struct Curl_easy *data,
          for a particular stream. */
       !is_http2 &&
 #endif
+      !is_http3 && /* Same reason mentioned above. */
       k->size != -1 && !k->header) {
       /* make sure we don't read too much */
       curl_off_t totalleft = k->size - k->bytecount;
@@ -596,6 +604,9 @@ static CURLcode readwrite_data(struct Curl_easy *data,
         DEBUGF(infof(data, "nread == 0, stream closed, bailing"));
       else
 #endif
+      if(is_http3 && !nread)
+        DEBUGF(infof(data, "nread == 0, stream closed, bailing"));
+      else
         DEBUGF(infof(data, "nread <= 0, server closed connection, bailing"));
       k->keepon &= ~KEEP_RECV;
       break;
@@ -753,7 +764,13 @@ static CURLcode readwrite_data(struct Curl_easy *data,
         if(nread < 0) /* this should be unusual */
           nread = 0;
 
-        k->keepon &= ~KEEP_RECV; /* we're done reading */
+        /* HTTP/3 over QUIC should keep reading until QUIC connection
+           is closed.  In contrast to HTTP/2 which can stop reading
+           from TCP connection, HTTP/3 over QUIC needs ACK from server
+           to ensure stream closure.  It should keep reading. */
+        if(!is_http3) {
+          k->keepon &= ~KEEP_RECV; /* we're done reading */
+        }
       }
 
       k->bytecount += nread;
index 2d5f7f3c1a2d965030ecc79d59628194bf426f73..27ac579bcb4dd847110614eed993b8b2439e408b 100644 (file)
@@ -899,6 +899,7 @@ static int cb_h3_stream_close(nghttp3_conn *conn, int64_t stream_id,
   H3BUGF(infof(data, "cb_h3_stream_close CALLED"));
 
   stream->closed = TRUE;
+  stream->error3 = app_error_code;
   Curl_expire(data, 0, EXPIRE_QUIC);
   /* make sure that ngh3_stream_recv is called again to complete the transfer
      even if there are no more packets to be received from the server. */
@@ -1010,6 +1011,10 @@ static int cb_h3_end_headers(nghttp3_conn *conn, int64_t stream_id,
       return -1;
     }
   }
+
+  if(stream->status_code / 100 != 1) {
+    stream->bodystarted = TRUE;
+  }
   return 0;
 }
 
@@ -1032,9 +1037,10 @@ static int cb_h3_recv_header(nghttp3_conn *conn, int64_t stream_id,
   if(token == NGHTTP3_QPACK_TOKEN__STATUS) {
     char line[14]; /* status line is always 13 characters long */
     size_t ncopy;
-    int status = decode_status_code(h3val.base, h3val.len);
-    DEBUGASSERT(status != -1);
-    ncopy = msnprintf(line, sizeof(line), "HTTP/3 %03d \r\n", status);
+    stream->status_code = decode_status_code(h3val.base, h3val.len);
+    DEBUGASSERT(stream->status_code != -1);
+    ncopy = msnprintf(line, sizeof(line), "HTTP/3 %03d \r\n",
+                      stream->status_code);
     result = write_data(stream, line, ncopy);
     if(result) {
       return -1;
@@ -1226,6 +1232,24 @@ static ssize_t ngh3_stream_recv(struct Curl_easy *data,
   }
 
   if(stream->closed) {
+    if(stream->error3 != NGHTTP3_H3_NO_ERROR) {
+      failf(data,
+            "HTTP/3 stream %" PRId64 " was not closed cleanly: (err %" PRIu64
+            ")",
+            stream->stream3_id, stream->error3);
+      *curlcode = CURLE_HTTP3;
+      return -1;
+    }
+
+    if(!stream->bodystarted) {
+      failf(data,
+            "HTTP/3 stream %" PRId64 " was closed cleanly, but before getting"
+            " all response header fields, treated as error",
+            stream->stream3_id);
+      *curlcode = CURLE_HTTP3;
+      return -1;
+    }
+
     *curlcode = CURLE_OK;
     return 0;
   }
@@ -1444,6 +1468,11 @@ static ssize_t ngh3_stream_send(struct Curl_easy *data,
   curl_socket_t sockfd = conn->sock[sockindex];
   struct HTTP *stream = data->req.p.http;
 
+  if(stream->closed) {
+    *curlcode = CURLE_HTTP3;
+    return -1;
+  }
+
   if(!stream->h3req) {
     CURLcode result = http_request(data, mem, len);
     if(result) {