]> git.ipfire.org Git - thirdparty/curl.git/commitdiff
pingpong: drain the input buffer when reading responses
authorDaniel Stenberg <daniel@haxx.se>
Mon, 19 Aug 2024 10:00:15 +0000 (12:00 +0200)
committerDaniel Stenberg <daniel@haxx.se>
Mon, 19 Aug 2024 21:31:38 +0000 (23:31 +0200)
As the data might be held by TLS buffers, leaving some and expecting to
get called again is error prone.

Reported-by: ralfjunker on github
Fixes #14201
Closes #14597

lib/pingpong.c

index f31d1256f5c5af75516e705958d2d4148848cff5..817e3f69a0b498b44bfe2ef3b67187bec0b14607 100644 (file)
@@ -286,94 +286,99 @@ CURLcode Curl_pp_readresp(struct Curl_easy *data,
 {
   struct connectdata *conn = data->conn;
   CURLcode result = CURLE_OK;
+  ssize_t gotbytes;
+  char buffer[900];
 
   *code = 0; /* 0 for errors or not done */
   *size = 0;
 
-  if(pp->nfinal) {
-    /* a previous call left this many bytes in the beginning of the buffer as
-       that was the final line; now ditch that */
-    size_t full = Curl_dyn_len(&pp->recvbuf);
+  do {
+    gotbytes = 0;
+    if(pp->nfinal) {
+      /* a previous call left this many bytes in the beginning of the buffer as
+         that was the final line; now ditch that */
+      size_t full = Curl_dyn_len(&pp->recvbuf);
 
-    /* trim off the "final" leading part */
-    Curl_dyn_tail(&pp->recvbuf, full -  pp->nfinal);
+      /* trim off the "final" leading part */
+      Curl_dyn_tail(&pp->recvbuf, full -  pp->nfinal);
 
-    pp->nfinal = 0; /* now gone */
-  }
-  if(!pp->overflow) {
-    ssize_t gotbytes = 0;
-    char buffer[900];
+      pp->nfinal = 0; /* now gone */
+    }
+    if(!pp->overflow) {
+      result = pingpong_read(data, sockindex, buffer, sizeof(buffer),
+                             &gotbytes);
+      if(result == CURLE_AGAIN)
+        return CURLE_OK;
 
-    result = pingpong_read(data, sockindex, buffer, sizeof(buffer), &gotbytes);
-    if(result == CURLE_AGAIN)
-      return CURLE_OK;
+      if(result)
+        return result;
 
-    if(result)
-      return result;
+      if(gotbytes <= 0) {
+        failf(data, "response reading failed (errno: %d)", SOCKERRNO);
+        return CURLE_RECV_ERROR;
+      }
 
-    if(gotbytes <= 0) {
-      failf(data, "response reading failed (errno: %d)", SOCKERRNO);
-      return CURLE_RECV_ERROR;
-    }
+      result = Curl_dyn_addn(&pp->recvbuf, buffer, gotbytes);
+      if(result)
+        return result;
 
-    result = Curl_dyn_addn(&pp->recvbuf, buffer, gotbytes);
-    if(result)
-      return result;
+      data->req.headerbytecount += (unsigned int)gotbytes;
 
-    data->req.headerbytecount += (unsigned int)gotbytes;
+      pp->nread_resp += gotbytes;
+    }
 
-    pp->nread_resp += gotbytes;
-  }
+    do {
+      char *line = Curl_dyn_ptr(&pp->recvbuf);
+      char *nl = memchr(line, '\n', Curl_dyn_len(&pp->recvbuf));
+      if(nl) {
+        /* a newline is CRLF in pp-talk, so the CR is ignored as
+           the line is not really terminated until the LF comes */
+        size_t length = nl - line + 1;
 
-  do {
-    char *line = Curl_dyn_ptr(&pp->recvbuf);
-    char *nl = memchr(line, '\n', Curl_dyn_len(&pp->recvbuf));
-    if(nl) {
-      /* a newline is CRLF in pp-talk, so the CR is ignored as
-         the line is not really terminated until the LF comes */
-      size_t length = nl - line + 1;
-
-      /* output debug output if that is requested */
+        /* output debug output if that is requested */
 #ifdef HAVE_GSSAPI
-      if(!conn->sec_complete)
+        if(!conn->sec_complete)
 #endif
-        Curl_debug(data, CURLINFO_HEADER_IN, line, length);
-
-      /*
-       * Pass all response-lines to the callback function registered for
-       * "headers". The response lines can be seen as a kind of headers.
-       */
-      result = Curl_client_write(data, CLIENTWRITE_INFO, line, length);
-      if(result)
-        return result;
-
-      if(pp->endofresp(data, conn, line, length, code)) {
-        /* When at "end of response", keep the endofresp line first in the
-           buffer since it will be accessed outside (by pingpong
-           parsers). Store the overflow counter to inform about additional
-           data in this buffer after the endofresp line. */
-        pp->nfinal = length;
+          Curl_debug(data, CURLINFO_HEADER_IN, line, length);
+
+        /*
+         * Pass all response-lines to the callback function registered for
+         * "headers". The response lines can be seen as a kind of headers.
+         */
+        result = Curl_client_write(data, CLIENTWRITE_INFO, line, length);
+        if(result)
+          return result;
+
+        if(pp->endofresp(data, conn, line, length, code)) {
+          /* When at "end of response", keep the endofresp line first in the
+             buffer since it will be accessed outside (by pingpong
+             parsers). Store the overflow counter to inform about additional
+             data in this buffer after the endofresp line. */
+          pp->nfinal = length;
+          if(Curl_dyn_len(&pp->recvbuf) > length)
+            pp->overflow = Curl_dyn_len(&pp->recvbuf) - length;
+          else
+            pp->overflow = 0;
+          *size = pp->nread_resp; /* size of the response */
+          pp->nread_resp = 0; /* restart */
+          gotbytes = 0; /* force break out of outer loop */
+          break;
+        }
         if(Curl_dyn_len(&pp->recvbuf) > length)
-          pp->overflow = Curl_dyn_len(&pp->recvbuf) - length;
+          /* keep the remaining piece */
+          Curl_dyn_tail((&pp->recvbuf), Curl_dyn_len(&pp->recvbuf) - length);
         else
-          pp->overflow = 0;
-        *size = pp->nread_resp; /* size of the response */
-        pp->nread_resp = 0; /* restart */
+          Curl_dyn_reset(&pp->recvbuf);
+      }
+      else {
+        /* without a newline, there is no overflow */
+        pp->overflow = 0;
         break;
       }
-      if(Curl_dyn_len(&pp->recvbuf) > length)
-        /* keep the remaining piece */
-        Curl_dyn_tail((&pp->recvbuf), Curl_dyn_len(&pp->recvbuf) - length);
-      else
-        Curl_dyn_reset(&pp->recvbuf);
-    }
-    else {
-      /* without a newline, there is no overflow */
-      pp->overflow = 0;
-      break;
-    }
 
-  } while(1); /* while there is buffer left to scan */
+    } while(1); /* while there is buffer left to scan */
+
+  } while(gotbytes == sizeof(buffer));
 
   pp->pending_resp = FALSE;