mime: do not perform more than one read in a row

Input buffer filling may delay the data sending if data reads are slow.
To overcome this problem, file and callback data reads do not accumulate
in buffer anymore. All other data (memory data and mime framing) are
considered as fast and still concatenated in buffer.
As this may highly impact performance in terms of data overhead, an early
end of part data check is added to spare a read call.
When encoding a part's data, an encoder may require more bytes than made
available by a single read. In this case, the above rule does not apply
and reads are performed until the encoder is able to deliver some data.

Tests 643, 644, 645, 650 and 654 have been adapted to the output data
changes, with test data size reduced to avoid the boredom of long lists of
1-byte chunks in verification data.
New test 667 checks mimepost using single-byte read callback with encoder.
New test 668 checks the end of part data early detection.

Fixes #4826
Reported-by: MrdUkk on github

2 files changed, 149 insertions, 50 deletions

diff --git a/lib/mime.c b/lib/mime.c
index 5f928a171..7d754e63b 100644
--- a/lib/mime.c
+++ b/lib/mime.c
@@ -26,6 +26,7 @@
 
 #include "mime.h"
 #include "non-ascii.h"
+#include "warnless.h"
 #include "urldata.h"
 #include "sendf.h"
 
@@ -52,6 +53,10 @@
 
 
 #define READ_ERROR                      ((size_t) -1)
