pingpong: stop using the download buffer

The pingpong logic now uses its own dynbuf for receiving command
response data.

When the "final" response header for a commanad has been received, that
final line is left first in the recvbuf for the protocols to parse at
will. If there is additional data behind the final response line, the
'overflow' counter is indicate how many bytes.

Closes #12757

lib/ftp.c

@@ -363,10 +363,11 @@ static CURLcode ReceivedServerConnect(struct Curl_easy *data, bool *received)
   curl_socket_t data_sock = conn->sock[SECONDARYSOCKET];
   struct ftp_conn *ftpc = &conn->proto.ftpc;
   struct pingpong *pp = &ftpc->pp;
-  int result;
+  int socketstate = 0;
   timediff_t timeout_ms;
   ssize_t nread;
   int ftpcode;
+  bool response = FALSE;
 
   *received = FALSE;
 
@@ -379,17 +380,21 @@ static CURLcode ReceivedServerConnect(struct Curl_easy *data, bool *received)
   }
 
   /* First check whether there is a cached response from server */
-  if(pp->cache_size && pp->cache && pp->cache[0] > '3') {
+  if(Curl_dyn_len(&pp->recvbuf) && (*Curl_dyn_ptr(&pp->recvbuf) > '3')) {
     /* Data connection could not be established, let's return */
     infof(data, "There is negative response in cache while serv connect");
     (void)Curl_GetFTPResponse(data, &nread, &ftpcode);
     return CURLE_FTP_ACCEPT_FAILED;
   }
 
-  result = Curl_socket_check(ctrl_sock, data_sock, CURL_SOCKET_BAD, 0);
+  if(pp->overflow)
+    /* there is pending control data still in the buffer to read */
+    response = TRUE;
+  else
+    socketstate = Curl_socket_check(ctrl_sock, data_sock, CURL_SOCKET_BAD, 0);
 
   /* see if the connection request is already here */
-  switch(result) {
+  switch(socketstate) {
   case -1: /* error */
     /* let's die here */
     failf(data, "Error while waiting for server connect");
@@ -397,23 +402,23 @@ static CURLcode ReceivedServerConnect(struct Curl_easy *data, bool *received)
   case 0:  /* Server connect is not received yet */
     break; /* loop */
   default:
-
-    if(result & CURL_CSELECT_IN2) {
+    if(socketstate & CURL_CSELECT_IN2) {
       infof(data, "Ready to accept data connection from server");
       *received = TRUE;
     }
-    else if(result & CURL_CSELECT_IN) {
-      infof(data, "Ctrl conn has data while waiting for data conn");
-      (void)Curl_GetFTPResponse(data, &nread, &ftpcode);
-
-      if(ftpcode/100 > 3)
-        return CURLE_FTP_ACCEPT_FAILED;
-
-      return CURLE_WEIRD_SERVER_REPLY;
-    }
-
+    else if(socketstate & CURL_CSELECT_IN)
+      response = TRUE;
     break;
-  } /* switch() */
+  }
+  if(response) {
+    infof(data, "Ctrl conn has data while waiting for data conn");
+    (void)Curl_GetFTPResponse(data, &nread, &ftpcode);
+
+    if(ftpcode/100 > 3)
+      return CURLE_FTP_ACCEPT_FAILED;
+
+    return CURLE_WEIRD_SERVER_REPLY;
+  }
 
   return CURLE_OK;
 }
@@ -554,7 +559,7 @@ static CURLcode ftp_readresp(struct Curl_easy *data,
 #ifdef HAVE_GSSAPI
   {
     struct connectdata *conn = data->conn;
-    char * const buf = data->state.buffer;
+    char * const buf = Curl_dyn_ptr(&data->conn->proto.ftpc.pp.recvbuf);
 
     /* handle the security-oriented responses 6xx ***/
     switch(code) {
@@ -660,7 +665,7 @@ CURLcode Curl_GetFTPResponse(struct Curl_easy *data,
      *
      */
 
-    if(pp->cache && (cache_skip < 2)) {
+    if(Curl_dyn_len(&pp->recvbuf) && (cache_skip < 2)) {
       /*
        * There's a cache left since before. We then skipping the wait for
        * socket action, unless this is the same cache like the previous round
@@ -688,7 +693,7 @@ CURLcode Curl_GetFTPResponse(struct Curl_easy *data,
     if(result)
       break;
 
-    if(!nread && pp->cache)
+    if(!nread && Curl_dyn_len(&pp->recvbuf))
       /* bump cache skip counter as on repeated skips we must wait for more
          data */
       cache_skip++;
@@ -1821,7 +1826,9 @@ static CURLcode ftp_state_pasv_resp(struct Curl_easy *data,
   struct Curl_dns_entry *addr = NULL;
   enum resolve_t rc;
   unsigned short connectport; /* the local port connect() should use! */
-  char *str = &data->state.buffer[4];  /* start on the first letter */
+  struct pingpong *pp = &ftpc->pp;
+  char *str =
+    Curl_dyn_ptr(&pp->recvbuf) + 4; /* start on the first letter */
 
   /* if we come here again, make sure the former name is cleared */
   Curl_safefree(ftpc->newhost);
@@ -2099,8 +2106,9 @@ static CURLcode ftp_state_mdtm_resp(struct Curl_easy *data,
       /* we got a time. Format should be: "YYYYMMDDHHMMSS[.sss]" where the
          last .sss part is optional and means fractions of a second */
       int year, month, day, hour, minute, second;
-      if(ftp_213_date(&data->state.buffer[4],
-                      &year, &month, &day, &hour, &minute, &second)) {
+      struct pingpong *pp = &ftpc->pp;
+      char *resp = Curl_dyn_ptr(&pp->recvbuf) + 4;
+      if(ftp_213_date(resp, &year, &month, &day, &hour, &minute, &second)) {
         /* we have a time, reformat it */
         char timebuf[24];
         msnprintf(timebuf, sizeof(timebuf),
@@ -2311,7 +2319,8 @@ static CURLcode ftp_state_size_resp(struct Curl_easy *data,
 {
   CURLcode result = CURLE_OK;
   curl_off_t filesize = -1;
-  char *buf = data->state.buffer;
+  char *buf = Curl_dyn_ptr(&data->conn->proto.ftpc.pp.recvbuf);
+  size_t len = data->conn->proto.ftpc.pp.nfinal;
 
   /* get the size from the ascii string: */
   if(ftpcode == 213) {
@@ -2319,13 +2328,13 @@ static CURLcode ftp_state_size_resp(struct Curl_easy *data,
        for all the digits at the end of the response and parse only those as a
        number. */
     char *start = &buf[4];
-    char *fdigit = strchr(start, '\r');
+    char *fdigit = memchr(start, '\r', len);
     if(fdigit) {
-      do
+      fdigit--;
+      if(*fdigit == '\n')
+        fdigit--;
+      while(ISDIGIT(fdigit[-1]) && (fdigit > start))
         fdigit--;
-      while(ISDIGIT(*fdigit) && (fdigit > start));
-      if(!ISDIGIT(*fdigit))
-        fdigit++;
     }
     else
       fdigit = start;
@@ -2494,7 +2503,7 @@ static CURLcode ftp_state_get_resp(struct Curl_easy *data,
        *
        * Example D above makes this parsing a little tricky */
       char *bytes;
-      char *buf = data->state.buffer;
+      char *buf = Curl_dyn_ptr(&conn->proto.ftpc.pp.recvbuf);
       bytes = strstr(buf, " bytes");
       if(bytes) {
         long in = (long)(--bytes-buf);
@@ -2763,7 +2772,7 @@ static CURLcode ftp_statemachine(struct Curl_easy *data,
     case FTP_AUTH:
       /* we have gotten the response to a previous AUTH command */
 
-      if(pp->cache_size)
+      if(pp->overflow)
         return CURLE_WEIRD_SERVER_REPLY; /* Forbid pipelining in response. */
 
       /* RFC2228 (page 5) says:
@@ -2861,8 +2870,8 @@ static CURLcode ftp_statemachine(struct Curl_easy *data,
 
     case FTP_PWD:
       if(ftpcode == 257) {
-        char *ptr = &data->state.buffer[4];  /* start on the first letter */
-        const size_t buf_size = data->set.buffer_size;
+        char *ptr = Curl_dyn_ptr(&pp->recvbuf) + 4; /* start on the first
+                                                       letter */
         bool entry_extracted = FALSE;
         struct dynbuf out;
         Curl_dyn_init(&out, 1000);
@@ -2877,8 +2886,7 @@ static CURLcode ftp_statemachine(struct Curl_easy *data,
         */
 
         /* scan for the first double-quote for non-standard responses */
-        while(ptr < &data->state.buffer[buf_size]
-              && *ptr != '\n' && *ptr != '\0' && *ptr != '"')
+        while(*ptr != '\n' && *ptr != '\0' && *ptr != '"')
           ptr++;
 
         if('\"' == *ptr) {
@@ -2950,7 +2958,8 @@ static CURLcode ftp_statemachine(struct Curl_easy *data,
 
     case FTP_SYST:
       if(ftpcode == 215) {
-        char *ptr = &data->state.buffer[4];  /* start on the first letter */
+        char *ptr = Curl_dyn_ptr(&pp->recvbuf) + 4; /* start on the first
+                                                       letter */
         char *os;
         char *start;
 
@@ -3191,8 +3200,7 @@ static CURLcode ftp_connect(struct Curl_easy *data,
     conn->bits.ftp_use_control_ssl = TRUE;
   }
 
-  Curl_pp_setup(pp); /* once per transfer */
-  Curl_pp_init(data, pp); /* init the generic pingpong data */
+  Curl_pp_init(pp); /* once per transfer */
 
   /* When we connect, we start in the state where we await the 220
      response */

lib/imap.c

@@ -355,8 +355,8 @@ static bool imap_endofresp(struct Curl_easy *data, struct connectdata *conn,
  */
 static CURLcode imap_get_message(struct Curl_easy *data, struct bufref *out)
 {
-  char *message = data->state.buffer;
-  size_t len = strlen(message);
+  char *message = Curl_dyn_ptr(&data->conn->proto.imapc.pp.recvbuf);
+  size_t len = data->conn->proto.imapc.pp.nfinal;
 
   if(len > 2) {
     /* Find the start of the message */
@@ -896,7 +896,7 @@ static CURLcode imap_state_capability_resp(struct Curl_easy *data,
   CURLcode result = CURLE_OK;
   struct connectdata *conn = data->conn;
   struct imap_conn *imapc = &conn->proto.imapc;
-  const char *line = data->state.buffer;
+  const char *line = Curl_dyn_ptr(&imapc->pp.recvbuf);
 
   (void)instate; /* no use for this yet */
 
@@ -982,7 +982,7 @@ static CURLcode imap_state_starttls_resp(struct Curl_easy *data,
   (void)instate; /* no use for this yet */
 
   /* Pipelining in response is forbidden. */
-  if(data->conn->proto.imapc.pp.cache_size)
+  if(data->conn->proto.imapc.pp.overflow)
     return CURLE_WEIRD_SERVER_REPLY;
 
   if(imapcode != IMAP_RESP_OK) {
@@ -1058,17 +1058,13 @@ static CURLcode imap_state_listsearch_resp(struct Curl_easy *data,
                                            imapstate instate)
 {
   CURLcode result = CURLE_OK;
-  char *line = data->state.buffer;
-  size_t len = strlen(line);
+  char *line = Curl_dyn_ptr(&data->conn->proto.imapc.pp.recvbuf);
+  size_t len = data->conn->proto.imapc.pp.nfinal;
 
   (void)instate; /* No use for this yet */
 
-  if(imapcode == '*') {
-    /* Temporarily add the LF character back and send as body to the client */
-    line[len] = '\n';
-    result = Curl_client_write(data, CLIENTWRITE_BODY, line, len + 1);
-    line[len] = '\0';
-  }
+  if(imapcode == '*')
+    result = Curl_client_write(data, CLIENTWRITE_BODY, line, len);
   else if(imapcode != IMAP_RESP_OK)
     result = CURLE_QUOTE_ERROR;
   else
@@ -1086,7 +1082,7 @@ static CURLcode imap_state_select_resp(struct Curl_easy *data, int imapcode,
   struct connectdata *conn = data->conn;
   struct IMAP *imap = data->req.p.imap;
   struct imap_conn *imapc = &conn->proto.imapc;
-  const char *line = data->state.buffer;
+  const char *line = Curl_dyn_ptr(&data->conn->proto.imapc.pp.recvbuf);
 
   (void)instate; /* no use for this yet */
 
@@ -1145,7 +1141,8 @@ static CURLcode imap_state_fetch_resp(struct Curl_easy *data,
   CURLcode result = CURLE_OK;
   struct imap_conn *imapc = &conn->proto.imapc;
   struct pingpong *pp = &imapc->pp;
-  const char *ptr = data->state.buffer;
+  const char *ptr = Curl_dyn_ptr(&data->conn->proto.imapc.pp.recvbuf);
+  size_t len = data->conn->proto.imapc.pp.nfinal;
   bool parsed = FALSE;
   curl_off_t size = 0;
 
@@ -1159,16 +1156,12 @@ static CURLcode imap_state_fetch_resp(struct Curl_easy *data,
 
   /* Something like this is received "* 1 FETCH (BODY[TEXT] {2021}\r" so parse
      the continuation data contained within the curly brackets */
-  while(*ptr && (*ptr != '{'))
-    ptr++;
-
-  if(*ptr == '{') {
+  ptr = memchr(ptr, '{', len);
+  if(ptr) {
     char *endptr;
-    if(!curlx_strtoofft(ptr + 1, &endptr, 10, &size)) {
-      if(endptr - ptr > 1 && endptr[0] == '}' &&
-         endptr[1] == '\r' && endptr[2] == '\0')
-        parsed = TRUE;
-    }
+    if(!curlx_strtoofft(ptr + 1, &endptr, 10, &size) &&
+       (endptr - ptr > 1 && *endptr == '}'))
+      parsed = TRUE;
   }
 
   if(parsed) {
@@ -1176,11 +1169,15 @@ static CURLcode imap_state_fetch_resp(struct Curl_easy *data,
           size);
     Curl_pgrsSetDownloadSize(data, size);
 
-    if(pp->cache) {
-      /* At this point there is a bunch of data in the header "cache" that is
-         actually body content, send it as body and then skip it. Do note
-         that there may even be additional "headers" after the body. */
-      size_t chunk = pp->cache_size;
+    if(pp->overflow) {
+      /* At this point there is a data in the receive buffer that is body
+         content, send it as body and then skip it. Do note that there may
+         even be additional "headers" after the body. */
+      size_t chunk = pp->overflow;
+
+      /* keep only the overflow */
+      Curl_dyn_tail(&pp->recvbuf, chunk);
+      pp->nfinal = 0; /* done */
 
       if(chunk > (size_t)size)
         /* The conversion from curl_off_t to size_t is always fine here */
@@ -1191,25 +1188,24 @@ static CURLcode imap_state_fetch_resp(struct Curl_easy *data,
         imap_state(data, IMAP_STOP);
         return CURLE_OK;
       }
-      result = Curl_client_write(data, CLIENTWRITE_BODY, pp->cache, chunk);
+      result = Curl_client_write(data, CLIENTWRITE_BODY,
+                                 Curl_dyn_ptr(&pp->recvbuf), chunk);
       if(result)
         return result;
 
       infof(data, "Written %zu bytes, %" CURL_FORMAT_CURL_OFF_TU
             " bytes are left for transfer", chunk, size - chunk);
 
-      /* Have we used the entire cache or just part of it?*/
-      if(pp->cache_size > chunk) {
-        /* Only part of it so shrink the cache to fit the trailing data */
-        memmove(pp->cache, pp->cache + chunk, pp->cache_size - chunk);
-        pp->cache_size -= chunk;
+      /* Have we used the entire overflow or just part of it?*/
+      if(pp->overflow > chunk) {
+        /* remember the remaining trailing overflow data */
+        pp->overflow -= chunk;
+        Curl_dyn_tail(&pp->recvbuf, pp->overflow);
       }
       else {
+        pp->overflow = 0; /* handled */
         /* Free the cache */
-        Curl_safefree(pp->cache);
-
-        /* Reset the cache size */
-        pp->cache_size = 0;
+        Curl_dyn_reset(&pp->recvbuf);
       }
     }
 
@@ -1472,9 +1468,7 @@ static CURLcode imap_connect(struct Curl_easy *data, bool *done)
   Curl_sasl_init(&imapc->sasl, data, &saslimap);
 
   Curl_dyn_init(&imapc->dyn, DYN_IMAP_CMD);
-  /* Initialise the pingpong layer */
-  Curl_pp_setup(pp);
-  Curl_pp_init(data, pp);
+  Curl_pp_init(pp);
 
   /* Parse the URL options */
   result = imap_parse_url_options(conn);

lib/pingpong.c

@@ -106,7 +106,7 @@ CURLcode Curl_pp_statemach(struct Curl_easy *data,
 
   if(Curl_conn_data_pending(data, FIRSTSOCKET))
     rc = 1;
-  else if(Curl_pp_moredata(pp))
+  else if(pp->overflow)
     /* We are receiving and there is data in the cache so just read it */
     rc = 1;
   else if(!pp->sendleft && Curl_conn_data_pending(data, FIRSTSOCKET))
@@ -140,19 +140,13 @@ CURLcode Curl_pp_statemach(struct Curl_easy *data,
 }
 
 /* initialize stuff to prepare for reading a fresh new response */
-void Curl_pp_init(struct Curl_easy *data, struct pingpong *pp)
+void Curl_pp_init(struct pingpong *pp)
 {
-  DEBUGASSERT(data);
   pp->nread_resp = 0;
-  pp->linestart_resp = data->state.buffer;
-  pp->pending_resp = TRUE;
   pp->response = Curl_now(); /* start response time-out now! */
-}
-
-/* setup for the coming transfer */
-void Curl_pp_setup(struct pingpong *pp)
-{
+  pp->pending_resp = TRUE;
   Curl_dyn_init(&pp->sendbuf, DYN_PINGPPONG_CMD);
+  Curl_dyn_init(&pp->recvbuf, DYN_PINGPPONG_CMD);
 }
 
 /***********************************************************************
@@ -198,9 +192,9 @@ CURLcode Curl_pp_vsendf(struct Curl_easy *data,
   if(result)
     return result;
 
+  pp->pending_resp = TRUE;
   write_len = Curl_dyn_len(&pp->sendbuf);
   s = Curl_dyn_ptr(&pp->sendbuf);
-  Curl_pp_init(data, pp);
 
 #ifdef HAVE_GSSAPI
   conn->data_prot = PROT_CMD;
@@ -256,6 +250,25 @@ CURLcode Curl_pp_sendf(struct Curl_easy *data, struct pingpong *pp,
   return result;
 }
 
+static CURLcode pingpong_read(struct Curl_easy *data,
+                              curl_socket_t sockfd,
+                              char *buffer,
+                              size_t buflen,
+                              ssize_t *nread)
+{
+  CURLcode result;
+#ifdef HAVE_GSSAPI
+  enum protection_level prot = data->conn->data_prot;
+  data->conn->data_prot = PROT_CLEAR;
+#endif
+  result = Curl_read(data, sockfd, buffer, buflen, nread);
+#ifdef HAVE_GSSAPI
+  DEBUGASSERT(prot  > PROT_NONE && prot < PROT_LAST);
+  data->conn->data_prot = (unsigned char)prot;
+#endif
+  return result;
+}
+
 /*
  * Curl_pp_readresp()
  *
@@ -267,179 +280,96 @@ CURLcode Curl_pp_readresp(struct Curl_easy *data,
                           int *code, /* return the server code if done */
                           size_t *size) /* size of the response */
 {
-  ssize_t perline; /* count bytes per line */
-  bool keepon = TRUE;
-  ssize_t gotbytes;
-  char *ptr;
   struct connectdata *conn = data->conn;
-  char * const buf = data->state.buffer;
   CURLcode result = CURLE_OK;
 
   *code = 0; /* 0 for errors or not done */
   *size = 0;
 
-  ptr = buf + pp->nread_resp;
+  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);
 
-  /* number of bytes in the current line, so far */
-  perline = (ssize_t)(ptr-pp->linestart_resp);
+    /* trim off the "final" leading part */
+    Curl_dyn_tail(&pp->recvbuf, full -  pp->nfinal);
 
-  while((pp->nread_resp < (size_t)data->set.buffer_size) &&
-        (keepon && !result)) {
+    pp->nfinal = 0; /* now gone */
+  }
+  if(!pp->overflow) {
+    ssize_t gotbytes = 0;
+    char buffer[900];
 
-    if(pp->cache) {
-      /* we had data in the "cache", copy that instead of doing an actual
-       * read
-       *
-       * pp->cache_size is cast to ssize_t here.  This should be safe, because
-       * it would have been populated with something of size int to begin
-       * with, even though its datatype may be larger than an int.
-       */
-      if((ptr + pp->cache_size) > (buf + data->set.buffer_size + 1)) {
-        failf(data, "cached response data too big to handle");
-        return CURLE_WEIRD_SERVER_REPLY;
-      }
-      memcpy(ptr, pp->cache, pp->cache_size);
-      gotbytes = (ssize_t)pp->cache_size;
-      free(pp->cache);    /* free the cache */
-      pp->cache = NULL;   /* clear the pointer */
-      pp->cache_size = 0; /* zero the size just in case */
-    }
-    else {
-#ifdef HAVE_GSSAPI
-      enum protection_level prot = conn->data_prot;
-      conn->data_prot = PROT_CLEAR;
-#endif
-      DEBUGASSERT((ptr + data->set.buffer_size - pp->nread_resp) <=
-                  (buf + data->set.buffer_size + 1));
-      result = Curl_read(data, sockfd, ptr,
-                         data->set.buffer_size - pp->nread_resp,
-                         &gotbytes);
-#ifdef HAVE_GSSAPI
-      DEBUGASSERT(prot  > PROT_NONE && prot < PROT_LAST);
-      conn->data_prot = (unsigned char)prot;
-#endif
-      if(result == CURLE_AGAIN)
-        return CURLE_OK; /* return */
+    result = pingpong_read(data, sockfd, buffer, sizeof(buffer), &gotbytes);
+    if(result == CURLE_AGAIN)
+      return CURLE_OK;
 
-      if(result)
-        /* Set outer result variable to this error. */
-        keepon = FALSE;
-    }
+    if(result)
+      return result;
 
-    if(!keepon)
-      ;
-    else if(gotbytes <= 0) {
-      keepon = FALSE;
-      result = 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;
+
+    data->req.headerbytecount += (unsigned int)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 isn't really terminated until the LF comes */
+      size_t length = nl - line + 1;
+
+      /* output debug output if that is requested */
+#ifdef HAVE_GSSAPI
+      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;
+        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 */
+        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 {
-      /* we got a whole chunk of data, which can be anything from one
-       * byte to a set of lines and possible just a piece of the last
-       * line */
-      ssize_t i;
-      ssize_t clipamount = 0;
-      bool restart = FALSE;
+      /* without a newline, there is no overflow */
+      pp->overflow = 0;
+      break;
+    }
 
-      data->req.headerbytecount += (unsigned int)gotbytes;
-
-      pp->nread_resp += gotbytes;
-      for(i = 0; i < gotbytes; ptr++, i++) {
-        perline++;
-        if(*ptr == '\n') {
-          /* a newline is CRLF in pp-talk, so the CR is ignored as
-             the line isn't really terminated until the LF comes */
-
-          /* output debug output if that is requested */
-#ifdef HAVE_GSSAPI
-          if(!conn->sec_complete)
-#endif
-            Curl_debug(data, CURLINFO_HEADER_IN,
-                       pp->linestart_resp, (size_t)perline);
-
-          /*
-           * We 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,
-                                     pp->linestart_resp, perline);
-          if(result)
-            return result;
-
-          if(pp->endofresp(data, conn, pp->linestart_resp, perline, code)) {
-            /* This is the end of the last line, copy the last line to the
-               start of the buffer and null-terminate, for old times sake */
-            size_t n = ptr - pp->linestart_resp;
-            memmove(buf, pp->linestart_resp, n);
-            buf[n] = 0; /* null-terminate */
-            keepon = FALSE;
-            pp->linestart_resp = ptr + 1; /* advance pointer */
-            i++; /* skip this before getting out */
-
-            *size = pp->nread_resp; /* size of the response */
-            pp->nread_resp = 0; /* restart */
-            break;
-          }
-          perline = 0; /* line starts over here */
-          pp->linestart_resp = ptr + 1;
-        }
-      }
-
-      if(!keepon && (i != gotbytes)) {
-        /* We found the end of the response lines, but we didn't parse the
-           full chunk of data we have read from the server. We therefore need
-           to store the rest of the data to be checked on the next invoke as
-           it may actually contain another end of response already! */
-        clipamount = gotbytes - i;
-        restart = TRUE;
-        DEBUGF(infof(data, "Curl_pp_readresp_ %d bytes of trailing "
-                     "server response left",
-                     (int)clipamount));
-      }
-      else if(keepon) {
-
-        if((perline == gotbytes) &&
-           (gotbytes > (ssize_t)data->set.buffer_size/2)) {
-          /* We got an excessive line without newlines and we need to deal
-             with it. We keep the first bytes of the line then we throw
-             away the rest. */
-          infof(data, "Excessive server response line length received, "
-                "%zd bytes. Stripping", gotbytes);
-          restart = TRUE;
-
-          /* we keep 40 bytes since all our pingpong protocols are only
-             interested in the first piece */
-          clipamount = 40;
-        }
-        else if(pp->nread_resp > (size_t)data->set.buffer_size/2) {
-          /* We got a large chunk of data and there's potentially still
-             trailing data to take care of, so we put any such part in the
-             "cache", clear the buffer to make space and restart. */
-          clipamount = perline;
-          restart = TRUE;
-        }
-      }
-      else if(i == gotbytes)
-        restart = TRUE;
-
-      if(clipamount) {
-        pp->cache_size = clipamount;
-        pp->cache = Curl_memdup(pp->linestart_resp, pp->cache_size);
-        if(!pp->cache)
-          return CURLE_OUT_OF_MEMORY;
-      }
-      if(restart) {
-        /* now reset a few variables to start over nicely from the start of
-           the big buffer */
-        pp->nread_resp = 0; /* start over from scratch in the buffer */
-        ptr = pp->linestart_resp = buf;
-        perline = 0;
-      }
-
-    } /* there was data */
-
-  } /* while there's buffer left and loop is requested */
+  } while(1); /* while there's buffer left to scan */
 
   pp->pending_resp = FALSE;
 
@@ -487,14 +417,13 @@ CURLcode Curl_pp_flushsend(struct Curl_easy *data,
 CURLcode Curl_pp_disconnect(struct pingpong *pp)
 {
   Curl_dyn_free(&pp->sendbuf);
-  Curl_safefree(pp->cache);
+  Curl_dyn_free(&pp->recvbuf);
   return CURLE_OK;
 }
 
 bool Curl_pp_moredata(struct pingpong *pp)
 {
-  return (!pp->sendleft && pp->cache && pp->nread_resp < pp->cache_size) ?
-    TRUE : FALSE;
+  return (!pp->sendleft && Curl_dyn_len(&pp->recvbuf));
 }
 
 #endif

lib/pingpong.h

@@ -47,16 +47,11 @@ typedef enum {
  * It holds response cache and non-blocking sending data.
  */
 struct pingpong {
-  char *cache;     /* data cache between getresponse()-calls */
-  size_t cache_size;  /* size of cache in bytes */
   size_t nread_resp;  /* number of bytes currently read of a server response */
-  char *linestart_resp; /* line start pointer for the server response
-                           reader function */
   bool pending_resp;  /* set TRUE when a server response is pending or in
                          progress, and is cleared once the last response is
                          read */
-  char *sendthis; /* allocated pointer to a buffer that is to be sent to the
-                     server */
+  char *sendthis; /* pointer to a buffer that is to be sent to the server */
   size_t sendleft; /* number of bytes left to send from the sendthis buffer */
   size_t sendsize; /* total size of the sendthis buffer */
   struct curltime response; /* set to Curl_now() when a command has been sent
@@ -64,6 +59,10 @@ struct pingpong {
   timediff_t response_time; /* When no timeout is given, this is the amount of
                                milliseconds we await for a server response. */
   struct dynbuf sendbuf;
+  struct dynbuf recvbuf;
+  size_t overflow; /* number of bytes left after a final response line */
+  size_t nfinal;   /* number of bytes in the final response line, which
+                      after a match is first in the receice buffer */
 
   /* Function pointers the protocols MUST implement and provide for the
      pingpong layer to function */
@@ -90,10 +89,7 @@ CURLcode Curl_pp_statemach(struct Curl_easy *data, struct pingpong *pp,
                            bool block, bool disconnecting);
 
 /* initialize stuff to prepare for reading a fresh new response */
-void Curl_pp_init(struct Curl_easy *data, struct pingpong *pp);
-
-/* setup for the transfer */
-void Curl_pp_setup(struct pingpong *pp);
+void Curl_pp_init(struct pingpong *pp);
 
 /* Returns timeout in ms. 0 or negative number means the timeout has already
    triggered */

lib/pop3.c

@@ -252,8 +252,8 @@ static bool pop3_endofresp(struct Curl_easy *data, struct connectdata *conn,
  */
 static CURLcode pop3_get_message(struct Curl_easy *data, struct bufref *out)
 {
-  char *message = data->state.buffer;
-  size_t len = strlen(message);
+  char *message = Curl_dyn_ptr(&data->conn->proto.pop3c.pp.recvbuf);
+  size_t len = data->conn->proto.pop3c.pp.nfinal;
 
   if(len > 2) {
     /* Find the start of the message */
@@ -649,8 +649,8 @@ static CURLcode pop3_state_servergreet_resp(struct Curl_easy *data,
   CURLcode result = CURLE_OK;
   struct connectdata *conn = data->conn;
   struct pop3_conn *pop3c = &conn->proto.pop3c;
-  const char *line = data->state.buffer;
-  size_t len = strlen(line);
+  const char *line = Curl_dyn_ptr(&data->conn->proto.pop3c.pp.recvbuf);
+  size_t len = data->conn->proto.pop3c.pp.nfinal;
 
   (void)instate; /* no use for this yet */
 
@@ -658,37 +658,30 @@ static CURLcode pop3_state_servergreet_resp(struct Curl_easy *data,
     failf(data, "Got unexpected pop3-server response");
     result = CURLE_WEIRD_SERVER_REPLY;
   }
-  else {
+  else if(len > 3) {
     /* Does the server support APOP authentication? */
-    if(len >= 4 && line[len - 2] == '>') {
-      /* Look for the APOP timestamp */
-      size_t i;
-      for(i = 3; i < len - 2; ++i) {
-        if(line[i] == '<') {
-          /* Calculate the length of the timestamp */
-          size_t timestamplen = len - 1 - i;
-          char *at;
-          if(!timestamplen)
-            break;
+    char *lt;
+    char *gt = NULL;
 
-          /* dupe the timestamp */
-          pop3c->apoptimestamp = Curl_memdup0(&line[i], timestamplen);
-          if(!pop3c->apoptimestamp) {
-            result = CURLE_OUT_OF_MEMORY;
-            break;
-          }
-
-          /* If the timestamp does not contain '@' it is not (as required by
-             RFC-1939) conformant to the RFC-822 message id syntax, and we
-             therefore do not use APOP authentication. */
-          at = strchr(pop3c->apoptimestamp, '@');
-          if(!at)
-            Curl_safefree(pop3c->apoptimestamp);
-          else
-            /* Store the APOP capability */
-            pop3c->authtypes |= POP3_TYPE_APOP;
-          break;
-        }
+    /* Look for the APOP timestamp */
+    lt = memchr(line, '<', len);
+    if(lt)
+      /* search the remainder for '>' */
+      gt = memchr(lt, '>', len - (lt - line));
+    if(gt) {
+      /* the length of the timestamp, including the brackets */
+      size_t timestamplen = gt - lt + 1;
+      char *at = memchr(lt, '@', timestamplen);
+      /* If the timestamp does not contain '@' it is not (as required by
+         RFC-1939) conformant to the RFC-822 message id syntax, and we
+         therefore do not use APOP authentication. */
+      if(at) {
+        /* dupe the timestamp */
+        pop3c->apoptimestamp = Curl_memdup0(lt, timestamplen);
+        if(!pop3c->apoptimestamp)
+          return CURLE_OUT_OF_MEMORY;
+        /* Store the APOP capability */
+        pop3c->authtypes |= POP3_TYPE_APOP;
       }
     }
 
@@ -706,8 +699,8 @@ static CURLcode pop3_state_capa_resp(struct Curl_easy *data, int pop3code,
   CURLcode result = CURLE_OK;
   struct connectdata *conn = data->conn;
   struct pop3_conn *pop3c = &conn->proto.pop3c;
-  const char *line = data->state.buffer;
-  size_t len = strlen(line);
+  const char *line = Curl_dyn_ptr(&data->conn->proto.pop3c.pp.recvbuf);
+  size_t len = data->conn->proto.pop3c.pp.nfinal;
 
   (void)instate; /* no use for this yet */
 
@@ -794,7 +787,7 @@ static CURLcode pop3_state_starttls_resp(struct Curl_easy *data,
   (void)instate; /* no use for this yet */
 
   /* Pipelining in response is forbidden. */
-  if(data->conn->proto.pop3c.pp.cache_size)
+  if(data->conn->proto.pop3c.pp.overflow)
     return CURLE_WEIRD_SERVER_REPLY;
 
   if(pop3code != '+') {
@@ -943,24 +936,29 @@ static CURLcode pop3_state_command_resp(struct Curl_easy *data,
     /* POP3 download */
     Curl_setup_transfer(data, FIRSTSOCKET, -1, FALSE, -1);
 
-    if(pp->cache) {
-      /* The header "cache" contains a bunch of data that is actually body
-         content so send it as such. Note that there may even be additional
-         "headers" after the body */
+    if(pp->overflow) {
+      /* The recv buffer contains data that is actually body content so send
+         it as such. Note that there may even be additional "headers" after
+         the body */
+
+      /* keep only the overflow */
+      Curl_dyn_tail(&pp->recvbuf, pp->overflow);
+      pp->nfinal = 0; /* done */
 
       if(!data->req.no_body) {
-        result = Curl_pop3_write(data, pp->cache, pp->cache_size);
+        result = Curl_pop3_write(data, Curl_dyn_ptr(&pp->recvbuf),
+                                 Curl_dyn_len(&pp->recvbuf));
         if(result)
           return result;
       }
 
-      /* Free the cache */
-      Curl_safefree(pp->cache);
-
-      /* Reset the cache size */
-      pp->cache_size = 0;
+      /* reset the buffer */
+      Curl_dyn_reset(&pp->recvbuf);
+      pp->overflow = 0;
     }
   }
+  else
+    pp->overflow = 0;
 
   /* End of DO phase */
   pop3_state(data, POP3_STOP);
@@ -1130,8 +1128,7 @@ static CURLcode pop3_connect(struct Curl_easy *data, bool *done)
   Curl_sasl_init(&pop3c->sasl, data, &saslpop3);
 
   /* Initialise the pingpong layer */
-  Curl_pp_setup(pp);
-  Curl_pp_init(data, pp);
+  Curl_pp_init(pp);
 
   /* Parse the URL options */
   result = pop3_parse_url_options(conn);

lib/smtp.c

@@ -250,8 +250,8 @@ static bool smtp_endofresp(struct Curl_easy *data, struct connectdata *conn,
  */
 static CURLcode smtp_get_message(struct Curl_easy *data, struct bufref *out)
 {
-  char *message = data->state.buffer;
-  size_t len = strlen(message);
+  char *message = Curl_dyn_ptr(&data->conn->proto.smtpc.pp.recvbuf);
+  size_t len = data->conn->proto.smtpc.pp.nfinal;
 
   if(len > 4) {
     /* Find the start of the message */
@@ -859,7 +859,7 @@ static CURLcode smtp_state_starttls_resp(struct Curl_easy *data,
   (void)instate; /* no use for this yet */
 
   /* Pipelining in response is forbidden. */
-  if(data->conn->proto.smtpc.pp.cache_size)
+  if(data->conn->proto.smtpc.pp.overflow)
     return CURLE_WEIRD_SERVER_REPLY;
 
   if(smtpcode != 220) {
@@ -883,8 +883,8 @@ static CURLcode smtp_state_ehlo_resp(struct Curl_easy *data,
 {
   CURLcode result = CURLE_OK;
   struct smtp_conn *smtpc = &conn->proto.smtpc;
-  const char *line = data->state.buffer;
-  size_t len = strlen(line);
+  const char *line = Curl_dyn_ptr(&smtpc->pp.recvbuf);
+  size_t len = smtpc->pp.nfinal;
 
   (void)instate; /* no use for this yet */
 
@@ -1033,8 +1033,8 @@ static CURLcode smtp_state_command_resp(struct Curl_easy *data, int smtpcode,
 {
   CURLcode result = CURLE_OK;
   struct SMTP *smtp = data->req.p.smtp;
-  char *line = data->state.buffer;
-  size_t len = strlen(line);
+  char *line = Curl_dyn_ptr(&data->conn->proto.smtpc.pp.recvbuf);
+  size_t len = data->conn->proto.smtpc.pp.nfinal;
 
   (void)instate; /* no use for this yet */
 
@@ -1044,12 +1044,8 @@ static CURLcode smtp_state_command_resp(struct Curl_easy *data, int smtpcode,
     result = CURLE_WEIRD_SERVER_REPLY;
   }
   else {
-    /* Temporarily add the LF character back and send as body to the client */
-    if(!data->req.no_body) {
-      line[len] = '\n';
-      result = Curl_client_write(data, CLIENTWRITE_BODY, line, len + 1);
-      line[len] = '\0';
-    }
+    if(!data->req.no_body)
+      result = Curl_client_write(data, CLIENTWRITE_BODY, line, len);
 
     if(smtpcode != 1) {
       if(smtp->rcpt) {
@@ -1361,8 +1357,7 @@ static CURLcode smtp_connect(struct Curl_easy *data, bool *done)
   Curl_sasl_init(&smtpc->sasl, data, &saslsmtp);
 
   /* Initialise the pingpong layer */
-  Curl_pp_setup(pp);
-  Curl_pp_init(data, pp);
+  Curl_pp_init(pp);
 
   /* Parse the URL options */
   result = smtp_parse_url_options(conn);
@@ -1540,6 +1535,8 @@ static CURLcode smtp_perform(struct Curl_easy *data, bool *connected,
 static CURLcode smtp_do(struct Curl_easy *data, bool *done)
 {
   CURLcode result = CURLE_OK;
+  DEBUGASSERT(data);
+  DEBUGASSERT(data->conn);
   *done = FALSE; /* default to false */
 
   /* Parse the custom request */

tests/data/test250

@@ -38,7 +38,7 @@ ftp
 <name>
 FTP dir list PASV with slow response
 </name>
-<command>
+<command option="binary-trace">
 ftp://%HOSTIP:%FTPPORT/
 </command>
 </client>