+#define STOP_FILLING                    ((size_t) -2)
+
+static size_t mime_subparts_read(char *buffer, size_t size, size_t nitems,
+                                 void *instream, bool *hasread);
 
 /* Encoders. */
 static size_t encoder_nop_read(char *buffer, size_t size, bool ateof,
@@ -354,10 +359,15 @@ static size_t encoder_nop_read(char *buffer, size_t size, bool ateof,
 
   (void) ateof;
 
+  if(!size)
+    return STOP_FILLING;
+
   if(size > insize)
     size = insize;
+
   if(size)
     memcpy(buffer, st->buf, size);
+
   st->bufbeg += size;
   return size;
 }
@@ -377,6 +387,9 @@ static size_t encoder_7bit_read(char *buffer, size_t size, bool ateof,
 
   (void) ateof;
 
+  if(!size)
+    return STOP_FILLING;
+
   if(size > cursize)
     size = cursize;
 
@@ -404,8 +417,11 @@ static size_t encoder_base64_read(char *buffer, size_t size, bool ateof,
     /* Line full ? */
     if(st->pos > MAX_ENCODED_LINE_LENGTH - 4) {
       /* Yes, we need 2 characters for CRLF. */
-      if(size < 2)
+      if(size < 2) {
+        if(!cursize)
+          return STOP_FILLING;
         break;
+      }
       *ptr++ = '\r';
       *ptr++ = '\n';
       st->pos = 0;
@@ -414,7 +430,12 @@ static size_t encoder_base64_read(char *buffer, size_t size, bool ateof,
     }
 
     /* Be sure there is enough space and input data for a base64 group. */
-    if(size < 4 || st->bufend - st->bufbeg < 3)
+    if(size < 4) {
+      if(!cursize)
+        return STOP_FILLING;
+      break;
+    }
+    if(st->bufend - st->bufbeg < 3)
       break;
 
     /* Encode three bytes as four characters. */
@@ -431,25 +452,31 @@ static size_t encoder_base64_read(char *buffer, size_t size, bool ateof,
   }
 
   /* If at eof, we have to flush the buffered data. */
-  if(ateof && size >= 4) {
-    /* Buffered data size can only be 0, 1 or 2. */
-    ptr[2] = ptr[3] = '=';
-    i = 0;
-    switch(st->bufend - st->bufbeg) {
-    case 2:
-      i = (st->buf[st->bufbeg + 1] & 0xFF) << 8;
-      /* FALLTHROUGH */
-    case 1:
-      i |= (st->buf[st->bufbeg] & 0xFF) << 16;
-      ptr[0] = base64[(i >> 18) & 0x3F];
-      ptr[1] = base64[(i >> 12) & 0x3F];
-      if(++st->bufbeg != st->bufend) {
-        ptr[2] = base64[(i >> 6) & 0x3F];
-        st->bufbeg++;
+  if(ateof) {
+    if(size < 4) {
+      if(!cursize)
+        return STOP_FILLING;
+    }
+    else {
+      /* Buffered data size can only be 0, 1 or 2. */
+      ptr[2] = ptr[3] = '=';
+      i = 0;
+      switch(st->bufend - st->bufbeg) {
+      case 2:
+        i = (st->buf[st->bufbeg + 1] & 0xFF) << 8;
+        /* FALLTHROUGH */
+      case 1:
+        i |= (st->buf[st->bufbeg] & 0xFF) << 16;
+        ptr[0] = base64[(i >> 18) & 0x3F];
+        ptr[1] = base64[(i >> 12) & 0x3F];
+        if(++st->bufbeg != st->bufend) {
+          ptr[2] = base64[(i >> 6) & 0x3F];
+          st->bufbeg++;
+        }
+        cursize += 4;
+        st->pos += 4;
+        break;
       }
-      cursize += 4;
-      st->pos += 4;
-      break;
     }
   }
 
@@ -581,8 +608,11 @@ static size_t encoder_qp_read(char *buffer, size_t size, bool ateof,
     }
 
     /* If the output buffer would overflow, do not store. */
-    if(len > size)
+    if(len > size) {
+      if(!cursize)
+        return STOP_FILLING;
       break;
+    }
 
     /* Append to output buffer. */
     memcpy(ptr, buf, len);
@@ -612,16 +642,18 @@ static size_t mime_mem_read(char *buffer, size_t size, size_t nitems,
                             void *instream)
 {
   curl_mimepart *part = (curl_mimepart *) instream;
-  size_t sz = (size_t) part->datasize - part->state.offset;
+  size_t sz = curlx_sotouz(part->datasize - part->state.offset);
   (void) size;   /* Always 1.*/
 
+  if(!nitems)
+    return STOP_FILLING;
+
   if(sz > nitems)
     sz = nitems;
 
   if(sz)
-    memcpy(buffer, (char *) &part->data[part->state.offset], sz);
+    memcpy(buffer, part->data + curlx_sotouz(part->state.offset), sz);
 
-  part->state.offset += sz;
   return sz;
 }
 
@@ -641,7 +673,7 @@ static int mime_mem_seek(void *instream, curl_off_t offset, int whence)
   if(offset < 0 || offset > part->datasize)
     return CURL_SEEKFUNC_FAIL;
 
-  part->state.offset = (size_t) offset;
+  part->state.offset = offset;
   return CURL_SEEKFUNC_OK;
 }
 
@@ -668,6 +700,9 @@ static size_t mime_file_read(char *buffer, size_t size, size_t nitems,
 {
   curl_mimepart *part = (curl_mimepart *) instream;
 
+  if(!nitems)
+    return STOP_FILLING;
+
   if(mime_open_file(part))
     return READ_ERROR;
 
@@ -711,15 +746,16 @@ static size_t readback_bytes(mime_state *state,
                              const char *trail)
 {
   size_t sz;
+  size_t offset = curlx_sotouz(state->offset);
 
-  if(numbytes > state->offset) {
-    sz = numbytes - state->offset;
-    bytes += state->offset;
+  if(numbytes > offset) {
+    sz = numbytes - offset;
+    bytes += offset;
   }
   else {
     size_t tsz = strlen(trail);
 
-    sz = state->offset - numbytes;
+    sz = offset - numbytes;
     if(sz >= tsz)
       return 0;
     bytes = trail + sz;
@@ -736,7 +772,7 @@ static size_t readback_bytes(mime_state *state,
 
 /* Read a non-encoded part content. */
 static size_t read_part_content(curl_mimepart *part,
-                                char *buffer, size_t bufsize)
+                                char *buffer, size_t bufsize, bool *hasread)
 {
   size_t sz = 0;
 
@@ -745,26 +781,70 @@ static size_t read_part_content(curl_mimepart *part,
   case CURL_READFUNC_ABORT:
   case CURL_READFUNC_PAUSE:
   case READ_ERROR:
+    return part->lastreadstatus;
+  default:
+    break;
+  }
+
+  /* If we can determine we are at end of part data, spare a read. */
+  if(part->datasize != (curl_off_t) -1 &&
+     part->state.offset >= part->datasize) {
+    /* sz is already zero. */
+  }
+  else {
+    switch(part->kind) {
+    case MIMEKIND_MULTIPART:
+      /*
+       * Cannot be processed as other kinds since read function requires
+       * an additional parameter and is highly recursive.
+       */
+       sz = mime_subparts_read(buffer, 1, bufsize, part->arg, hasread);
+       break;
+    case MIMEKIND_FILE:
+      if(part->fp && feof(part->fp))
+        break;  /* At EOF. */
+      /* FALLTHROUGH */
+    default:
+      if(part->readfunc) {
+        if(!(part->flags & MIME_FAST_READ)) {
+          if(*hasread)
+            return STOP_FILLING;
+          *hasread = TRUE;
+        }
+        sz = part->readfunc(buffer, 1, bufsize, part->arg);
+      }
+      break;
+    }
+  }
+
+  switch(sz) {
+  case STOP_FILLING:
+    break;
+  case 0:
+  case CURL_READFUNC_ABORT:
+  case CURL_READFUNC_PAUSE:
+  case READ_ERROR:
+    part->lastreadstatus = sz;
     break;
   default:
-    if(part->readfunc)
-      sz = part->readfunc(buffer, 1, bufsize, part->arg);
+    part->state.offset += sz;
     part->lastreadstatus = sz;
     break;
   }
-  return part->lastreadstatus;
+
+  return sz;
 }
 
 /* Read and encode part content. */
-static size_t read_encoded_part_content(curl_mimepart *part,
-                                        char *buffer, size_t bufsize)
+static size_t read_encoded_part_content(curl_mimepart *part, char *buffer,
+                                        size_t bufsize, bool *hasread)
 {
   mime_encoder_state *st = &part->encstate;
   size_t cursize = 0;
   size_t sz;
   bool ateof = FALSE;
 
-  while(bufsize) {
+  for(;;) {
     if(st->bufbeg < st->bufend || ateof) {
       /* Encode buffered data. */
       sz = part->encoder->encodefunc(buffer, bufsize, ateof, part);
@@ -773,9 +853,8 @@ static size_t read_encoded_part_content(curl_mimepart *part,
         if(ateof)
           return cursize;
         break;
-      case CURL_READFUNC_ABORT:
-      case CURL_READFUNC_PAUSE:
       case READ_ERROR:
+      case STOP_FILLING:
         return cursize? cursize: sz;
       default:
         cursize += sz;
@@ -797,7 +876,7 @@ static size_t read_encoded_part_content(curl_mimepart *part,
     if(st->bufend >= sizeof(st->buf))
       return cursize? cursize: READ_ERROR;    /* Buffer full. */
     sz = read_part_content(part, st->buf + st->bufend,
-                           sizeof(st->buf) - st->bufend);
+                           sizeof(st->buf) - st->bufend, hasread);
     switch(sz) {
     case 0:
       ateof = TRUE;
@@ -805,6 +884,7 @@ static size_t read_encoded_part_content(curl_mimepart *part,
     case CURL_READFUNC_ABORT:
     case CURL_READFUNC_PAUSE:
     case READ_ERROR:
+    case STOP_FILLING:
       return cursize? cursize: sz;
     default:
       st->bufend += sz;
@@ -812,12 +892,12 @@ static size_t read_encoded_part_content(curl_mimepart *part,
     }
   }
 
-  return cursize;
+  /* NOTREACHED */
 }
 
 /* Readback a mime part. */
 static size_t readback_part(curl_mimepart *part,
-                            char *buffer, size_t bufsize)
+                            char *buffer, size_t bufsize, bool *hasread)
 {
   size_t cursize = 0;
 #ifdef CURL_DOES_CONVERSIONS
@@ -876,9 +956,9 @@ static size_t readback_part(curl_mimepart *part,
       break;
     case MIMESTATE_CONTENT:
       if(part->encoder)
-        sz = read_encoded_part_content(part, buffer, bufsize);
+        sz = read_encoded_part_content(part, buffer, bufsize, hasread);
       else
-        sz = read_part_content(part, buffer, bufsize);
+        sz = read_part_content(part, buffer, bufsize, hasread);
       switch(sz) {
       case 0:
         mimesetstate(&part->state, MIMESTATE_END, NULL);
@@ -891,6 +971,7 @@ static size_t readback_part(curl_mimepart *part,
       case CURL_READFUNC_ABORT:
       case CURL_READFUNC_PAUSE:
       case READ_ERROR:
+      case STOP_FILLING:
         return cursize? cursize: sz;
       }
       break;
@@ -919,9 +1000,9 @@ static size_t readback_part(curl_mimepart *part,
   return cursize;
 }
 
-/* Readback from mime. */
+/* Readback from mime. Warning: not a read callback function. */
 static size_t mime_subparts_read(char *buffer, size_t size, size_t nitems,
-                                 void *instream)
+                                 void *instream, bool *hasread)
 {
   curl_mime *mime = (curl_mime *) instream;
   size_t cursize = 0;
@@ -942,7 +1023,7 @@ static size_t mime_subparts_read(char *buffer, size_t size, size_t nitems,
 #endif
       mimesetstate(&mime->state, MIMESTATE_BOUNDARY1, mime->firstpart);
       /* The first boundary always follows the header termination empty line,
-         so is always preceded by a CRLK. We can then spare 2 characters
+         so is always preceded by a CRLF. We can then spare 2 characters
          by skipping the leading CRLF in boundary. */
       mime->state.offset += 2;
       break;
@@ -972,11 +1053,12 @@ static size_t mime_subparts_read(char *buffer, size_t size, size_t nitems,
         mimesetstate(&mime->state, MIMESTATE_END, NULL);
         break;
       }
-      sz = readback_part(part, buffer, nitems);
+      sz = readback_part(part, buffer, nitems, hasread);
       switch(sz) {
       case CURL_READFUNC_ABORT:
       case CURL_READFUNC_PAUSE:
       case READ_ERROR:
+      case STOP_FILLING:
         return cursize? cursize: sz;
       case 0:
 #ifdef CURL_DOES_CONVERSIONS
@@ -1084,6 +1166,7 @@ static void cleanup_part_content(curl_mimepart *part)
   part->datasize = (curl_off_t) 0;    /* No size yet. */
   cleanup_encoder_state(&part->encstate);
   part->kind = MIMEKIND_NONE;
+  part->flags &= ~MIME_FAST_READ;
   part->lastreadstatus = 1; /* Successful read status. */
 }
 
@@ -1341,6 +1424,7 @@ CURLcode curl_mime_data(curl_mimepart *part,
     part->readfunc = mime_mem_read;
     part->seekfunc = mime_mem_seek;
     part->freefunc = mime_mem_free;
+    part->flags |= MIME_FAST_READ;
     part->kind = MIMEKIND_DATA;
   }
 
@@ -1515,7 +1599,7 @@ CURLcode Curl_mime_set_subparts(curl_mimepart *part,
     }
 
     subparts->parent = part;
-    part->readfunc = mime_subparts_read;
+    /* Subparts are processed internally: no read callback. */
     part->seekfunc = mime_subparts_seek;
     part->freefunc = take_ownership? mime_subparts_free: mime_subparts_unbind;
     part->arg = subparts;
@@ -1537,9 +1621,23 @@ CURLcode curl_mime_subparts(curl_mimepart *part, curl_mime *subparts)
 size_t Curl_mime_read(char *buffer, size_t size, size_t nitems, void *instream)
 {
   curl_mimepart *part = (curl_mimepart *) instream;
+  size_t ret;
+  bool hasread;
 
   (void) size;   /* Always 1. */
-  return readback_part(part, buffer, nitems);
+
+  do {
+    hasread = FALSE;
+    ret = readback_part(part, buffer, nitems, &hasread);
+    /*
+     * If this is not possible to get some data without calling more than
+     * one read callback (probably because a content encoder is not able to
+     * deliver a new bunch for the few data accumulated so far), force another
+     * read until we get enough data or a special exit code.
+     */
+  } while(ret == STOP_FILLING);
+
+  return ret;
 }
 
 /* Rewind mime stream. */
diff --git a/lib/mime.h b/lib/mime.h
index c6d374ec1..d7f25132e 100644
--- a/lib/mime.h
+++ b/lib/mime.h
@@ -31,6 +31,7 @@
 /* Part flags. */
 #define MIME_USERHEADERS_OWNER  (1 << 0)
 #define MIME_BODY_ONLY          (1 << 1)
+#define MIME_FAST_READ          (1 << 2)
 
 #define FILE_CONTENTTYPE_DEFAULT        "application/octet-stream"
 #define MULTIPART_CONTENTTYPE_DEFAULT   "multipart/mixed"
@@ -87,7 +88,7 @@ typedef struct {
 typedef struct {
   enum mimestate state;       /* Current state token. */
   void *ptr;                  /* State-dependent pointer. */
-  size_t offset;              /* State-dependent offset. */
+  curl_off_t offset;          /* State-dependent offset. */
 }  mime_state;
 
 /* minimum buffer size for the boundary string */